Re: GUC names in messages - Mailing list pgsql-hackers

Hi,

This thread seems to be a bit stuck, so I thought I would try to
summarize my understanding to hopefully get it moving again...

The original intent of the thread was just to introduce some
guidelines for quoting or not quoting GUC names in messages because
previously it seemed quite ad-hoc. Along the way, there was some scope
creep. IIUC, now there are 3 main topics in this thread:

1. GUC name quoting
2. GUC name case
3. Using common messages

======

#1. GUC name quoting.

Some basic guidelines were decided and a patch is already pushed [1].

   <para>
    In messages containing configuration variable names, do not include quotes
    when the names are visibly not natural English words, such as when they
    have underscores, are all-uppercase or have mixed case. Otherwise, quotes
    must be added. Do include quotes in a message where an arbitrary variable
    name is to be expanded.
   </para>

AFAIK there is nothing controversial there, although maybe the
guideline for 'mixed case' needs revisiting depending on objections
about point #2.

~~~

#2. GUC name case.

GUC names defined in guc_tables.c are either lowercase (xxxx),
lowercase with underscores (xxxx_yyyy) or mixed case (XxxxYyyy).

There are only a few examples of mixed case. They are a bit
problematic, but IIUC they are not going to change away so we need to
deal with them:
- TimeZone
- DateStyle
- IntervalStyle

It was proposed (e.g. [2]) that it would be better/intuitive if the
GUC name of the error message would be the same case as in the
guc_table.c. In other words, other words you should be able to find
the same name from the message in pg_settings.

So mesages with "datestyle" should become DateStyle because:
SELECT * FROM pg_settings WHERE name = 'DateStyle'; ==> found
SELECT * FROM pg_settings WHERE name = 'datestlye'; ==> not found

That latest v5 patches make those adjustments
- Patch v5-0001 fixes case for DateStyle. Yeah, there is some diff
churn because there are a lot of DateStyle tests, but IMO that's too
bad.
- Patch v5-0002 fixed case for IntervalStyle.

~~~

#3. Using common messages

Any message with a non-translatable component to them (e.g. the GUC
name) can be restructured in a way so there is a common translatable
errmsg part with the non-translatable parameters substituted.

e.g.
- GUC_check_errdetail("The only allowed value is \"immediate\".");
+ GUC_check_errdetail("The only allowed value is \"%s\".", "immediate");

AFAIK think there is no disagreement that this is a good idea,
although IMO it deserved to be in a separate thread.

I think there will be many messages that qualify to be modified, and
probably there will be some discussion about whether certain common
messages that can be merged -- (e.g. Is "You might need to increase
%s." same as "Consider increasing %s." or not?).

Anyway, this part is a WIP. Currently, patch v5-0003 makes a start for
this task.

//////

I think patches v5-0002, v5-0003 are uncontroversial.

So the sticking point seems to be the MixedCase GUC (e.g. patch
v5-0001). I agree, that the churn is not ideal (it's only because
there are lots of DateStyle messages in test output), but OTOH that's
just what happens if a rule is applied when previously there were no
rules.

Also, PeterE wrote [4]

> On Thu, Dec 14, 2023 at 09:38:40AM +0100, Peter Eisentraut wrote:
> > After these discussions, I think this rule change was not a good idea. It
> > effectively enforces these kinds of inconsistencies.  For example, if you
> > ever refactored
> >
> >     "DateStyle is wrong"
> >
> > to
> >
> >     "%s is wrong"
> >
> > you'd need to adjust the quotes, and thus user-visible behavior, for
> > entirely internal reasons.  This is not good.
>

I didn't understand the problem. By the current guidelines the mixed
case GUC won't quoted in the message (see patch v5-0001)

So whether it is:
errmsg("DateStyle is wrong"), OR
errmsg("%s is wrong", "DateStyle")

where is the "you'd need to adjust the quotes" problem there?

======
[1] GUC quoting guidelines --
https://www.postgresql.org/docs/devel/error-style-guide.html
[2] Case in messages should be same as pg_settings --
https://www.postgresql.org/message-id/db3e4290ced77111c17e7a2adfb1d660734f5f78.camel%40cybertec.at
[3] v5 patches --
https://www.postgresql.org/message-id/202312081255.wlsfmhe2sri7%40alvherre.pgsql
[4] PeterE concerna about DateStyle --
https://www.postgresql.org/message-id/6d66eb1a-290d-4aaa-972a-0a06a1af02af%40eisentraut.org

Kind Regards,
Peter Smith.
Fujitsu Australia