Re: GUC names in messages

On Fri, Dec 1, 2023 at 7:38 AM Peter Eisentraut <peter@eisentraut.org> wrote:
>
> On 30.11.23 06:59, Michael Paquier wrote:
> >                       ereport(elevel,
> >                                       (errcode(ERRCODE_UNDEFINED_OBJECT),
> > -                                      errmsg("unrecognized configuration parameter \"%s\" in file \"%s\" line %d",
> > -                                                     item->name,
> > +                     /* translator: %s%s%s is for an optionally quoted GUC name */
> > +                                      errmsg("unrecognized configuration parameter %s%s%s in file \"%s\" line %d",
> > +                                                     GUC_FORMAT(item->name),
> >                                                       item->filename, item->sourceline)));
>
> I think this is completely over-engineered and wrong.  If we start down
> this road, then the next person is going to start engineering some rules
> by which we should quote file names and other things.  Which will lead
> to more confusion, not less.  The whole point of this quoting thing is
> that you do it all the time or not, not dynamically based on what's
> inside of it.
>
> The original version of this string (and similar ones) seems the most
> correct, simple, and useful one to me.
>

Yeah, trying to manipulate the quoting dynamically seems like it was
an overreach...

Removing that still leaves some other changes needed to "fix" the
messages using MixedCase GUCs.


PSA v4

======
Details:

Patch 0001 -- "datestyle" becomes DateStyle in messages
Rebased this again, which was part of an earlier patch set
- I think any GUC names documented as MixedCase should keep that same
case in messages; this also obeys the guidelines recently pushed [1].
- Some others agreed, expecting the exact GUC name (in the message)
can be found in pg_settings [2].
- OTOH, Michael didn't like the diff churn [3] caused by this patch.

~~~

Patch 0002 -- use mixed case for intervalstyle error message
I found that the GUC name substituted to the error message was coming
from the statement, not from the original name in the guc_tables, so
there was a case mismatch:

BEFORE Patch 0002 (see the lowercase in the error message)
2023-12-08 13:21:32.897 AEDT [32609] STATEMENT:  set intervalstyle = 1234;
ERROR:  invalid value for parameter "intervalstyle": "1234"
HINT:  Available values: postgres, postgres_verbose, sql_standard, iso_8601.

AFTER Patch 0002
2023-12-08 13:38:48.638 AEDT [29684] STATEMENT:  set intervalstyle = 1234;
ERROR:  invalid value for parameter "IntervalStyle": "1234"
HINT:  Available values: postgres, postgres_verbose, sql_standard, iso_8601.

======
[1] GUC quoting guidelines -
https://github.com/postgres/postgres/commit/a243569bf65c5664436e8f63d870b7ee9c014dcb
[2] The case should match pg_settings -
https://www.postgresql.org/message-id/db3e4290ced77111c17e7a2adfb1d660734f5f78.camel%40cybertec.at
[3] Dislike of diff churn -
https://www.postgresql.org/message-id/ZWUd8dYYA9v83KvI%40paquier.xyz

Kind Regards,
Peter Smith.
Fujitsu Australia