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

On Wed, Nov 13, 2019 at 8:05 PM Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
>
>
> st 13. 11. 2019 v 7:13 odesílatel Pavel Stehule <pavel.stehule@gmail.com> napsal:
>>
>>
>>
>> st 13. 11. 2019 v 7:12 odesílatel Amit Kapila <amit.kapila16@gmail.com> napsal:
>>>
>>> On Tue, Nov 12, 2019 at 11:17 AM Amit Kapila <amit.kapila16@gmail.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