On 04/04/2017 10:13 AM, Daniel Gustafsson wrote:
On 04 Apr 2017, at 05:52, Alvaro Herrera <alvhe...@2ndquadrant.com> wrote:

Daniel Gustafsson wrote:
Testing DefElem options is done with both strcmp() and pg_strcasecmp() a bit
mixed.  Since the option defnames are all lowercased, either via IDENT, keyword
rules or “by hand” with makeString(), using strcmp() is safe (and assumed to be
so in quite a lot of places).

While it’s not incorrect per se to use pg_strcasecmp(), it has the potential to
hide a DefElem created with a mixed-case defname where it in other places is
expected to be in lowercase, which may lead to subtle bugs.

The attached patch refactors to use strcmp() consistently for option processing
in the command code as a pre-emptive belts+suspenders move against such subtle
bugs and to make the code more consistent.  Also reorders a few checks to have
all in the same “format” and removes a comment related to the above.

Tested with randomizing case on options in make check (not included in patch).

Does it work correctly in the Turkish locale?

Yes, running make check with random case defnames under tr_TR works fine.

This no longer works:

postgres=# CREATE TEXT SEARCH DICTIONARY public.simple_dict (
    TEMPLATE = pg_catalog.simple,
    "STOPWORds" = english
ERROR:  unrecognized simple dictionary parameter: "STOPWORds"

In hindsight, perhaps we should always have been more strict about that to begin wtih, but let's not break backwards-compatibility without a better reason. I didn't thoroughly check all of the cases here, to see if there are more like this.

It'd be nice to have some kind of a rule on when pg_strcasecmp should be used and when strcmp() is enough. Currently, by looking at the code, I can't tell.

- Heikki

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to