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

From Pavel Stehule
Subject Re: dropdb --force
Date
Msg-id CAFj8pRDuoSn0j_p29oi+=r0SSM4HTbbHvrO3jYLHJxDiqXaWbA@mail.gmail.com
Whole thread Raw
In response to Re: dropdb --force  (Dilip Kumar <dilipbalaut@gmail.com>)
Responses Re: dropdb --force
Re: dropdb --force
List pgsql-hackers


pá 20. 9. 2019 v 7:58 odesílatel Dilip Kumar <dilipbalaut@gmail.com> napsal:
On Thu, Sep 19, 2019 at 12:14 AM Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
> Hi
>
> I am sending updated version - the changes against last patch are two. I use pg_terminate_backed for killing other terminates like Tom proposed. I am not sure if it is 100% practical - on second hand the necessary right to kill other sessions is  almost available - and consistency with pg_terminal_backend has sense. More - native implementation is significantly better then solution implemented outer. I fixed docs little bit - the timeout for logout (as reaction on SIGTERM is only 5 sec).
>

Few comments on the patch.

@@ -598,10 +598,20 @@ standard_ProcessUtility(PlannedStmt *pstmt,
  case T_DropdbStmt:
  {
  DropdbStmt *stmt = (DropdbStmt *) parsetree;
+ bool force = false;
+ ListCell   *lc;
+
+ foreach(lc, stmt->options)
+ {
+ DefElem    *item = (DefElem *) lfirst(lc);
+
+ if (strcmp(item->defname, "force") == 0)
+ force = true;
+ }

  /* no event triggers for global objects */
  PreventInTransactionBlock(isTopLevel, "DROP DATABASE");
- dropdb(stmt->dbname, stmt->missing_ok);
+ dropdb(stmt->dbname, stmt->missing_ok, force);

If you see the createdb, then options are processed inside the
createdb function but here we are processing outside
the function.  Wouldn't it be good to keep them simmilar and also it
will be expandable in case we add more options
in the future?


I though about it, but I think so current state is good enough. There are only few parameters - and I don't think significantly large number of new options.

When number of parameters of any functions is not too high, then I think so is better to have a function with classic list of parameters instead function with one parameter of some struct type.

If somebody try to use function dropdb from outside (extension), then he don't need to create new structure, new list, and simply call simple C API. I prefer API of simple types against structures and nodes there. Just I think so it's more practical of somebody try to use this function from other places than from parser.
 

initPQExpBuffer(&sql);

- 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,10 @@ main(int argc, char *argv[])
    host, port, username, prompt_password,
    progname, echo);
+ appendPQExpBuffer(&sql, "DROP DATABASE %s%s%s;",
+   (force ? " (FORCE) " : ""),
+   (if_exists ? "IF EXISTS " : ""), fmtId(dbname));
+

I did not understand why you have moved this code after
connectMaintenanceDatabase function?  I don't see any problem but in
earlier code
initPQExpBuffer and appendPQExpBuffer were together now you have moved
appendPQExpBuffer after the connectMaintenanceDatabase so if there is
no reason to do that then better to keep it how it was earlier.


I am not a author of this change, but it is not necessary and I returned original order


-extern bool CountOtherDBBackends(Oid databaseId,
- int *nbackends, int *nprepared);
+extern bool CountOtherDBBackends(Oid databaseId, int *nbackends, int
*nprepared);

Unrelated change

fixed

Thank you for check. I am sending updated patch

Regards

Pavel



--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
Attachment

pgsql-hackers by date:

Previous
From: Thunder
Date:
Subject: PATCH: standby crashed when replay block which truncated in standbybut failed to truncate in master node
Next
From: Pavel Stehule
Date:
Subject: Re: dropdb --force