pá 20. 9. 2019 v 7:58 odesílatel Dilip Kumar <dilipbal...@gmail.com> napsal:
> On Thu, Sep 19, 2019 at 12:14 AM Pavel Stehule <pavel.steh...@gmail.com>
> wrote:
> >
> > Hi
> >
> > I am sending updated version - the changes against last patch are two. I
> use pg_terminate_backed for killing other terminates like Tom proposed. I
> am not sure if it is 100% practical - on second hand the necessary right to
> kill other sessions is almost available - and consistency with
> pg_terminal_backend has sense. More - native implementation is
> significantly better then solution implemented outer. I fixed docs little
> bit - the timeout for logout (as reaction on SIGTERM is only 5 sec).
> >
>
> Few comments on the patch.
>
> @@ -598,10 +598,20 @@ standard_ProcessUtility(PlannedStmt *pstmt,
> case T_DropdbStmt:
> {
> DropdbStmt *stmt = (DropdbStmt *) parsetree;
> + bool force = false;
> + ListCell *lc;
> +
> + foreach(lc, stmt->options)
> + {
> + DefElem *item = (DefElem *) lfirst(lc);
> +
> + if (strcmp(item->defname, "force") == 0)
> + force = true;
> + }
>
> /* no event triggers for global objects */
> PreventInTransactionBlock(isTopLevel, "DROP DATABASE");
> - dropdb(stmt->dbname, stmt->missing_ok);
> + dropdb(stmt->dbname, stmt->missing_ok, force);
>
> If you see the createdb, then options are processed inside the
> createdb function but here we are processing outside
> the function. Wouldn't it be good to keep them simmilar and also it
> will be expandable in case we add more options
> in the future?
>
>
I though about it, but I think so current state is good enough. There are
only few parameters - and I don't think significantly large number of new
options.
When number of parameters of any functions is not too high, then I think so
is better to have a function with classic list of parameters instead
function with one parameter of some struct type.
If somebody try to use function dropdb from outside (extension), then he
don't need to create new structure, new list, and simply call simple C API.
I prefer API of simple types against structures and nodes there. Just I
think so it's more practical of somebody try to use this function from
other places than from parser.
>
> initPQExpBuffer(&sql);
>
> - appendPQExpBuffer(&sql, "DROP DATABASE %s%s;",
> - (if_exists ? "IF EXISTS " : ""), fmtId(dbname));
> -
> /* Avoid trying to drop postgres db while we are connected to it. */
> if (maintenance_db == NULL && strcmp(dbname, "postgres") == 0)
> maintenance_db = "template1";
> @@ -134,6 +136,10 @@ main(int argc, char *argv[])
> host, port, username, prompt_password,
> progname, echo);
> + appendPQExpBuffer(&sql, "DROP DATABASE %s%s%s;",
> + (force ? " (FORCE) " : ""),
> + (if_exists ? "IF EXISTS " : ""), fmtId(dbname));
> +
>
> I did not understand why you have moved this code after
> connectMaintenanceDatabase function? I don't see any problem but in
> earlier code
> initPQExpBuffer and appendPQExpBuffer were together now you have moved
> appendPQExpBuffer after the connectMaintenanceDatabase so if there is
> no reason to do that then better to keep it how it was earlier.
>
>
I am not a author of this change, but it is not necessary and I returned
original order
> -extern bool CountOtherDBBackends(Oid databaseId,
> - int *nbackends, int *nprepared);
> +extern bool CountOtherDBBackends(Oid databaseId, int *nbackends, int
> *nprepared);
>
> Unrelated change
>
fixed
Thank you for check. I am sending updated patch
Regards
Pavel
> --
> Regards,
> Dilip Kumar
> EnterpriseDB: http://www.enterprisedb.com
>
diff --git a/doc/src/sgml/ref/drop_database.sgml b/doc/src/sgml/ref/drop_database.sgml
index 3ac06c984a..11a31899d2 100644
--- a/doc/src/sgml/ref/drop_database.sgml
+++ b/doc/src/sgml/ref/drop_database.sgml
@@ -21,7 +21,11 @@ PostgreSQL documentation
<refsynopsisdiv>
<synopsis>
-DROP DATABASE [ IF EXISTS ] <replaceable class="parameter">name</replaceable>
+DROP DATABASE [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] [ IF EXISTS ] <replaceable class="parameter">name</replaceable>
+
+<phrase>where <replaceable class="parameter">option</replaceable> can be one of:</phrase>
+
+ FORCE
</synopsis>
</refsynopsisdiv>
@@ -32,9 +36,11 @@ DROP DATABASE [ IF EXISTS ] <replaceable class="parameter">name</replaceable>
<command>DROP DATABASE</command> 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 <literal>postgres</literal> or any
- other database to issue this command.)
+ Also, it cannot be executed while you are connected to the target database.
+ (Connect to <literal>postgres</literal> or any other database to issue this
+ command.)
+ If anyone else is connected to the target database, this command will fail
+ - unless you use the <literal>FORCE</literal> option described below.
</para>
<para>
@@ -64,6 +70,24 @@ DROP DATABASE [ IF EXISTS ] <replaceable class="parameter">name</replaceable>
</para>
</listitem>
</varlistentry>
+
+ <varlistentry>
+ <term><literal>FORCE</literal></term>
+ <listitem>
+ <para>
+ Attempt to terminate all existing connections to the target database.
+ </para>
+ <para>
+ This will fail, if current user has no permissions to terminate other
+ connections. Required permissions are the same as with
+ <literal>pg_terminate_backend</literal>, described
+ in <xref linkend="functions-admin-signal"/>.
+
+ This will also fail, if the connections do not terminate in 5 seconds.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</refsect1>
diff --git a/doc/src/sgml/ref/dropdb.sgml b/doc/src/sgml/ref/dropdb.sgml
index 3fbdb33716..5d10ad1c92 100644
--- a/doc/src/sgml/ref/dropdb.sgml
+++ b/doc/src/sgml/ref/dropdb.sgml
@@ -86,6 +86,20 @@ PostgreSQL documentation
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>-f</option></term>
+ <term><option>--force</option></term>
+ <listitem>
+ <para>
+ Force termination of connected backends before removing the database.
+ </para>
+ <para>
+ This will add the <literal>FORCE</literal> option to the <literal>DROP
+ DATABASE</literal> command sent to the server.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>-V</option></term>
<term><option>--version</option></term>
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 95881a8550..faa5bdbf71 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -810,7 +810,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;
@@ -895,6 +895,9 @@ dropdb(const char *dbname, bool missing_ok)
nslots_active, nslots_active)));
}
+ 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.)
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 3432bb921d..2f267e4bb6 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 18cb014373..da0e1d139a 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 3f67aaf30e..f7bab61175 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 <defelt> vac_analyze_option_elem
%type <list> vac_analyze_option_list
%type <node> vac_analyze_option_arg
+%type <defelt> drop_option
%type <boolean> 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 opt_drop_option_list
%type <list> group_by_list
%type <node> group_by_item empty_grouping_set rollup_clause cube_clause
@@ -10213,27 +10215,56 @@ AlterDatabaseSetStmt:
/*****************************************************************************
*
- * DROP DATABASE [ IF EXISTS ]
+ * DROP DATABASE [ ( FORCE ) ] [ IF EXISTS ]
*
* This is implicitly CASCADE, no need for drop behavior
*****************************************************************************/
-DropdbStmt: DROP DATABASE database_name
+DropdbStmt: DROP DATABASE opt_drop_option_list database_name
{
DropdbStmt *n = makeNode(DropdbStmt);
- n->dbname = $3;
+ n->dbname = $4;
n->missing_ok = false;
+ n->options = $3;
$$ = (Node *)n;
}
- | DROP DATABASE IF_P EXISTS database_name
+ | DROP DATABASE opt_drop_option_list IF_P EXISTS database_name
{
DropdbStmt *n = makeNode(DropdbStmt);
- n->dbname = $5;
+ n->dbname = $6;
n->missing_ok = true;
+ n->options = $3;
$$ = (Node *)n;
}
;
+opt_drop_option_list:
+ '(' drop_option_list ')'
+ {
+ $$ = $2;
+ }
+ | /* EMPTY */
+ {
+ $$ = NIL;
+ }
+ ;
+
+drop_option_list:
+ drop_option
+ {
+ $$ = list_make1((Node *) $1);
+ }
+ | drop_option_list ',' drop_option
+ {
+ $$ = lappend($1, (Node *) $3);
+ }
+ ;
+
+drop_option: FORCE
+ {
+ $$ = makeDefElem("force", NULL, @1);
+ }
+ ;
/*****************************************************************************
*
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 8abcfdf841..4a0bd5b76a 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -52,6 +52,7 @@
#include "access/xact.h"
#include "access/xlog.h"
#include "catalog/catalog.h"
+#include "catalog/pg_authid.h"
#include "miscadmin.h"
#include "pgstat.h"
#include "storage/proc.h"
@@ -2970,6 +2971,92 @@ CountOtherDBBackends(Oid databaseId, int *nbackends, int *nprepared)
return true; /* timed out, still conflicts */
}
+/*
+ * Terminate other db connections. This routine is used by
+ * DROP DATABASE FORCE to eliminate all others clients from
+ * database.
+ */
+void
+TerminateOtherDBBackends(Oid databaseId)
+{
+ ProcArrayStruct *arrayP = procArray;
+ List *pids = NIL;
+ 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);
+ }
+
+ LWLockRelease(ProcArrayLock);
+
+ if (pids)
+ {
+ ListCell *lc;
+
+ /*
+ * Similar code to pg_terminate_backend, but we check rigts first
+ * here, and only when we have all necessary rights we start to
+ * terminate other clients. In this case we should not to raise
+ * some warnings - like "PID %d is not a PostgreSQL server process",
+ * because for this purpose - already finished session is not
+ * 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"))));
+ }
+ }
+
+ /* We know so we have all necessary rights now */
+ 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
+ }
+ }
+
+ /* sleep 100ms */
+ pg_usleep(100 * 1000L);
+ }
+}
+
/*
* ProcArraySetReplicationSlotXmin
*
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index c6faa6619d..d69e30972f 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -598,10 +598,20 @@ standard_ProcessUtility(PlannedStmt *pstmt,
case T_DropdbStmt:
{
DropdbStmt *stmt = (DropdbStmt *) parsetree;
+ bool force = false;
+ ListCell *lc;
+
+ foreach(lc, stmt->options)
+ {
+ DefElem *item = (DefElem *) lfirst(lc);
+
+ if (strcmp(item->defname, "force") == 0)
+ force = true;
+ }
/* no event triggers for global objects */
PreventInTransactionBlock(isTopLevel, "DROP DATABASE");
- dropdb(stmt->dbname, stmt->missing_ok);
+ dropdb(stmt->dbname, stmt->missing_ok, force);
}
break;
diff --git a/src/bin/scripts/dropdb.c b/src/bin/scripts/dropdb.c
index dacd8e5f1d..1bb80fda74 100644
--- a/src/bin/scripts/dropdb.c
+++ b/src/bin/scripts/dropdb.c
@@ -34,6 +34,7 @@ main(int argc, char *argv[])
{"interactive", no_argument, NULL, 'i'},
{"if-exists", no_argument, &if_exists, 1},
{"maintenance-db", required_argument, NULL, 2},
+ {"force", no_argument, NULL, 'f'},
{NULL, 0, NULL, 0}
};
@@ -49,6 +50,7 @@ main(int argc, char *argv[])
enum trivalue prompt_password = TRI_DEFAULT;
bool echo = false;
bool interactive = false;
+ bool force = false;
PQExpBufferData sql;
@@ -61,7 +63,7 @@ main(int argc, char *argv[])
handle_help_version_opts(argc, argv, "dropdb", help);
- while ((c = getopt_long(argc, argv, "h:p:U:wWei", long_options, &optindex)) != -1)
+ while ((c = getopt_long(argc, argv, "h:p:U:wWeif", long_options, &optindex)) != -1)
{
switch (c)
{
@@ -86,6 +88,9 @@ main(int argc, char *argv[])
case 'i':
interactive = true;
break;
+ case 'f':
+ force = true;
+ break;
case 0:
/* this covers the long options */
break;
@@ -123,7 +128,8 @@ main(int argc, char *argv[])
initPQExpBuffer(&sql);
- appendPQExpBuffer(&sql, "DROP DATABASE %s%s;",
+ appendPQExpBuffer(&sql, "DROP DATABASE %s%s%s;",
+ (force ? "(FORCE) " : ""),
(if_exists ? "IF EXISTS " : ""), fmtId(dbname));
/* Avoid trying to drop postgres db while we are connected to it. */
@@ -159,6 +165,7 @@ help(const char *progname)
printf(_("\nOptions:\n"));
printf(_(" -e, --echo show the commands being sent to the server\n"));
printf(_(" -i, --interactive prompt before deleting anything\n"));
+ printf(_(" -f, --force force termination of connected backends\n"));
printf(_(" -V, --version output version information, then exit\n"));
printf(_(" --if-exists don't report error if database doesn't exist\n"));
printf(_(" -?, --help show this help, then exit\n"));
diff --git a/src/bin/scripts/t/050_dropdb.pl b/src/bin/scripts/t/050_dropdb.pl
index 25aa54a4ae..1c54df2cb2 100644
--- a/src/bin/scripts/t/050_dropdb.pl
+++ b/src/bin/scripts/t/050_dropdb.pl
@@ -3,7 +3,7 @@ use warnings;
use PostgresNode;
use TestLib;
-use Test::More tests => 11;
+use Test::More tests => 13;
program_help_ok('dropdb');
program_version_ok('dropdb');
@@ -19,5 +19,11 @@ $node->issues_sql_like(
qr/statement: DROP DATABASE foobar1/,
'SQL DROP DATABASE run');
+$node->safe_psql('postgres', 'CREATE DATABASE foobar2');
+$node->issues_sql_like(
+ [ 'dropdb', '--force', 'foobar2' ],
+ qr/statement: DROP DATABASE \(FORCE\) foobar2/,
+ 'SQL DROP DATABASE (FORCE) run');
+
$node->command_fails([ 'dropdb', 'nonexistent' ],
'fails with nonexistent database');
diff --git a/src/include/commands/dbcommands.h b/src/include/commands/dbcommands.h
index 154c8157ee..66e0b904bf 100644
--- a/src/include/commands/dbcommands.h
+++ b/src/include/commands/dbcommands.h
@@ -20,7 +20,7 @@
#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 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 d93a79a554..ff626cbe61 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 da8b672096..8f67b860e7 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,
