On Tue, Sep 24, 2019 at 6:22 PM Amit Kapila <amit.kapila16@gmail.com> wrote: > 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. > > 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? >
One more point I would like to add here is that I think it is worth considering to split this patch by keeping the changes in dropdb utility as a separate patch. Even though the code is not very much but I think it can be a separate patch atop the main patch which contains the core server changes.
I did it - last patch contains server side only. I expect so client side (very small patch) can be next.
