Re: GUC names in messages

On Mon, Nov 27, 2023 at 12:44 PM Michael Paquier <michael(at)paquier(dot)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 <alvherre(at)alvh(dot)no-ip(dot)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