On Fri, Dec 1, 2017 at 4:14 AM, Robert Haas <robertmh...@gmail.com> wrote: > I think the changes in DefineView and ATExecSetRelOptions is wrong, > because transformRelOptions() is still using pg_strcasecmp. With the > patch: > > rhaas=# create view v(x) with ("Check_option"="local") as select 1; > CREATE VIEW > rhaas=# create view v(x) with ("check_option"="local") as select 1; > ERROR: WITH CHECK OPTION is supported only on automatically updatable views > HINT: Views that do not select from a single table or view are not > automatically updatable. > > Oops.
I am getting the feeling that there could be other issues than this one... If this patch ever gets integrated, I think that this should actually be shaped as two patches: - One patch with the set of DDL queries taking advantage of the current grammar with pg_strcasecmp. - A second patch that does the switch from pg_strcasecmp to strcmp, and allows checking which paths of patch 1 are getting changed. Patch 1 is the hard part to figure out all the possible patterns that could be changed. > I am actually pretty dubious that we want to do this. I found one bug > already (see above), and I don't think there's any guarantee that it's > the only one, because it's pretty hard to be sure that none of the > remaining calls to pg_strcasecmp are being applied to any of these > values in some other part of the code. I'm not sure that the backward > compatibility issue is a huge deal, but from my point of view this > carries a significant risk of introducing new bugs, might annoy users > who spell any of these keywords in all caps with surrounding quotation > marks, and really has no clear benefit that I can see. The > originally-offered justification is that making this consistent would > help us avoid subtle bugs, but it seems at least as likely to CREATE > subtle bugs, and the status quo is AFAICT harming nobody. Changing opinion here ;) Yes, I agree that the risk of bugs may not be worth the compatibility effort at the end. I still see value in this patch for long-term purposes by making the code more consistent though. -- Michael