Re: dropdb --force - Mailing list pgsql-hackers

st 18. 9. 2019 v 4:53 odesílatel Ryan Lambert <ryan@rustprooflabs.com> napsal:

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.

see discussion - it was one from Tom's objections. It is due possible future enhancing or modification. It can looks strange, because now there is only one option, but it allow very easy modifications. More it is consistent with lot of other pg commands.

- * 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:

It was discussed before. FORCE is not reserved keyword, so inside list is due safety against possible collisions.

+ 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