Re: dropdb --force

On Wed, Nov 13, 2019 at 8:05 PM Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>
>
>
> st 13. 11. 2019 v 7:13 odesílatel Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> napsal:
>>
>>
>>
>> st 13. 11. 2019 v 7:12 odesílatel Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> napsal:
>>>
>>> On Tue, Nov 12, 2019 at 11:17 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>>> >
>>> > I am planning to commit this patch tomorrow unless I see more comments
>>> > or interest from someone else to review this.
>>> >
>>>
>>> Pushed. Pavel, feel free to submit dropdb utility-related patch if you want.
>>
>>
>> I hope I send this patch today. It's simply job.
>
>
> here it is. It's based on Filip Rembialkowski's patch if I remember correctly
>

Thanks for working on the patch.
Few minor comments:
+ 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>

The above documentation can be changed similar to drop_database.sgml:
<para>
Attempt to terminate all existing connections to the target database.
It doesn't terminate if prepared transactions, active logical replication
slots or subscriptions are present in the target database.
</para>
<para>
This will fail if the 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 we
are not able to terminate connections.
</para>

We can make the modification in the same location as earlier in the below case:
- 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,12 @@ main(int argc, char *argv[])
host, port, username, prompt_password,
progname, echo);

+ /* now, only FORCE option can be used, so usage is very simple */
+ appendPQExpBuffer(&sql, "DROP DATABASE %s%s%s;",
+ (if_exists ? "IF EXISTS " : ""),
+ fmtId(dbname),
+ force ? " WITH (FORCE)" : "");
+

We can slightly rephrase the below:
+ printf(_(" -f, --force force termination of
connected backends\n"));
can be changed to:
+ printf(_(" -f, --force terminate the existing
connections to the target database forcefully\n"));

We can slightly rephrase the below:
+ /* now, only FORCE option can be used, so usage is very simple */
+ appendPQExpBuffer(&sql, "DROP DATABASE %s%s%s;",
can be changed to:
+ /* Generate drop db command using the options specified */
+ appendPQExpBuffer(&sql, "DROP DATABASE %s%s%s;",

Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com