Re: Refactoring identifier checks to consistently use strcmp

Daniel Gustafsson <daniel@yesql.se>

From: Daniel Gustafsson <daniel@yesql.se>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2017-04-04T07:13:03Z
Lists: pgsql-hackers
> On 04 Apr 2017, at 05:52, Alvaro Herrera <alvherre@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.

cheers ./daniel

Commits

  1. Avoid unnecessary use of pg_strcasecmp for already-downcased identifiers.