Re: dropdb --force - Mailing list pgsql-hackers
| From | Pavel Stehule |
|---|---|
| Subject | Re: dropdb --force |
| Date | |
| Msg-id | CAFj8pRAzOZOAKsXYSA68WqSuJ+pM2AgQmwhCRn-CCSCKm3+omQ@mail.gmail.com Whole thread Raw |
| In response to | Re: dropdb --force (Amit Kapila <amit.kapila16@gmail.com>) |
| Responses |
Re: dropdb --force
Re: dropdb --force |
| List | pgsql-hackers |
út 24. 9. 2019 v 14:52 odesílatel Amit Kapila <amit.kapila16@gmail.com> napsal:
On Sat, Sep 21, 2019 at 10:09 PM Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
> Thank you for check. I am sending updated patch
>
Alvaro has up thread suggested some alternative syntax [1] for this
patch, but I don't see any good argument to not go with what he has
proposed. In other words, why we should prefer the current syntax as
in the patch over what Alvaro has proposed?
IIUC, the current syntax implemented by the patch is:
Drop Database [(options)] [If Exists] name
Alvaro suggested using options at the end (and use optional keyword
WITH) based on what other Drop commands does. I see some merits to
that idea which are (a) if tomorrow we want to introduce new options
like CASCADE, RESTRICT then it will be better to have all the options
at the end as we have for other Drop commands, (b) It will resemble
more with Create Database syntax.
Now, I think the current syntax is also not bad and we already do
something like that for other commands like Vaccum where options are
provided before object_name, but I think in this case putting at the
end is more appealing unless there are some arguments against that.
Originally it was placed after name. Tom's objection was possibility to collision with some future standards and requirement to be implemented safe.
cite: "* I'm concerned that the proposed syntax is not future-proof.
FORCE is not a reserved word, and we surely don't want to make
it one; but just appending it to the end of the command without
any decoration seems like a recipe for problems if anybody wants
to add other options later. (Possible examples: RESTRICT/CASCADE,
or a user-defined timeout.) Maybe some parentheses would help?
Or possibly I'm being overly paranoid, but ..."
FORCE is not a reserved word, and we surely don't want to make
it one; but just appending it to the end of the command without
any decoration seems like a recipe for problems if anybody wants
to add other options later. (Possible examples: RESTRICT/CASCADE,
or a user-defined timeout.) Maybe some parentheses would help?
Or possibly I'm being overly paranoid, but ..."
When I use parenthesis, then current placement looks correct - and it is well known syntax already.
Alternative is DROP DATABASE [IF EXISTS] name [ CASCADE | RESTRICT ] [ WITH FORCE ]
but in this case WIDTH keyword should not be optional (If I need to solve Tom's note). Currently WITH keyword is optional every where, so I think so using syntax with required WIDTH keyword is not happy.
When I looks to other statement, then the most similar case is DROP INDEX CONCURRENTLY ... so most consistent syntax is DROP DATABASE FORCE ... or DROP DATABASE (FORCE, ..)
Optional syntax can be (similar to CREATE USER MAPPING - but it looks like too verbose
DROP DATABASE xxx OPTIONS (FORCE, ...)
It's easy to change syntax, and I'll do it - I have not strong preferences, but If wouldn't to increase Tom's paranoia, I think so current syntax is most common in pg, and well known.
What do you think about it?
One other minor comment:
+
+ This will also fail, if the connections do not terminate in 5 seconds.
+ </para>
Is there any implementation in the patch for the above note?
Yes, is there.
The force flag ensure sending SIGTERM to related clients. Nothing more. There are still check
-->if (CountOtherDBBackends(db_id, ¬herbackends, &npreparedxacts))
<--><-->ereport(ERROR,
<--><--><--><-->(errcode(ERRCODE_OBJECT_IN_USE),
<--><--><--><--> errmsg("database \"%s\" is being accessed by other users",
<--><--><--><--><--><-->dbname),
<--><--><--><--> errdetail_busy_db(notherbackends, npreparedxacts)));
<--><-->ereport(ERROR,
<--><--><--><-->(errcode(ERRCODE_OBJECT_IN_USE),
<--><--><--><--> errmsg("database \"%s\" is being accessed by other users",
<--><--><--><--><--><-->dbname),
<--><--><--><--> errdetail_busy_db(notherbackends, npreparedxacts)));
that can fails after 5 sec. Sending signal doesn't ensure nothing, so I am for no changes in these check.
Regards
Pavel
[1] - https://www.postgresql.org/message-id/20190903164633.GA16408%40alvherre.pgsql
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
pgsql-hackers by date: