Re: dropdb --force

From: Ryan Lambert <ryan(at)rustprooflabs(dot)com>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Anthony Nowocien <anowocien(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Filip Rembiałkowski <filip(dot)rembialkowski(at)gmail(dot)com>
Subject: Re: dropdb --force
Date: 2019-09-18 02:53:01
Message-ID: CAN-V+g9mNdvMz2W2DVqvxQLus9u82QY9ysTdMHARVh8UZ-Ugvg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Pavel,
I took a quick look through the patch, I'll try to build and test it
tomorrow.

--- 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;

Why put FORCE as the single item in an options list? A bool var seems like
it would be more clear and consistent.

- * DROP DATABASE [ IF EXISTS ]
+ * DROP DATABASE [ ( FORCE ) ] [ IF EXISTS ]

Why is it `[ ( FORCE ) ]` instead of `[ FORCE ]`?
There are also places in the code that seem like extra () are around
FORCE. Like here:

+ appendPQExpBuffer(&sql, "DROP DATABASE %s%s%s;",
+ (force ? " (FORCE) " : ""),
+ (if_exists ? "IF EXISTS " : ""), fmtId(dbname));

And here:

+$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');
+

I'll try to build and test the patch tomorrow.

Thanks,

*Ryan Lambert*

>
>>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2019-09-18 02:57:47 Re: dropdb --force
Previous Message Michael Paquier 2019-09-18 02:10:31 Re: [HACKERS] CLUSTER command progress monitor