> On 26 Jan 2018, at 23:58, Tom Lane <t...@sss.pgh.pa.us> wrote: > > Daniel Gustafsson <dan...@yesql.se> writes: >>> On 26 Jan 2018, at 22:32, Tom Lane <t...@sss.pgh.pa.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.
That’s a good point, the reloptions will only ever come from the same major version. > 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. Completely agree. >> 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 … Agreed. Taking a step back and thinking it’s clear that the comparison in the oldoptions loop should’ve been changed in the patch for it to be consistent with the original objective. cheers ./daniel
