On Thu, Oct 3, 2019 at 11:18 PM vignesh C <vignes...@gmail.com> wrote: > > On Wed, Oct 2, 2019 at 10:21 PM Pavel Stehule <pavel.steh...@gmail.com> wrote: > > > > Thank you for careful review. I hope so all issues are out. > > > > > Thanks Pavel for fixing the comments. > Few comments: > The else part cannot be hit in DropDatabase function as gram.y expects FORCE. > + > + if (strcmp(opt->defname, "force") == 0) > + force = true; > + else > + ereport(ERROR, > + (errcode(ERRCODE_SYNTAX_ERROR), > + errmsg("unrecognized DROP DATABASE option \"%s\"", opt->defname), > + parser_errposition(pstate, opt->location))); > + } > + > > We should change gram.y to accept any keyword and throw error from > DropDatabase function. > + */ > +drop_option: FORCE > + { > + $$ = makeDefElem("force", NULL, @1); > + } > + ; > > "This will also fail" should be "This will fail" > + <para> > + This will fail, if current user has no permissions to terminate other > + connections. Required permissions are the same as with > + <literal>pg_terminate_backend</literal>, described > + in <xref linkend="functions-admin-signal"/>. > + > + This will also fail if we are not able to terminate connections or > + when there are active prepared transactions or active logical > replication > + slots. > + </para> > > Can we add few tests for this feature. > Couple of minor comments: DBNAME can be included - * DROP DATABASE [ IF EXISTS ] + * DROP DATABASE [ IF EXISTS ] [ [ WITH ] ( options ) ] can be - * DROP DATABASE [ IF EXISTS ] + * DROP DATABASE [ IF EXISTS ] DBNAME [ [ WITH ] ( options ) ]
Should we include dbname in the below also? +DROP DATABASE [ IF EXISTS ] <replaceable class="parameter">name</replaceable> [ [ WITH ] ( <replaceable class="parameter">option</replaceable> [, ...] ) ] + +<phrase>where <replaceable class="parameter">option</replaceable> can be one of:</phrase> Regards, Vignesh EnterpriseDB: http://www.enterprisedb.com