Add force option to dropdb

Started by salah jubehalmost 12 years ago12 messages
#1salah jubeh
s_jubeh@yahoo.com

Hello,

I think adding a force option to dropdb is usefull for many purposes, mainly; scripting. For example, in a test enviroment, I restore a daily backup of production databases. The newly created databases are used as templates for development and also for reading, thus replacing them is completly safe.

I uses dropdb and pg_terminate_backend  for this purpose, since some users might forget to close connections.
I have had a look on dropdb.c and I have amended the code to also terminate client connections using pg_terminate_backend

So the functionallity of the dropdb if force option is used is as follows:
1. Terminate client connections using pg_terminate_backend
2. Drop the database

For the sake of completeness:
1. I think also, I need also to temporary disallow conecting to the database, is that right?
2. Is there other factors can hinder dropping database?

3. Should I write two patches one for pg_version>=9.2 and one for pg_version<9.2 

Regards

#2Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: salah jubeh (#1)
Re: Add force option to dropdb

salah jubeh wrote:

For the sake of completeness:
1. I think also, I need also to temporary disallow conecting to the database, is that right?
2. Is there other factors can hinder dropping database?

If the user owns objects, that will prevent this from working also. I
have the feeling that adding DROP OWNED BY and/or REASSIGNED OWNED BY
calls to this utility would be a bit excessive, but who knows.

3. Should I write two patches one for pg_version>=9.2 and one for pg_version<9.2�

No point -- nothing gets applied to branches older than current
development anyway.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3salah jubeh
s_jubeh@yahoo.com
In reply to: Alvaro Herrera (#2)
1 attachment(s)
Re: Add force option to dropdb

If the user owns objects, that will prevent this from working also.  I
have the feeling that adding DROP OWNED BY and/or REASSIGNED OWNED BY
calls to this utility would be a bit excessive, but who knows.

Please find attached the first attempt to drop drop the client connections.

I have added an option -k, --kill instead of force since killing client connection does not guarantee -drop force-. 

Regards

On Tuesday, January 14, 2014 8:06 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:

salah jubeh wrote:

For the sake of completeness:
1. I think also, I need also to temporary disallow conecting to the database, is that right?
2. Is there other factors can hinder dropping database?

If the user owns objects, that will prevent this from working also.  I
have the feeling that adding DROP OWNED BY and/or REASSIGNED OWNED BY
calls to this utility would be a bit excessive, but who knows.

3. Should I write two patches one for pg_version>=9.2 and one for pg_version<9.2 

No point -- nothing gets applied to branches older than current
development anyway.

--
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachments:

dropdb.patchtext/x-patch; name=dropdb.patchDownload
--- dropdb_orig.c	2014-01-16 15:21:51.059243617 +0100
+++ dropdb.c	2014-01-16 16:38:57.419414200 +0100
@@ -32,6 +32,7 @@
 		{"echo", no_argument, NULL, 'e'},
 		{"interactive", no_argument, NULL, 'i'},
 		{"if-exists", no_argument, &if_exists, 1},
+		{"kill", no_argument, NULL, 'k'},
 		{"maintenance-db", required_argument, NULL, 2},
 		{NULL, 0, NULL, 0}
 	};
@@ -48,6 +49,8 @@
 	enum trivalue prompt_password = TRI_DEFAULT;
 	bool		echo = false;
 	bool		interactive = false;
+	bool		kill = false;
+	char *database_conn_limit = "-1";
 
 	PQExpBufferData sql;
 
