Re: dropdb --force

On Thu, Oct 3, 2019 at 11:18 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Wed, Oct 2, 2019 at 10:21 PM Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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