Re: [HACKERS] Refactoring identifier checks to consistently use strcmp - Mailing list pgsql-hackers

From Daniel Gustafsson
Subject Re: [HACKERS] Refactoring identifier checks to consistently use strcmp
Date
Msg-id 636F9683-C4B7-47EF-A5BB-57735B6A0894@yesql.se
Whole thread Raw
In response to Re: [HACKERS] Refactoring identifier checks to consistently use strcmp  (Michael Paquier <michael.paquier@gmail.com>)
Responses Re: [HACKERS] Refactoring identifier checks to consistently use strcmp
List pgsql-hackers
> On 17 Nov 2017, at 03:31, Michael Paquier <michael.paquier@gmail.com> wrote:
>
> On Tue, Sep 5, 2017 at 5:34 PM, Daniel Gustafsson <daniel@yesql.se> wrote:
>>> On 17 Aug 2017, at 11:08, Daniel Gustafsson <daniel@yesql.se> wrote:
>>>> On 16 Aug 2017, at 17:51, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>>> My thought is that if we are looking at words that have been through the
>>>> parser, then it should *always* be plain strcmp; we should expect that
>>>> the parser already did the appropriate case-folding.
>>>
>>> +1
>>>
>>>> pg_strcasecmp would be appropriate, perhaps, if we're dealing with stuff
>>>> that somehow came in without going through the parser.
>>>
>>> In that case it would be up to the consumer of the data to handle required
>>> case-folding for the expected input, so pg_strcasecmp or strcmp depending on
>>> situation.
>>
>> This patch has been marked “Waiting on Author”, but I’m not sure what the
>> concensus of this thread came to with regards to quoted keywords and backwards
>> compatibility.  There seems to be a 2-1 vote for allowing a break, and forcing
>> all keywords out of the parser to be casefolded. Any other opinions?
>
> This patch impacts the DDL grammar of aggregates, operators,
> collations, text search, views, etc. Still I agree with the purpose of
> this thread that it would be nice to get a more consistent behavior
> even if it breaks some queries, so +1 for the argument with the
> post-parser comparison which should use strcmp.

Thanks for reviewing!

> The patch needs a rebase, and there are a couple of places that need
> an extra lookup I think:
> $ git grep defname -- *.c | grep strcasecmp | wc -l
>      39

Rebased and handled a few more places which I had either missed in the last
round, or that had been added in the meantime.  “PARALLEL” in aggregatecmds.c
is intentionally using pg_strcasecmp() due to the old-style syntax which is
still supported. AFAICS this covers all relevant codepaths from the 39 above.

cheers ./daniel


Attachment

pgsql-hackers by date:

Previous
From: Oliver Ford
Date:
Subject: Re: Add RANGE with values and exclusions clauses to the Window Functions
Next
From: Tom Lane
Date:
Subject: Re: [HACKERS] Bug in ExecModifyTable function and trigger issues for foreign tables