@@ -59,7 +62,7 @@
 
 	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:wWeik", long_options, &optindex)) != -1)
 	{
 		switch (c)
 		{
@@ -84,6 +87,9 @@
 			case 'i':
 				interactive = true;
 				break;
+			case 'k':
+				kill = true;
+				break;
 			case 0:
 				/* this covers the long options */
 				break;
@@ -121,8 +127,6 @@
 
 	initPQExpBuffer(&sql);
 
-	appendPQExpBuffer(&sql, "DROP DATABASE %s%s;\n",
-					  (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)
@@ -131,11 +135,64 @@
 	conn = connectMaintenanceDatabase(maintenance_db,
 							host, port, username, prompt_password, progname);
 
+	/*disallow database connections and terminate client connections*/
+	if (kill) 
+	{
+		appendPQExpBuffer(&sql, "SELECT datconnlimit FROM pg_database WHERE datname= '%s';",fmtId(dbname));
+		result = executeQuery(conn, sql.data, progname, echo);
+		// get the datconnĺimit to do cleanup in case of dropdb fail
+		if (PQntuples(result) == 1)
+		{
+			database_conn_limit = PQgetvalue(result, 0, 0);
+		} else 
+		{
+			fprintf(stderr, _("%s: database removal failed: %s\n"),
+			progname, dbname);
+			PQclear(result);
+			PQfinish(conn);
+			exit(1);
+		}
+		PQclear(result);
+		resetPQExpBuffer(&sql);
+		//new connections are not allowed 
+		appendPQExpBuffer(&sql, "UPDATE pg_database SET datconnlimit = 0 WHERE datname= '%s';\n",fmtId(dbname));
+		if (echo)
+			printf("%s", sql.data);
+		result = PQexec(conn, sql.data);
+		if (PQresultStatus(result) != PGRES_COMMAND_OK)
+		{
+			fprintf(stderr, _("%s: database removal failed: %s\n"),
+			progname, dbname);
+			PQclear(result);
+			PQfinish(conn);
+			exit(1);
+		}
+		PQclear(result);
+		resetPQExpBuffer(&sql);
+		// terminate client connections
+		appendPQExpBuffer(&sql, "SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname ='%s';",fmtId(dbname));
+		result = executeQuery(conn, sql.data, progname, echo);
+		PQclear(result);
+		resetPQExpBuffer(&sql);
+	}
+
+	appendPQExpBuffer(&sql, "DROP DATABASE %s%s;\n",
+					  (if_exists ? "IF EXISTS " : ""), fmtId(dbname));
+
 	if (echo)
 		printf("%s", sql.data);
 	result = PQexec(conn, sql.data);
 	if (PQresultStatus(result) != PGRES_COMMAND_OK)
 	{
+		//cleanup: reset datconnlimit	
+		if (kill)
+		{
+			resetPQExpBuffer(&sql);
+			appendPQExpBuffer(&sql, "UPDATE pg_database SET datconnlimit = %s WHERE datname= '%s';",database_conn_limit,fmtId(dbname));
+			if (echo)
+				printf("%s", sql.data);
+			result = PQexec(conn, sql.data);
+		}
 		fprintf(stderr, _("%s: database removal failed: %s"),
 				progname, PQerrorMessage(conn));
 		PQfinish(conn);
@@ -159,6 +216,7 @@
 	printf(_("  -i, --interactive         prompt before deleting anything\n"));
 	printf(_("  -V, --version             output version information, then exit\n"));
 	printf(_("  --if-exists               don't report error if database doesn't exist\n"));
+	printf(_("  -k, --kill                kill client connections\n"));
 	printf(_("  -?, --help                show this help, then exit\n"));
 	printf(_("\nConnection options:\n"));
 	printf(_("  -h, --host=HOSTNAME       database server host or socket directory\n"));
#4Sawada Masahiko
sawada.mshk@gmail.com
In reply to: salah jubeh (#3)
Re: Add force option to dropdb

On 2014年1月17日 0:56, salah jubeh <s_jubeh@yahoo.com> wrote:

If the user owns objects, that will prevent this from working also. I
have the feeling that adding DROP OWNED BY and/or REASSIGNED OWNED BY
calls to this utility would be a bit excessive, but who knows.

Please find attached the first attempt to drop drop the client connections.
I have added an option -k, --kill instead of force since killing client
connection does not guarantee -drop force-.
Regards

On Tuesday, January 14, 2014 8:06 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
salah jubeh wrote:

For the sake of completeness:
1. I think also, I need also to temporary disallow conecting to the
database, is that right?
2. Is there other factors can hinder dropping database?

If the user owns objects, that will prevent this from working also. I
have the feeling that adding DROP OWNED BY and/or REASSIGNED OWNED BY
calls to this utility would be a bit excessive, but who knows.

3. Should I write two patches one for pg_version>=9.2 and one for
pg_version<9.2

No point -- nothing gets applied to branches older than current
development anyway.

Thank you for the patch.
And sorry for delay in reviewing.

I started to look this patch, So the following is first review comment.

- This patch is not patched to master branch
I tried to patch this patch file to master branch, but I got following error.
$ cd postgresql
$ patch -d. -p1 < ../dropdb.patch
can't find fiel to patch at input line 3
Perhaps you used the wrong -p or --strip option?
the text leading up to this was:
------------------------------
|--- dropdb_org.c 2014-01-16
|+++ dropdb.c 2014-01-16
------------------------------

There is not dropdb_org.c. I think that you made mistake when the
patch is created.

- This patch is not according the coding rule
For example, line 71 of the patch:
//new connections are not allowed
It should be:
/* new connections are not allowed */
(Comment blocks that need specific line breaks should be formatted as
block comments, where the comment starts as /*------.)
Please refer to coding rule.
<http://wiki.postgresql.org/wiki/Developer_FAQ#What.27s_the_formatting_style_used_in_PostgreSQL_source_code.3F&gt;

Regards,

-------
Sawada Masahiko

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5salah jubeh
s_jubeh@yahoo.com
In reply to: Sawada Masahiko (#4)
1 attachment(s)
Re: Add force option to dropdb

Hello Sawada,

- This patch is not patched to master branch

Sorry, My mistake

//new connections are not allowed

Corrected.

I hope now the patch in better state, if somthing left, I will be glad to fix it

Regards

On Tuesday, January 28, 2014 4:17 AM, Sawada Masahiko <sawada.mshk@gmail.com> wrote:

On 2014年1月17日 0:56, salah jubeh <s_jubeh@yahoo.com> wrote:

If the user owns objects, that will prevent this from working also.  I
have the feeling that adding DROP OWNED BY and/or REASSIGNED OWNED BY
calls to this utility would be a bit excessive, but who knows.

Please find attached the first attempt to drop drop the client connections.
I have added an option -k, --kill instead of force since killing client
connection does not guarantee -drop force-.
Regards

On Tuesday, January 14, 2014 8:06 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
salah jubeh wrote:

For the sake of completeness:
1. I think also, I need also to temporary disallow conecting to the
database, is that right?
2. Is there other factors can hinder dropping database?

If the user owns objects, that will prevent this from working also.  I
have the feeling that adding DROP OWNED BY and/or REASSIGNED OWNED BY
calls to this utility would be a bit excessive, but who knows.

3. Should I write two patches one for pg_version>=9.2 and one for
pg_version<9.2

No point -- nothing gets applied to branches older than current
development anyway.

Thank you for the patch.
And sorry for delay in reviewing.

I started to look this patch, So the following is first review comment.

- This patch is not patched to master branch
I tried to patch this patch file to master branch, but I got following error.
$ cd postgresql
$ patch -d. -p1 < ../dropdb.patch
can't find fiel to patch at input line 3
Perhaps you used the wrong -p or --strip option?
the text leading up to this was:
------------------------------
|--- dropdb_org.c 2014-01-16
|+++ dropdb.c 2014-01-16
------------------------------

There is not dropdb_org.c. I think  that you made mistake when the
patch is created.

- This patch is not according the coding rule
For example, line 71 of the patch:
//new connections are not allowed
It should be:
/* new connections are not allowed */
(Comment blocks that need specific line breaks should be formatted as
block comments, where the comment starts as /*------.)
Please refer to coding rule.
<http://wiki.postgresql.org/wiki/Developer_FAQ#What.27s_the_formatting_style_used_in_PostgreSQL_source_code.3F&gt;

Regards,

-------
Sawada Masahiko

Attachments:

dropdb.patchtext/x-patch; name=dropdb.patchDownload
diff --git a/src/bin/scripts/dropdb.c b/src/bin/scripts/dropdb.c
index fa6ea3e..755abf4 100644
--- a/src/bin/scripts/dropdb.c
+++ b/src/bin/scripts/dropdb.c
@@ -32,6 +32,7 @@ main(int argc, char *argv[])
 		{"echo", no_argument, NULL, 'e'},
 		{"interactive", no_argument, NULL, 'i'},
 		{"if-exists", no_argument, &if_exists, 1},
+		{"kill", no_argument, NULL, 'k'},
 		{"maintenance-db", required_argument, NULL, 2},
 		{NULL, 0, NULL, 0}
 	};
@@ -48,6 +49,8 @@ main(int argc, char *argv[])
 	enum trivalue prompt_password = TRI_DEFAULT;
 	bool		echo = false;
 	bool		interactive = false;
+	bool		kill = false;
+	char *database_conn_limit = "-1";
 
 	PQExpBufferData sql;
 
@@ -59,7 +62,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:wWeik", long_options, &optindex)) != -1)
 	{
 		switch (c)
 		{
@@ -84,6 +87,9 @@ main(int argc, char *argv[])
 			case 'i':
 				interactive = true;
 				break;
+			case 'k':
+				kill = true;
+				break;
 			case 0:
 				/* this covers the long options */
 				break;
@@ -121,8 +127,6 @@ main(int argc, char *argv[])
 
 	initPQExpBuffer(&sql);
 
-	appendPQExpBuffer(&sql, "DROP DATABASE %s%s;\n",
-					  (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)
@@ -131,11 +135,64 @@ main(int argc, char *argv[])
 	conn = connectMaintenanceDatabase(maintenance_db,
 							host, port, username, prompt_password, progname);
 
+	/* Disallow database connections and terminate client connections */
+	if (kill) 
+	{
+		appendPQExpBuffer(&sql, "SELECT datconnlimit FROM pg_database WHERE datname= '%s';",fmtId(dbname));
+		result = executeQuery(conn, sql.data, progname, echo);
+		/* Get the datconnĺimit to do a cleanup in case of dropdb fail */
+		if (PQntuples(result) == 1)
+		{
+			database_conn_limit = PQgetvalue(result, 0, 0);
+		} else 
+		{
+			fprintf(stderr, _("%s: database removal failed: %s\n"),
+			progname, dbname);
+			PQclear(result);
+			PQfinish(conn);
+			exit(1);
+		}
+		PQclear(result);
+		resetPQExpBuffer(&sql);
+		/* New connections are not allowed */
+		appendPQExpBuffer(&sql, "UPDATE pg_database SET datconnlimit = 0 WHERE datname= '%s';\n",fmtId(dbname));
+		if (echo)
+			printf("%s", sql.data);
+		result = PQexec(conn, sql.data);
+		if (PQresultStatus(result) != PGRES_COMMAND_OK)
+		{
+			fprintf(stderr, _("%s: database removal failed: %s\n"),
+			progname, dbname);
+			PQclear(result);
+			PQfinish(conn);
+			exit(1);
+		}
+		PQclear(result);
+		resetPQExpBuffer(&sql);
+		/* Terminate client connections */
+		appendPQExpBuffer(&sql, "SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname ='%s';",fmtId(dbname));
+		result = executeQuery(conn, sql.data, progname, echo);
+		PQclear(result);
+		resetPQExpBuffer(&sql);
+	}
+
+	appendPQExpBuffer(&sql, "DROP DATABASE %s%s;\n",
+					  (if_exists ? "IF EXISTS " : ""), fmtId(dbname));
+
 	if (echo)
 		printf("%s", sql.data);
 	result = PQexec(conn, sql.data);
 	if (PQresultStatus(result) != PGRES_COMMAND_OK)
 	{
+		/* Cleanup: reset the datconnlimit value to the original value */	
+		if (kill)
+		{
+			resetPQExpBuffer(&sql);
+			appendPQExpBuffer(&sql, "UPDATE pg_database SET datconnlimit = %s WHERE datname= '%s';",database_conn_limit,fmtId(dbname));
+			if (echo)
+				printf("%s", sql.data);
+			result = PQexec(conn, sql.data);
+		}
 		fprintf(stderr, _("%s: database removal failed: %s"),
 				progname, PQerrorMessage(conn));
 		PQfinish(conn);
@@ -159,6 +216,7 @@ help(const char *progname)
 	printf(_("  -i, --interactive         prompt before deleting anything\n"));
 	printf(_("  -V, --version             output version information, then exit\n"));
 	printf(_("  --if-exists               don't report error if database doesn't exist\n"));
+	printf(_("  -k, --kill                kill client connections\n"));
 	printf(_("  -?, --help                show this help, then exit\n"));
 	printf(_("\nConnection options:\n"));
 	printf(_("  -h, --host=HOSTNAME       database server host or socket directory\n"));
#6Robert Haas
robertmhaas@gmail.com
In reply to: salah jubeh (#5)
Re: Add force option to dropdb

On Tue, Jan 28, 2014 at 9:01 AM, salah jubeh <s_jubeh@yahoo.com> wrote:

Hello Sawada,

- This patch is not patched to master branch

Sorry, My mistake

//new connections are not allowed

Corrected.

I hope now the patch in better state, if somthing left, I will be glad to
fix it
Regards

I'm not particularly in favor of implementing this as client-side
functionality, because then it's only available to people who use that
particular client. Simon Riggs proposed a server-side option to the
DROP DATABASE command some time ago, and I think that might be the way
to go.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7salah jubeh
s_jubeh@yahoo.com
In reply to: Robert Haas (#6)
Re: Add force option to dropdb

Hello Robert,

I'm not particularly in favor of implementing this as client-side
functionality, because then it's only available to people who use that
particular client.  Simon Riggs proposed a server-side option to the
DROP DATABASE command some time ago, and I think that might be the way
to go.

Could
you please direct me -if possible- to the thread. I think,implementing
it on the client side gives the user the some visibility and control. Initially, I wanted to force drop the
database, then I have changed it to kill connections. I think the
change in the client tool, is simple and covers the main reason for not
being able to drop a database. I think, killing client connection is one of the FAQs.

OTOH,
having an option like "DROP DATABASE [IF EXISTS, FORCE] database" is
more crisp. However, what does "force" mean?  many options exist such as killing the connections gently, waiting
for connections to terminate, killing connections immediately. Also,
as Alvaro Herrera mentioned, DROP OWNED BY and/or REASSIGNED OWNED BY
might hinder the force option -an example here would be nice-. So, for quick wins, I prefer the kill
option in the client side; but, for mature solution , certainly back-end is the way to proceed.

Regards

#8Sawada Masahiko
sawada.mshk@gmail.com
In reply to: salah jubeh (#5)
Re: Add force option to dropdb

On Tue, Jan 28, 2014 at 11:01 PM, salah jubeh <s_jubeh@yahoo.com> wrote:

Hello Sawada,

- This patch is not patched to master branch

Sorry, My mistake

//new connections are not allowed

Corrected.

I hope now the patch in better state, if somthing left, I will be glad to
fix it
Regards

On Tuesday, January 28, 2014 4:17 AM, Sawada Masahiko
<sawada.mshk@gmail.com> wrote:
On 2014年1月17日 0:56, salah jubeh <s_jubeh@yahoo.com> wrote:

If the user owns objects, that will prevent this from working also. I
have the feeling that adding DROP OWNED BY and/or REASSIGNED OWNED BY
calls to this utility would be a bit excessive, but who knows.

Please find attached the first attempt to drop drop the client
connections.
I have added an option -k, --kill instead of force since killing client
connection does not guarantee -drop force-.
Regards

On Tuesday, January 14, 2014 8:06 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
salah jubeh wrote:

For the sake of completeness:
1. I think also, I need also to temporary disallow conecting to the
database, is that right?
2. Is there other factors can hinder dropping database?

If the user owns objects, that will prevent this from working also. I
have the feeling that adding DROP OWNED BY and/or REASSIGNED OWNED BY
calls to this utility would be a bit excessive, but who knows.

3. Should I write two patches one for pg_version>=9.2 and one for
pg_version<9.2

No point -- nothing gets applied to branches older than current
development anyway.

Thank you for the patch.
And sorry for delay in reviewing.

I started to look this patch, So the following is first review comment.

- This patch is not patched to master branch
I tried to patch this patch file to master branch, but I got following
error.
$ cd postgresql
$ patch -d. -p1 < ../dropdb.patch
can't find fiel to patch at input line 3
Perhaps you used the wrong -p or --strip option?
the text leading up to this was:
------------------------------
|--- dropdb_org.c 2014-01-16
|+++ dropdb.c 2014-01-16
------------------------------

There is not dropdb_org.c. I think that you made mistake when the
patch is created.

- This patch is not according the coding rule
For example, line 71 of the patch:
//new connections are not allowed
It should be:
/* new connections are not allowed */
(Comment blocks that need specific line breaks should be formatted as
block comments, where the comment starts as /*------.)
Please refer to coding rule.
<http://wiki.postgresql.org/wiki/Developer_FAQ#What.27s_the_formatting_style_used_in_PostgreSQL_source_code.3F&gt;

Thank you for updating patch!

It did not works fine with following case.

---
$ createdb -U postgres hoge
$ psql -d hoge -U postgres
hoge=# create table test (col text);
hoge=# insert into test select repeat(chr(code),10000) from
generate_series(1,100000) code;

<Execute dropdb -k while the client is inserting many tuples into database>
$ dropdb -k hoge
2014-01-29 23:10:49 JST FATAL: terminating connection due to
administrator command
2014-01-29 23:10:49 JST STATEMENT: insert into test select
repeat(chr(code),10000) from generate_series(1,2000000) code;
2014-01-29 23:10:54 JST ERROR: database "hoge" is being accessed by other users
2014-01-29 23:10:54 JST DETAIL: There is 1 other session using the database.
2014-01-29 23:10:54 JST STATEMENT: DROP DATABASE hoge;

2014-01-29 23:10:54 JST ERROR: syntax error at or near ""hoge"" at character 41
2014-01-29 23:10:54 JST STATEMENT: UPDATE pg_database SET
datconnlimit = e "hoge" is being accessed by other users WHERE
datname= 'hoge';
dropdb: database removal failed: ERROR: syntax error at or near ""hoge""
LINE 1: UPDATE pg_database SET datconnlimit = e "hoge" is being acce...
^
hoge=# \l
List of databases
Name | Owner | Encoding | Collate | Ctype | Access privileges
-----------+----------+----------+---------+-------+-----------------------
hoge | postgres | UTF8 | C | C |
postgres | postgres | UTF8 | C | C |
template0 | postgres | UTF8 | C | C | =c/postgres +
| | | | | postgres=CTc/postgres
template1 | postgres | UTF8 | C | C | =c/postgres +
| | | | | postgres=CTc/postgres

hoge database is not dropped yet.
Is this the bug? or not?

Regards,

-------
Sawada Masahiko

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#9Robert Haas
robertmhaas@gmail.com
In reply to: salah jubeh (#7)
Re: Add force option to dropdb

On Wed, Jan 29, 2014 at 4:56 AM, salah jubeh <s_jubeh@yahoo.com> wrote:

I'm not particularly in favor of implementing this as client-side
functionality, because then it's only available to people who use that
particular client. Simon Riggs proposed a server-side option to the
DROP DATABASE command some time ago, and I think that might be the way
to go.

Could you please direct me -if possible- to the thread. I think,implementing
it on the client side gives the user the some visibility and control.
Initially, I wanted to force drop the database, then I have changed it to
kill connections. I think the change in the client tool, is simple and
covers the main reason for not being able to drop a database. I think,
killing client connection is one of the FAQs.

OTOH, having an option like "DROP DATABASE [IF EXISTS, FORCE] database" is
more crisp. However, what does "force" mean? many options exist such as
killing the connections gently, waiting for connections to terminate,
killing connections immediately. Also, as Alvaro Herrera mentioned, DROP
OWNED BY and/or REASSIGNED OWNED BY might hinder the force option -an
example here would be nice-. So, for quick wins, I prefer the kill option in
the client side; but, for mature solution , certainly back-end is the way to
proceed.

/messages/by-id/1296552979.1779.8622.camel@ebony

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#10salah jubeh
s_jubeh@yahoo.com
In reply to: Robert Haas (#9)
1 attachment(s)
Re: Add force option to dropdb

$ createdb -U postgres hoge
$ psql -d hoge -U postgres
hoge=# create table test (col text);
hoge=# insert into test select repeat(chr(code),10000) from
generate_series(1,100000) code;

<Execute dropdb -k while the client is inserting many tuples into database>
$ dropdb -k hoge
2014-01-29 23:10:49 JST FATAL:  terminating connection due to
administrator command
2014-01-29 23:10:49 JST STATEMENT:  insert into test select
repeat(chr(code),10000) from generate_series(1,2000000) code;
2014-01-29 23:10:54 JST ERROR:  database "hoge" is being accessed by other users
2014-01-29 23:10:54 JST DETAIL:  There is 1 other session using the database.
2014-01-29 23:10:54 JST STATEMENT:  DROP DATABASE hoge;

2014-01-29 23:10:54 JST ERROR:  syntax error at or near ""hoge"" at character 41
2014-01-29 23:10:54 JST STATEMENT:  UPDATE pg_database SET
datconnlimit = e "hoge" is being accessed by other users WHERE
datname= 'hoge';
dropdb: database removal failed: ERROR:  syntax error at or near ""hoge""
LINE 1: UPDATE pg_database SET datconnlimit = e "hoge" is being acce...

                                                ^

hoge=# \l
                            List of databases
  Name    |  Owner  | Encoding | Collate | Ctype |  Access privileges
-----------+----------+----------+---------+-------+-----------------------
hoge      | postgres | UTF8    | C      | C    |
postgres  | postgres | UTF8    | C      | C    |
template0 | postgres | UTF8    | C      | C    | =c/postgres          +
          |          |          |        |      | postgres=CTc/postgres
template1 | postgres | UTF8    | C      | C    | =c/postgres          +
          |          |          |        |      | postgres=CTc/postgres

hoge database is not dropped yet.
Is this the bug? or not?

 It is a bug, sorry for doubling your work. I have updated the patch. 

Regards

On Wednesday, January 29, 2014 8:50 PM, Robert Haas <robertmhaas@gmail.com> wrote:

On Wed, Jan 29, 2014 at 4:56 AM, salah jubeh <s_jubeh@yahoo.com> wrote:

I'm not particularly in favor of implementing this as client-side
functionality, because then it's only available to people who use that
particular client.  Simon Riggs proposed a server-side option to the
DROP DATABASE command some time ago, and I think that might be the way
to go.

Could you please direct me -if possible- to the thread. I think,implementing
it on the client side gives the user the some visibility and control.
Initially, I wanted to force drop the database, then I have changed it to
kill connections. I think the change in the client tool, is simple and
covers the main reason for not being able to drop a database. I think,
killing client connection is one of the FAQs.

OTOH, having an option like "DROP DATABASE [IF EXISTS, FORCE] database" is
more crisp. However, what does "force" mean?  many options exist such as
killing the connections gently, waiting for connections to terminate,
killing connections immediately. Also, as Alvaro Herrera mentioned, DROP
OWNED BY and/or REASSIGNED OWNED BY might hinder the force option -an
example here would be nice-. So, for quick wins, I prefer the kill option in
the client side; but, for mature solution , certainly back-end is the way to
proceed.

/messages/by-id/1296552979.1779.8622.camel@ebony

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Attachments:

dropdb.patchtext/x-patch; name=dropdb.patchDownload
diff --git a/src/bin/scripts/dropdb.c b/src/bin/scripts/dropdb.c
index fa6ea3e..9041caa 100644
--- a/src/bin/scripts/dropdb.c
+++ b/src/bin/scripts/dropdb.c
@@ -32,6 +32,7 @@ main(int argc, char *argv[])
 		{"echo", no_argument, NULL, 'e'},
 		{"interactive", no_argument, NULL, 'i'},
 		{"if-exists", no_argument, &if_exists, 1},
+		{"kill", no_argument, NULL, 'k'},
 		{"maintenance-db", required_argument, NULL, 2},
 		{NULL, 0, NULL, 0}
 	};
@@ -48,6 +49,8 @@ main(int argc, char *argv[])
 	enum trivalue prompt_password = TRI_DEFAULT;
 	bool		echo = false;
 	bool		interactive = false;
+	bool		kill = false;
+	char *database_conn_limit = "-1";
 
 	PQExpBufferData sql;
 
@@ -59,7 +62,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:wWeik", long_options, &optindex)) != -1)
 	{
 		switch (c)
 		{
@@ -84,6 +87,9 @@ main(int argc, char *argv[])
 			case 'i':
 				interactive = true;
 				break;
+			case 'k':
+				kill = true;
+				break;
 			case 0:
 				/* this covers the long options */
 				break;
@@ -121,8 +127,6 @@ main(int argc, char *argv[])
 
 	initPQExpBuffer(&sql);
 
-	appendPQExpBuffer(&sql, "DROP DATABASE %s%s;\n",
-					  (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)
@@ -131,6 +135,51 @@ main(int argc, char *argv[])
 	conn = connectMaintenanceDatabase(maintenance_db,
 							host, port, username, prompt_password, progname);
 
+	/* Disallow database connections and terminate client connections */
+	if (kill)
+	{
+		appendPQExpBuffer(&sql, "SELECT datconnlimit FROM pg_database WHERE datname='%s';",fmtId(dbname));
+		result = executeQuery(conn, sql.data, progname, echo);
+		/* Get the datconnĺimit to do a cleanup in case of dropdb fail */
+		if (PQntuples(result) == 1)
+		{
+			database_conn_limit = malloc (1 + strlen(PQgetvalue(result, 0, 0)));
+			strcpy (database_conn_limit, PQgetvalue(result, 0, 0));
+		} else
+		{
+			fprintf(stderr, _("%s: database removal failed: %s\n"),
+			progname, dbname);
+			PQclear(result);
+			PQfinish(conn);
+			exit(1);
+		}
+		PQclear(result);
+		resetPQExpBuffer(&sql);
+		/* New connections are not allowed */
+		appendPQExpBuffer(&sql, "UPDATE pg_database SET datconnlimit=0 WHERE datname='%s';\n",fmtId(dbname));
+		if (echo)
+			printf("%s", sql.data);
+		result = PQexec(conn, sql.data);
+		if (PQresultStatus(result) != PGRES_COMMAND_OK)
+		{
+			fprintf(stderr, _("%s: database removal failed: %s\n"),
+			progname, dbname);
+			PQclear(result);
+			PQfinish(conn);
+			exit(1);
+		}
+		PQclear(result);
+		resetPQExpBuffer(&sql);
+		/* Terminate client connections */
+		appendPQExpBuffer(&sql, "SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname='%s';",fmtId(dbname));
+		result = executeQuery(conn, sql.data, progname, echo);
+		PQclear(result);
+		resetPQExpBuffer(&sql);
+	}
+
+	appendPQExpBuffer(&sql, "DROP DATABASE %s%s;\n",
+					  (if_exists ? "IF EXISTS " : ""), fmtId(dbname));
+
 	if (echo)
 		printf("%s", sql.data);
 	result = PQexec(conn, sql.data);
@@ -138,6 +187,16 @@ main(int argc, char *argv[])
 	{
 		fprintf(stderr, _("%s: database removal failed: %s"),
 				progname, PQerrorMessage(conn));
+		/* Cleanup: reset the datconnlimit value to the original value */
+		if (kill)
+		{
+			resetPQExpBuffer(&sql);
+			appendPQExpBuffer(&sql, "UPDATE pg_database SET datconnlimit=%s WHERE datname='%s';\n",
+					database_conn_limit,fmtId(dbname));
+			if (echo)
+				printf("%s", sql.data);
+			result = PQexec(conn, sql.data);
+		}
 		PQfinish(conn);
 		exit(1);
 	}
@@ -159,6 +218,7 @@ help(const char *progname)
 	printf(_("  -i, --interactive         prompt before deleting anything\n"));
 	printf(_("  -V, --version             output version information, then exit\n"));
 	printf(_("  --if-exists               don't report error if database doesn't exist\n"));
+	printf(_("  -k, --kill                kill client connections\n"));
 	printf(_("  -?, --help                show this help, then exit\n"));
 	printf(_("\nConnection options:\n"));
 	printf(_("  -h, --host=HOSTNAME       database server host or socket directory\n"));
#11Robert Haas
robertmhaas@gmail.com
In reply to: salah jubeh (#10)
Re: Add force option to dropdb

On Fri, Jan 31, 2014 at 9:09 AM, salah jubeh <s_jubeh@yahoo.com> wrote:

$ createdb -U postgres hoge
$ psql -d hoge -U postgres
hoge=# create table test (col text);
hoge=# insert into test select repeat(chr(code),10000) from
generate_series(1,100000) code;

<Execute dropdb -k while the client is inserting many tuples into database>
$ dropdb -k hoge
2014-01-29 23:10:49 JST FATAL: terminating connection due to
administrator command
2014-01-29 23:10:49 JST STATEMENT: insert into test select
repeat(chr(code),10000) from generate_series(1,2000000) code;
2014-01-29 23:10:54 JST ERROR: database "hoge" is being accessed by other
users
2014-01-29 23:10:54 JST DETAIL: There is 1 other session using the
database.
2014-01-29 23:10:54 JST STATEMENT: DROP DATABASE hoge;

2014-01-29 23:10:54 JST ERROR: syntax error at or near ""hoge"" at
character 41
2014-01-29 23:10:54 JST STATEMENT: UPDATE pg_database SET
datconnlimit = e "hoge" is being accessed by other users WHERE
datname= 'hoge';
dropdb: database removal failed: ERROR: syntax error at or near ""hoge""
LINE 1: UPDATE pg_database SET datconnlimit = e "hoge" is being acce...

^

hoge=# \l
List of databases
Name | Owner | Encoding | Collate | Ctype | Access privileges
-----------+----------+----------+---------+-------+-----------------------
hoge | postgres | UTF8 | C | C |
postgres | postgres | UTF8 | C | C |
template0 | postgres | UTF8 | C | C | =c/postgres +
| | | | | postgres=CTc/postgres
template1 | postgres | UTF8 | C | C | =c/postgres +
| | | | | postgres=CTc/postgres

hoge database is not dropped yet.
Is this the bug? or not?

It is a bug, sorry for doubling your work. I have updated the patch.

In case it wasn't clear before, I think that a client-side hack like
this has zero chance of being acceptable to the community, and we
should mark this patch rejected. I'm not saying it couldn't be useful
to someone, but the scripts are intended to be thin wrappers around
the underlying database functionality, and I think this is straying
too far from that core mission.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#12salah jubeh
s_jubeh@yahoo.com
In reply to: Robert Haas (#11)
Re: Add force option to dropdb

Hello Robert,

but the scripts are intended to be thin wrappers around
the underlying database functionality, and I think this is straying
too far from that core mission.

I think, you have a  good point here.
Regards

On Friday, January 31, 2014 4:47 PM, Robert Haas <robertmhaas@gmail.com> wrote:

On Fri, Jan 31, 2014 at 9:09 AM, salah jubeh <s_jubeh@yahoo.com> wrote:

$ createdb -U postgres hoge
$ psql -d hoge -U postgres
hoge=# create table test (col text);
hoge=# insert into test select repeat(chr(code),10000) from
generate_series(1,100000) code;

<Execute dropdb -k while the client is inserting many tuples into database>
$ dropdb -k hoge
2014-01-29 23:10:49 JST FATAL:  terminating connection due to
administrator command
2014-01-29 23:10:49 JST STATEMENT:  insert into test select
repeat(chr(code),10000) from generate_series(1,2000000) code;
2014-01-29 23:10:54 JST ERROR:  database "hoge" is being accessed by other
users
2014-01-29 23:10:54 JST DETAIL:  There is 1 other session using the
database.
2014-01-29 23:10:54 JST STATEMENT:  DROP DATABASE hoge;

2014-01-29 23:10:54 JST ERROR:  syntax error at or near ""hoge"" at
character 41
2014-01-29 23:10:54 JST STATEMENT:  UPDATE pg_database SET
datconnlimit = e "hoge" is being accessed by other users WHERE
datname= 'hoge';
dropdb: database removal failed: ERROR:  syntax error at or near ""hoge""
LINE 1: UPDATE pg_database SET datconnlimit = e "hoge" is being acce...

                                                ^

hoge=# \l
                            List of databases
  Name    |  Owner  | Encoding | Collate | Ctype |  Access privileges
-----------+----------+----------+---------+-------+-----------------------
hoge      | postgres | UTF8    | C      | C    |
postgres  | postgres | UTF8    | C      | C    |
template0 | postgres | UTF8    | C      | C    | =c/postgres          +
          |          |          |        |      | postgres=CTc/postgres
template1 | postgres | UTF8    | C      | C    | =c/postgres          +
          |          |          |        |      | postgres=CTc/postgres

hoge database is not dropped yet.
Is this the bug? or not?

  It is a bug, sorry for doubling your work. I have updated the patch.

In case it wasn't clear before, I think that a client-side hack like
this has zero chance of being acceptable to the community, and we
should mark this patch rejected.  I'm not saying it couldn't be useful
to someone, but the scripts are intended to be thin wrappers around
the underlying database functionality, and I think this is straying
too far from that core mission.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company