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

On Fri, Jan 26, 2018 at 6:36 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> I've pushed this mostly as-is. I fixed the missed places in reloptions
> code as we discussed. I also took out the parser changes related to
> allowing unquoted PARALLEL in old-style CREATE AGGREGATE, because that
> is not a goal I consider worthy of adding extra grammar complexity.
> We don't document that PARALLEL works there, and there has never been
> any expectation that deprecated legacy syntax would grow new options
> as needed to have feature parity with the modern syntax. I don't feel
> a need to go out of our way to prevent new options from working in the
> old syntax, if they happen to, but I definitely don't want to expend
> code on making them do so.
>
> Accordingly, that regression test that expected PARALLEL to work in
> the old-style syntax was just ill-considered, and I changed it to use
> the new-style syntax instead.
>
> I also trimmed the new regression test cases a bit, as most of them seemed
> pretty duplicative. How many times do we need to verify that the lexer
> doesn't downcase quoted identifiers? I'm not really sure that checking
> this behavior from SQL is useful at all, actually. The interesting
> question is whether there are code paths that can reach these string
> comparisons with strings that *didn't* get processed as identifiers by the
> lexer, and these tests do basically nothing to prove that there aren't.
> Still, I think we can push this and wait to see if there are complaints.

I think it's a shame that the commit message didn't document (for the
release notes) exactly which cases just got changed incompatibly. I
admit that not many people are likely to get bitten by this, but I
still think it's better if we're precise about what might cause a
particular user to be in that set.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company