út 24. 9. 2019 v 14:52 odesílatel Amit Kapila <amit.kapil...@gmail.com>
napsal:
> On Sat, Sep 21, 2019 at 10:09 PM Pavel Stehule <pavel.steh...@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 ..."
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)));
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
>
