Re: [HACKERS] Refactoring identifier checks to consistently use strcmp

On Fri, Jan 12, 2018 at 11:35:48PM +0100, Daniel Gustafsson wrote:
> On 11 Jan 2018, at 09:01, Michael Paquier <michael(dot)paquier(at)gmail(dot)com> wrote:
>> I would like to think that a special section
>> dedicated to option compatibility for each command would be welcome to
>> track which grammar is supported and which grammar is not supported.
>
> I’m not sure I follow?
>
>>> One open question from this excercise is how to write a good test for
>>> this. It can either be made part of the already existing test queries
>>> or a separate suite. I’m leaning on the latter simply because the
>>> case-flipping existing tests seems like something that can be cleaned
>>> up years from now accidentally because it looks odd.
>>
>> Adding them into src/test/regress/ sounds like a good plan to me.
>
> If there is interest in this patch now that the list exists and aids review, I
> can turn the list into a proper test that makes a little more sense than the
> current list which is rather aimed at helping reviewers.

Sorry if my words were hard to catch here. What I mean here is to add in
each command's test file the set of commands which check the
compatibility. There is no need to test all the options in my opinion,
as just testing one option is enoughto show the purpose. So for example
to cover the grounds of DefineAggregate(), you could add a set of
commands in create_aggregate.sql. For DefineCollation(), those can go in
collate.sql, etc.

>> Here is another idea: nuking isstrict and iscachable from CREATE
>> FUNCTION syntax and forget about them. I would be tempted of the opinion
>> to do that before the rest.
>
> Thats certainly an option, I have no idea about the prevalence in real life
> production environments to have much an opinion to offer.

Please let me raise a new thread about this point with a proper
patch. That's rather separate to the work you are doing here, even if
those parameters are using pg_strcasecmp().
--
Michael