Re: GUC names in messages

From: Peter Smith <smithpb2250(at)gmail(dot)com>
To: Peter Eisentraut <peter(at)eisentraut(dot)org>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Nathan Bossart <nathandbossart(at)gmail(dot)com>, Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Daniel Gustafsson <daniel(at)yesql(dot)se>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: GUC names in messages
Date: 2023-12-08 04:10:29
Message-ID: CAHut+Ps3Lbo=hGevYf8AyjFo8+=SSwXUo7onB-6nS6e-Q-bQoQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Dec 1, 2023 at 7:38 AM Peter Eisentraut <peter(at)eisentraut(dot)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

Attachment Content-Type Size
v4-0002-GUC-names-use-mixed-case-for-intervalstyle-error-.patch application/octet-stream 853 bytes
v4-0001-GUC-names-use-mixed-case-for-datestyle-in-message.patch application/octet-stream 11.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2023-12-08 04:10:45 Re: micro-optimizing json.c
Previous Message Nathan Bossart 2023-12-08 04:02:09 Re: micro-optimizing json.c