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

Daniel Gustafsson <daniel(at)yesql(dot)se> writes:
>> On 26 Jan 2018, at 22:32, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> I notice that there are two reloptions-related
>> "pg_strncasecmp" calls that did not get converted to "strncmp":
>> reloptions.c:804

> The way I read transformRelOptions(), oldOptions is not guaranteed to
> come from the parser (though in reality it probably will be).

Well, one response to that is that it should contain values that were
deemed acceptable at some previous time. If we allowed catalog contents
to be migrated physically across major releases, we'd need to worry about
having mixed-case reloptions in a v11 catalog ... but we don't, so we
won't. The old reloptions should always be ones that got through
parseRelOptions before, and those now will always have to be exact case.

Another response is that leaving it like this would mean that
transformRelOptions and parseRelOptions have different opinions about
whether two option names are the same or not, which seems to me to be
exactly the same sort of bug hazard that you were on about at the
beginning of this thread.

> The namespace isn’t either
> but passing an uppercase namespace should never be valid AFAICT, hence the
> patch changing it to case sensitive comparison.

Well, it's no more or less valid than an uppercase option name ...

>> and reloptions.h:169.

> Oversight, completely missed that one.

It looks like no core code uses that macro, so it's got no effect
unless some third-party code does ... but I changed it anyway.

regards, tom lane