From 54f32d3e2786dc012b8b16a592ddd0623630a76e Mon Sep 17 00:00:00 2001 From: Amit Kapila Date: Fri, 8 Nov 2019 10:51:21 +0530 Subject: [PATCH] Introduce the 'force' option for the Drop Database command. This new option terminates the other sessions connected to the target database and then drop it. To terminate other sessions, the current user must have desired permissions (same as pg_terminate_backend()). We don't allow to terminate the sessions if prepared transactions, active logical replication slots or subscriptions are present in the target database. Author: Pavel Stehule with some changes by me Reviewed-by: Dilip Kumar, Vignesh C, Ibrar Ahmed, Anthony Nowocien, Ryan Lambert and Amit Kapila Discussion: https://postgr.es/m/CAP_rwwmLJJbn70vLOZFpxGw3XD7nLB_7+NKz46H5EOO2k5H7OQ@mail.gmail.com --- doc/src/sgml/ref/drop_database.sgml | 33 +++++++- src/backend/commands/dbcommands.c | 33 +++++++- src/backend/nodes/copyfuncs.c | 1 + src/backend/nodes/equalfuncs.c | 1 + src/backend/parser/gram.y | 43 ++++++++++- src/backend/storage/ipc/procarray.c | 111 +++++++++++++++++++++++++++ src/backend/tcop/utility.c | 10 +-- src/bin/psql/tab-complete.c | 4 + src/include/commands/dbcommands.h | 3 +- src/include/nodes/parsenodes.h | 1 + src/include/storage/procarray.h | 1 + src/test/regress/expected/drop_if_exists.out | 11 +++ src/test/regress/sql/drop_if_exists.sql | 8 ++ 13 files changed, 246 insertions(+), 14 deletions(-) diff --git a/doc/src/sgml/ref/drop_database.sgml b/doc/src/sgml/ref/drop_database.sgml index 3ac06c9..cef1b90 100644 --- a/doc/src/sgml/ref/drop_database.sgml +++ b/doc/src/sgml/ref/drop_database.sgml @@ -21,7 +21,11 @@ PostgreSQL documentation -DROP DATABASE [ IF EXISTS ] name +DROP DATABASE [ IF EXISTS ] name [ [ WITH ] ( option [, ...] ) ] + +where option can be: + + FORCE @@ -32,9 +36,11 @@ DROP DATABASE [ IF EXISTS ] name DROP DATABASE drops a database. It removes the catalog entries for the database and deletes the directory containing the data. It can only be executed by the database owner. - Also, it cannot be executed while you or anyone else are connected - to the target database. (Connect to postgres or any - other database to issue this command.) + It cannot be executed while you are connected to the target database. + (Connect to postgres or any other database to issue this + command.) + Also, if anyone else is connected to the target database, this command will + fail unless you use the FORCE option described below. @@ -64,6 +70,25 @@ DROP DATABASE [ IF EXISTS ] name + + + FORCE + + + Attempt to terminate all existing connections to the target database. + It doesn't terminate if prepared transactions, active logical replication + slots or subscriptions are present in the target database. + + + This will fail if the current user has no permissions to terminate other + connections. Required permissions are the same as with + pg_terminate_backend, described in + . This will also fail if we + are not able to terminate connections. + + + + diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c index 01d6621..38a2bfa 100644 --- a/src/backend/commands/dbcommands.c +++ b/src/backend/commands/dbcommands.c @@ -811,7 +811,7 @@ createdb_failure_callback(int code, Datum arg) * DROP DATABASE */ void -dropdb(const char *dbname, bool missing_ok) +dropdb(const char *dbname, bool missing_ok, bool force) { Oid db_id; bool db_istemplate; @@ -897,6 +897,13 @@ dropdb(const char *dbname, bool missing_ok) } /* + * Attempt to terminate all existing connections to the target database if + * the user has requested to do so. + */ + if (force) + TerminateOtherDBBackends(db_id); + + /* * Check for other backends in the target database. (Because we hold the * database lock, no new ones can start after this.) * @@ -1003,6 +1010,30 @@ dropdb(const char *dbname, bool missing_ok) ForceSyncCommit(); } +/* + * Process options and call dropdb function. + */ +void +DropDatabase(ParseState *pstate, DropdbStmt *stmt) +{ + bool force = false; + ListCell *lc; + + foreach(lc, stmt->options) + { + DefElem *opt = (DefElem *) lfirst(lc); + + if (strcmp(opt->defname, "force") == 0) + force = true; + else + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("unrecognized DROP DATABASE option \"%s\"", opt->defname), + parser_errposition(pstate, opt->location))); + } + + dropdb(stmt->dbname, stmt->missing_ok, force); +} /* * Rename database diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 3432bb9..2f267e4 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -3868,6 +3868,7 @@ _copyDropdbStmt(const DropdbStmt *from) COPY_STRING_FIELD(dbname); COPY_SCALAR_FIELD(missing_ok); + COPY_NODE_FIELD(options); return newnode; } diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 18cb014..da0e1d1 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -1676,6 +1676,7 @@ _equalDropdbStmt(const DropdbStmt *a, const DropdbStmt *b) { COMPARE_STRING_FIELD(dbname); COMPARE_SCALAR_FIELD(missing_ok); + COMPARE_NODE_FIELD(options); return true; } diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 3f67aaf..2f7bd66 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -310,6 +310,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); %type vac_analyze_option_elem %type vac_analyze_option_list %type vac_analyze_option_arg +%type drop_option %type opt_or_replace opt_grant_grant_option opt_grant_admin_option opt_nowait opt_if_exists opt_with_data @@ -406,6 +407,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); TriggerTransitions TriggerReferencing publication_name_list vacuum_relation_list opt_vacuum_relation_list + drop_option_list %type group_by_list %type group_by_item empty_grouping_set rollup_clause cube_clause @@ -10213,7 +10215,7 @@ AlterDatabaseSetStmt: /***************************************************************************** * - * DROP DATABASE [ IF EXISTS ] + * DROP DATABASE [ IF EXISTS ] dbname [ [ WITH ] ( options ) ] * * This is implicitly CASCADE, no need for drop behavior *****************************************************************************/ @@ -10223,6 +10225,7 @@ DropdbStmt: DROP DATABASE database_name DropdbStmt *n = makeNode(DropdbStmt); n->dbname = $3; n->missing_ok = false; + n->options = NULL; $$ = (Node *)n; } | DROP DATABASE IF_P EXISTS database_name @@ -10230,10 +10233,48 @@ DropdbStmt: DROP DATABASE database_name DropdbStmt *n = makeNode(DropdbStmt); n->dbname = $5; n->missing_ok = true; + n->options = NULL; $$ = (Node *)n; } + | DROP DATABASE database_name opt_with '(' drop_option_list ')' + { + DropdbStmt *n = makeNode(DropdbStmt); + n->dbname = $3; + n->missing_ok = false; + n->options = $6; + $$ = (Node *)n; + } + | DROP DATABASE IF_P EXISTS database_name opt_with '(' drop_option_list ')' + { + DropdbStmt *n = makeNode(DropdbStmt); + n->dbname = $5; + n->missing_ok = true; + n->options = $8; + $$ = (Node *)n; + } + ; + +drop_option_list: + drop_option + { + $$ = list_make1((Node *) $1); + } + | drop_option_list ',' drop_option + { + $$ = lappend($1, (Node *) $3); + } ; +/* + * Currently only the FORCE option is supported, but the syntax is designed + * to be extensible so that we can add more options in the future if required. + */ +drop_option: + FORCE + { + $$ = makeDefElem("force", NULL, @1); + } + ; /***************************************************************************** * diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index 3da5307..54bc81f 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -52,6 +52,8 @@ #include "access/xact.h" #include "access/xlog.h" #include "catalog/catalog.h" +#include "catalog/pg_authid.h" +#include "commands/dbcommands.h" #include "miscadmin.h" #include "pgstat.h" #include "storage/proc.h" @@ -2971,6 +2973,115 @@ CountOtherDBBackends(Oid databaseId, int *nbackends, int *nprepared) } /* + * Terminate existing connections to the specified database. This routine + * is used by the DROP DATABASE command when user has asked to forcefully + * drop the database. + * + * The current backend is always ignored; it is caller's responsibility to + * check whether the current backend uses the given DB, if it's important. + * + * It doesn't allow to terminate the connections even if there is a one + * backend with the prepared transaction in the target database. + */ +void +TerminateOtherDBBackends(Oid databaseId) +{ + ProcArrayStruct *arrayP = procArray; + List *pids = NIL; + int nprepared = 0; + int i; + + LWLockAcquire(ProcArrayLock, LW_SHARED); + + for (i = 0; i < procArray->numProcs; i++) + { + int pgprocno = arrayP->pgprocnos[i]; + PGPROC *proc = &allProcs[pgprocno]; + + if (proc->databaseId != databaseId) + continue; + if (proc == MyProc) + continue; + + if (proc->pid != 0) + pids = lappend_int(pids, proc->pid); + else + nprepared++; + } + + LWLockRelease(ProcArrayLock); + + if (nprepared > 0) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_IN_USE), + errmsg("database \"%s\" is being used by prepared transaction", + get_database_name(databaseId)), + errdetail_plural("There is %d prepared transaction using the database.", + "There are %d prepared transactions using the database.", + nprepared, + nprepared))); + + if (pids) + { + ListCell *lc; + + /* + * Check whether we have the necessary rights to terminate other + * sessions. We don't terminate any session untill we ensure that we + * have rights on all the sessions to be terminated. These checks are + * the same as we do in pg_terminate_backend. + * + * In this case we don't raise some warnings - like "PID %d is not a + * PostgreSQL server process", because for us already finished + * session is not a problem. + */ + foreach (lc, pids) + { + int pid = lfirst_int(lc); + PGPROC *proc = BackendPidGetProc(pid); + + if (proc != NULL) + { + /* Only allow superusers to signal superuser-owned backends. */ + if (superuser_arg(proc->roleId) && !superuser()) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + (errmsg("must be a superuser to terminate superuser process")))); + + /* Users can signal backends they have role membership in. */ + if (!has_privs_of_role(GetUserId(), proc->roleId) && + !has_privs_of_role(GetUserId(), DEFAULT_ROLE_SIGNAL_BACKENDID)) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + (errmsg("must be a member of the role whose process is being terminated or member of pg_signal_backend")))); + } + } + + /* + * There's a race condition here: once we release the ProcArrayLock, + * it's possible for the session to exit before we issue kill. That + * race condition possibility seems too unlikely to worry about. See + * pg_signal_backend. + */ + foreach (lc, pids) + { + int pid = lfirst_int(lc); + PGPROC *proc = BackendPidGetProc(pid); + + if (proc != NULL) + { + /* If we have setsid(), signal the backend's whole process group */ +#ifdef HAVE_SETSID + (void) kill(-pid, SIGTERM); +#else + (void) kill(pid, SIGTERM); +#endif + } + } + } +} + +/* * ProcArraySetReplicationSlotXmin * * Install limits to future computations of the xmin horizon to prevent vacuum diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index e984545..6d85304 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -596,13 +596,9 @@ standard_ProcessUtility(PlannedStmt *pstmt, break; case T_DropdbStmt: - { - DropdbStmt *stmt = (DropdbStmt *) parsetree; - - /* no event triggers for global objects */ - PreventInTransactionBlock(isTopLevel, "DROP DATABASE"); - dropdb(stmt->dbname, stmt->missing_ok); - } + /* no event triggers for global objects */ + PreventInTransactionBlock(isTopLevel, "DROP DATABASE"); + DropDatabase(pstate, (DropdbStmt *) parsetree); break; /* Query-level asynchronous notification */ diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 2b1e3cd..a949434 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -2844,6 +2844,10 @@ psql_completion(const char *text, int start, int end) COMPLETE_WITH_FUNCTION_ARG(prev2_wd); else if (Matches("DROP", "FOREIGN")) COMPLETE_WITH("DATA WRAPPER", "TABLE"); + else if (Matches("DROP", "DATABASE", MatchAny)) + COMPLETE_WITH("WITH ("); + else if (HeadMatches("DROP", "DATABASE") && (ends_with(prev_wd, '(') )) + COMPLETE_WITH("FORCE"); /* DROP INDEX */ else if (Matches("DROP", "INDEX")) diff --git a/src/include/commands/dbcommands.h b/src/include/commands/dbcommands.h index 154c815..d1e91a2 100644 --- a/src/include/commands/dbcommands.h +++ b/src/include/commands/dbcommands.h @@ -20,7 +20,8 @@ #include "nodes/parsenodes.h" extern Oid createdb(ParseState *pstate, const CreatedbStmt *stmt); -extern void dropdb(const char *dbname, bool missing_ok); +extern void dropdb(const char *dbname, bool missing_ok, bool force); +extern void DropDatabase(ParseState *pstate, DropdbStmt *stmt); extern ObjectAddress RenameDatabase(const char *oldname, const char *newname); extern Oid AlterDatabase(ParseState *pstate, AlterDatabaseStmt *stmt, bool isTopLevel); extern Oid AlterDatabaseSet(AlterDatabaseSetStmt *stmt); diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index d93a79a..ff626cb 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -3145,6 +3145,7 @@ typedef struct DropdbStmt NodeTag type; char *dbname; /* database to drop */ bool missing_ok; /* skip error if db is missing? */ + List *options; /* currently only FORCE is supported */ } DropdbStmt; /* ---------------------- diff --git a/src/include/storage/procarray.h b/src/include/storage/procarray.h index da8b672..8f67b86 100644 --- a/src/include/storage/procarray.h +++ b/src/include/storage/procarray.h @@ -113,6 +113,7 @@ extern void CancelDBBackends(Oid databaseid, ProcSignalReason sigmode, bool conf extern int CountUserBackends(Oid roleid); extern bool CountOtherDBBackends(Oid databaseId, int *nbackends, int *nprepared); +extern void TerminateOtherDBBackends(Oid databaseId); extern void XidCacheRemoveRunningXids(TransactionId xid, int nxids, const TransactionId *xids, diff --git a/src/test/regress/expected/drop_if_exists.out b/src/test/regress/expected/drop_if_exists.out index 6a17467..eba0f3f 100644 --- a/src/test/regress/expected/drop_if_exists.out +++ b/src/test/regress/expected/drop_if_exists.out @@ -330,3 +330,14 @@ HINT: Specify the argument list to select the routine unambiguously. -- cleanup DROP PROCEDURE test_ambiguous_procname(int); DROP PROCEDURE test_ambiguous_procname(text); +-- +-- DROP DATABASE FORCE test of syntax (should not to raise syntax error) +-- +drop database not_exists (force); +ERROR: database "not_exists" does not exist +drop database not_exists with (force); +ERROR: database "not_exists" does not exist +drop database if exists not_exists (force); +NOTICE: database "not_exists" does not exist, skipping +drop database if exists not_exists with (force); +NOTICE: database "not_exists" does not exist, skipping diff --git a/src/test/regress/sql/drop_if_exists.sql b/src/test/regress/sql/drop_if_exists.sql index 8a791b1..03aebb0 100644 --- a/src/test/regress/sql/drop_if_exists.sql +++ b/src/test/regress/sql/drop_if_exists.sql @@ -295,3 +295,11 @@ DROP ROUTINE IF EXISTS test_ambiguous_procname; -- cleanup DROP PROCEDURE test_ambiguous_procname(int); DROP PROCEDURE test_ambiguous_procname(text); + +-- +-- DROP DATABASE FORCE test of syntax (should not to raise syntax error) +-- +drop database not_exists (force); +drop database not_exists with (force); +drop database if exists not_exists (force); +drop database if exists not_exists with (force); -- 1.8.3.1