Re: dropdb --force

st 25. 9. 2019 v 6:38 odesílatel vignesh C <vignesh21(at)gmail(dot)com> napsal:

> On Sat, Sep 21, 2019 at 10:09 PM Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote:
> > fixed
> >
> > Thank you for check. I am sending updated patch
> >
> Thanks for fixing all the comments.
> Couple of suggestions:
> -DROP DATABASE [ IF EXISTS ] <replaceable
> class="parameter">name</replaceable>
> +DROP DATABASE [ ( <replaceable class="parameter">option</replaceable>
> [, ...] ) ] [ IF EXISTS ] <replaceable
> class="parameter">name</replaceable>
> +
> +<phrase>where <replaceable class="parameter">option</replaceable> can
> be one of:</phrase>
> +
> + FORCE
>
> +drop_option_list:
> + drop_option
> + {
> + $$ = list_make1((Node *) $1);
> + }
> + | drop_option_list ',' drop_option
> + {
> + $$ = lappend($1, (Node *) $3);
> + }
> + ;
> +
> +drop_option: FORCE
> + {
> + $$ = makeDefElem("force", NULL, @1);
> + }
> + ;
>
> Currently only force option is supported in: drop database (force) dbname
> Should we have a list or a single element for this?
> I'm not sure if we have any plans to add more option in this area.
>

If we open door for next possible syntax enhancing and it is motivation for
this syntax, then we should to use pattern for this situation.
It looks little bit strange, but I think so current code is very well
readable. I wrote comment, so only one flag is supported now, but
syntax allows add other flags. I don't think so using defelem directly
reduces significantly enough lines - just if we implement some
what looks like possible list, then we should to use lists inside.

> If possible we can add an error message like 'ERROR: unrecognized
> drop database option "force1"' if an invalid input is given.
> The above error message will be inline with error message of vacuum's
> error message whose syntax is similar to the current feature.
> We could throw the error from here:
> case T_DropdbStmt:
> {
> DropdbStmt *stmt = (DropdbStmt *) parsetree;
> + bool force = false;
> + ListCell *lc;
> +
> + foreach(lc, stmt->options)
> + {
> + DefElem *item = (DefElem *) lfirst(lc);
> +
> + if (strcmp(item->defname, "force") == 0)
> + force = true;
> + }
> Thoughts?
>

I moved this check to separate function DropDatabase with new check and
exception like you proposed.

Regards

Pavel

>
> Regards,
> Vignesh
> EnterpriseDB: http://www.enterprisedb.com
>