st 25. 9. 2019 v 4:14 odesÃlatel Amit Kapila <amit.kapil...@gmail.com> napsal:
> On Tue, Sep 24, 2019 at 6:22 PM Amit Kapila <amit.kapil...@gmail.com> > wrote: > > 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. > > > > 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. Regards Pavel > > -- > With Regards, > Amit Kapila. > EnterpriseDB: http://www.enterprisedb.com >
