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
