On Mon, Nov 27, 2023 at 12:44 PM Michael Paquier <mich...@paquier.xyz> wrote: > > On Mon, Nov 27, 2023 at 10:04:35AM +1100, Peter Smith wrote: > > On Fri, Nov 24, 2023 at 8:53 PM Alvaro Herrera <alvhe...@alvh.no-ip.org> > > wrote: > >> Yeah. Also, these could be changed to have the GUC name outside the > >> message proper, which would reduce the total number of messages. (But > >> care must be given to the word "the" there.) > > > > I had posted something similar a few posts back [1], but it just > > caused more questions unrelated to GUC name quotes so I abandoned that > > temporarily. > > Yes, I kind of agree to let that out of the picture for the moment. > It would be good to reduce the translation chunks. > > > So for now, I hope this thread can be only about quotes on GUC names, > > otherwise, I thought it may become stuck debating dozens of individual > > messages. Certainly later, or in another thread, we can revisit all > > messages again to try to identify/extract any "common" ones. > > -HINT: Perhaps you need a different "datestyle" setting. > +HINT: Perhaps you need a different DateStyle setting. > > Is the change for "datestyle" really required? It does not betray the > GUC quoting policy added by 0001. >
TBH, I suspect something fishy about these mixed-case GUCs. In the documentation and in the guc_tables.c they are all described in MixedCase (e.g. "DateStyle" instead of "datestyle"), so I felt the messages should use the same case the documentation, which is why I changed all the ones you are referring to. I know the code is doing a case-insensitive hashtable lookup but I suspect some of the string passing still in the code for those particular GUCs ought to be using the same mixed case string literal as in the guc_tables.c. Currently, I have seen a few quirks where the case is inconsistent with the MixedCase docs. It needs some more investigation to understand the reason. For example, 2023-11-27 11:03:48.565 AEDT [15303] STATEMENT: set intervalstyle=123; ERROR: invalid value for parameter "intervalstyle": "123" versus 2023-11-27 11:13:56.018 AEDT [15303] STATEMENT: set datestyle=123; ERROR: invalid value for parameter DateStyle: "123" > >> I think we could leave these improvements for a second round. They > >> don't need to hold back the improvement we already have. > > > > I tried something for this already but kept it in a separate patch. See > > v2-0003 > > + if (*p == '_') > + underscore = true; > > Is there a reason why we don't just use islower() or is that just to > get something entirely local independent? I am not sure that it needs > to be that complicated. We should just check that all the characters > are lower-case and apply quotes. Thanks for the feedback. Probably I have overcomplicated it. I'll revisit it. ====== Kind Regards, Peter Smith. Fujitsu Australia
