Re: dropdb --force

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