Thread: Rejecting redundant options in Create Collation

  Hi,

Currently, it's not an error for CREATE COLLATION to be invoked
with options repeated several times. The last (rightmost) value is kept
and the others are lost.
For instance CREATE COLLATION x (lc_ctype='en_US.UTF8',
lc_collate='en_US.UTF8', lc_ctype='C')
silently ignores lc_ctype='en_US.UTF8'. But that kind of invocation
isn't likely to be legit. It's more plausible that it's the result of
some mistake or confusion.
The same goes for the other options:

CREATE COLLATION [ IF NOT EXISTS ] name (
    [ LOCALE = locale, ]
    [ LC_COLLATE = lc_collate, ]
    [ LC_CTYPE = lc_ctype, ]
    [ PROVIDER = provider, ]
    [ DETERMINISTIC = boolean, ]
    [ VERSION = version ]
)

I suggest the attached simple patch to raise an error when any of
these options is specified multiple times.


Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: https://www.manitou-mail.org
Twitter: @DanielVerite

"Daniel Verite" <daniel@manitou-mail.org> writes:
> Currently, it's not an error for CREATE COLLATION to be invoked
> with options repeated several times. The last (rightmost) value is kept
> and the others are lost.
> ...
> I suggest the attached simple patch to raise an error when any of
> these options is specified multiple times.

Hmm ... I think that that is pretty standard behavior for a lot of
our utility commands.  Trying something at random,

regression=# create operator <<< (leftarg = int, leftarg = int,
regression(# rightarg = int99, procedure = int4pl, rightarg = int);
CREATE OPERATOR

Note the lack of any comment on the flat-out-wrong first value
for rightarg :-(

I can see the argument for complaining about repeated options rather
than letting this pass.  But shouldn't we try to make it uniform
instead of letting different commands have different policies?

            regards, tom lane

    Michael Paquier wrote:

> > Hmm ... I think that that is pretty standard behavior for a lot of
> > our utility commands.  Trying something at random,
>
> The behavior handling is a bit inconsistent.  For example EXPLAIN and
> VACUUM don't do that, because their parenthesized grammar got
> introduced after the flavor that handles options as separate items in
> the query, so redundant options was not something possible with only
> the original grammar.

Assuming we agree that redundant options should consistently
raise an error for a certain class of statements, could it be handled
at the grammar level?
If "list of options enforcing uniqueness" was a grammatical construct,
the redundancy would be caught by the parser and there would be no
need for ad-hoc code in the implementation of utility statements.
I don't know if that makes sense, unfortunately I know next to nothing
about bison.


Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: https://www.manitou-mail.org
Twitter: @DanielVerite

"Daniel Verite" <daniel@manitou-mail.org> writes:
> Assuming we agree that redundant options should consistently
> raise an error for a certain class of statements, could it be handled
> at the grammar level?

I don't think this'd be a great idea.  The grammar would have to
do something pretty brute-force to check for duplication, whereas
the individual statements typically know already whether they've
seen an instance of a particular option.

It will be kind of annoying to make similar changes in every statement,
agreed.  I wonder if there'd be any value in trying to make a subroutine
that deconstructs a DefElem list according to a provided list of option
names, allowing centralized handling of unknown-option and
duplicate-option cases, and maybe sharing handling of the simpler cases
such as boolean and integer options.  I'm not quite sure what the output
ought to look like though.  Some sort of statement-specific struct would
be really ideal, but C doesn't make that easy.

            regards, tom lane