Обсуждение: Miscellaneous message fixes

Поиск
Список
Период
Сортировка

Miscellaneous message fixes

От
Kyotaro Horiguchi
Дата:
Hello.

I noticed that some error messages have issues such as non-standard
wording, slightly odd output, and minor grammatical mistakes, and have
attached patches addressing each of the items below.

0001:
guc_table.c:
>        .name = "effective_wal_level",
>...
>        .short_desc = gettext_noop("Show effective WAL level."),

We use "shows" in that context.


0002:
partbounds.c:
> errhint("ALTER TABLE ... MERGE PARTITIONS requires the partition bounds to be adjacent."),
...
> errhint("ALTER TABLE ... SPLIT PARTITION requires the partition bounds to be adjacent."),

These messages share the same body only differ in the command name. On
the other hand we have the following message in the same file.

> errhint("%s require combined bounds of new partitions must exactly match the bound of the split partition",
>                    "ALTER TABLE ... SPLIT PARTITION"),

So, I think that the first two messages shoud use the same structure
with the last one.


0003:

The last message above mistakenly uses the present tense, and some
other messages do the same. I think this should be fixed.


0004:

In the messages mentioned in 0002, the sentence structure is somewhat
hard to parse. Adding “that” after “requires” should improve clarity,
as in several other messages with the same issue.


0005:

In wait.c, there are three error messages with inconsistent wording:
two use "timeout value", while one uses "timeout" to refer to the same
object. This makes translation slightly awkward, so I suggest unifying
the wording.


0006: 
In extended_stats_funcs.c:
> errmsg("could not find extended statistics object \"%s\".\"%s\"",
>        quote_identifier(nspname),
>        quote_identifier(stxname)));

Since quote_identifier() already adds quoting when needed, adding
"%s"."%s" in the format string results in double quoting. It would be
better to use %s.%s instead.  I think quoting should be applied only
when necessary here. I'm not sure we should use
quote_qualified_identifier() instead.. (It's not used in the attached
patch).

There's a similar inconsistency.

>    errmsg("could not clear extended statistics object \"%s\".\"%s\": incorrect relation \"%s\".\"%s\" specified",
>           get_namespace_name(nspoid), stxname,
>           relnspname, relname));

In this part, the names are not passed through quote_identifier().  It
would be better to unify this as well.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

Вложения

Re: Miscellaneous message fixes

От
Kyotaro Horiguchi
Дата:
At Tue, 10 Feb 2026 14:37:52 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in 
> Hello.
> 
> I noticed that some error messages have issues such as non-standard
> wording, slightly odd output, and minor grammatical mistakes, and have
> attached patches addressing each of the items below.

Oops, sorry, the patch set causes a test failure.
I’ll look into it.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

Re: Miscellaneous message fixes

От
Michael Paquier
Дата:
On Tue, Feb 10, 2026 at 02:37:52PM +0900, Kyotaro Horiguchi wrote:
> 0006:
> In extended_stats_funcs.c:
>> errmsg("could not find extended statistics object \"%s\".\"%s\"",
>>        quote_identifier(nspname),
>>        quote_identifier(stxname)));
>
> Since quote_identifier() already adds quoting when needed, adding
> "%s"."%s" in the format string results in double quoting. It would be
> better to use %s.%s instead.  I think quoting should be applied only
> when necessary here. I'm not sure we should use
> quote_qualified_identifier() instead.. (It's not used in the attached
> patch).
>
> There's a similar inconsistency.
>
>>    errmsg("could not clear extended statistics object \"%s\".\"%s\": incorrect relation \"%s\".\"%s\" specified",
>>           get_namespace_name(nspoid), stxname,
>>           relnspname, relname));
>
> In this part, the names are not passed through quote_identifier().  It
> would be better to unify this as well.

0006 relates to some of my recent business, so I'll go look and
address this part.  Thanks for the report.
--
Michael

Вложения

Re: Miscellaneous message fixes

От
Álvaro Herrera
Дата:
On 2026-Feb-10, Kyotaro Horiguchi wrote:

Hello,

> 0006: 
> In extended_stats_funcs.c:
> > errmsg("could not find extended statistics object \"%s\".\"%s\"",
> >        quote_identifier(nspname),
> >        quote_identifier(stxname)));
> 
> Since quote_identifier() already adds quoting when needed, adding
> "%s"."%s" in the format string results in double quoting. It would be
> better to use %s.%s instead.  I think quoting should be applied only
> when necessary here. I'm not sure we should use
> quote_qualified_identifier() instead.. (It's not used in the attached
> patch).
> 
> There's a similar inconsistency.
> 
> >    errmsg("could not clear extended statistics object \"%s\".\"%s\": incorrect relation \"%s\".\"%s\" specified",
> >           get_namespace_name(nspoid), stxname,
> >           relnspname, relname));
> 
> In this part, the names are not passed through quote_identifier().  It
> would be better to unify this as well.

Hmm, in the vast majority of messages, the quotes are in the literal
string, and we do not add quote_identifier().  This way, the names are
always quoted, not just when they are funny identifiers; also the
translator chooses the quoting style they want, which is not necessarily
the same as the one used in English.

So my recommendation would be to remove the quote_identifier() calls and
leave the quotes in the message.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/



Re: Miscellaneous message fixes

От
Tom Lane
Дата:
=?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de> writes:
> On 2026-Feb-10, Kyotaro Horiguchi wrote:
>> In extended_stats_funcs.c:
>>> errmsg("could not find extended statistics object \"%s\".\"%s\"",
>>>        quote_identifier(nspname),
>>>        quote_identifier(stxname)));
>>
>> Since quote_identifier() already adds quoting when needed, adding
>> "%s"."%s" in the format string results in double quoting. It would be
>> better to use %s.%s instead.  I think quoting should be applied only
>> when necessary here. I'm not sure we should use
>> quote_qualified_identifier() instead.

> Hmm, in the vast majority of messages, the quotes are in the literal
> string, and we do not add quote_identifier().  This way, the names are
> always quoted, not just when they are funny identifiers; also the
> translator chooses the quoting style they want, which is not necessarily
> the same as the one used in English.

Yeah, this code is simply wrong.  You should remove the
quote_identifier() calls and otherwise leave it as-is.

quote_identifier(), quote_qualified_identifier(), etc are meant for
building valid SQL strings.  However, an error message is a totally
different animal.  We decided years ago that the preferred style is
to wrap "..." around the unadorned identifier *in the error text*,
not by using quote_identifier(), so that translators could replace
the double-quote marks with appropriate marks for their language.
Is the English version 100% consistent in the presence of identifiers
containing double-quote marks?  No.  But we'd be making matters
worse not better for other languages if we did it differently.

This choice is documented at

https://www.postgresql.org/docs/devel/error-style-guide.html#ERROR-STYLE-GUIDE-QUOTATION-MARKS

            regards, tom lane



Re: Miscellaneous message fixes

От
Michael Paquier
Дата:
On Tue, Feb 10, 2026 at 09:55:23AM -0500, Tom Lane wrote:
> Yeah, this code is simply wrong.  You should remove the
> quote_identifier() calls and otherwise leave it as-is.
>
> quote_identifier(), quote_qualified_identifier(), etc are meant for
> building valid SQL strings.  However, an error message is a totally
> different animal.  We decided years ago that the preferred style is
> to wrap "..." around the unadorned identifier *in the error text*,
> not by using quote_identifier(), so that translators could replace
> the double-quote marks with appropriate marks for their language.
> Is the English version 100% consistent in the presence of identifiers
> containing double-quote marks?  No.  But we'd be making matters
> worse not better for other languages if we did it differently.

Right, I had a brain fart on this one.  Using "%s"."%s" where the
namespace and object names were independently quoted is also not
project style.  At the end, I have removed the quote_identifier()
calls altogether, and reduced the number of quotes in the error
strings, leading to f33c58577422 as a result.  If there is anything
else, please let me know.
--
Michael

Вложения

Re: Miscellaneous message fixes

От
Tom Lane
Дата:
Michael Paquier <michael@paquier.xyz> writes:
> Right, I had a brain fart on this one.  Using "%s"."%s" where the
> namespace and object names were independently quoted is also not
> project style.  At the end, I have removed the quote_identifier()
> calls altogether, and reduced the number of quotes in the error
> strings, leading to f33c58577422 as a result.  If there is anything
> else, please let me know. 

Yeah, f33c58577422 looks like what we've done elsewhere.  OK by me.

            regards, tom lane



Re:Miscellaneous message fixes

От
"zengman"
Дата:
Hi,

Per (https://www.postgresql.org/docs/devel/error-style-guide.html#ERROR-STYLE-GUIDE-TRICKY-WORDS) where it says: 
```
Avoid contractions, like “can't”; use “cannot” instead.
```
could we add an extra 0007 patch for this?

--
regards,
Man Zeng
Вложения

Re: Miscellaneous message fixes

От
Michael Paquier
Дата:
On Tue, Feb 10, 2026 at 02:37:52PM +0900, Kyotaro Horiguchi wrote:
> I noticed that some error messages have issues such as non-standard
> wording, slightly odd output, and minor grammatical mistakes, and have
> attached patches addressing each of the items below.

In the set of patches that Horiguchi-san has sent, I have taken care
of 0006 as it was my own business.

Added in CC the two committers that own the other items reported.

Sawada-san, could you take care of 0001 as effective_wal_level is your
thing?

Alexander-san, could you take take of 0002~0005?  These relate to the
WAIT command and the partition manipulation patches.

Thanks,
--
Michael

Вложения

Re: Miscellaneous message fixes

От
Masahiko Sawada
Дата:
On Mon, Mar 9, 2026 at 11:05 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Tue, Feb 10, 2026 at 02:37:52PM +0900, Kyotaro Horiguchi wrote:
> > I noticed that some error messages have issues such as non-standard
> > wording, slightly odd output, and minor grammatical mistakes, and have
> > attached patches addressing each of the items below.
>
> In the set of patches that Horiguchi-san has sent, I have taken care
> of 0006 as it was my own business.
>
> Added in CC the two committers that own the other items reported.
>
> Sawada-san, could you take care of 0001 as effective_wal_level is your
> thing?

Sure, I'll take care of it.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com



Re: Miscellaneous message fixes

От
Masahiko Sawada
Дата:
On Tue, Mar 10, 2026 at 11:20 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Mon, Mar 9, 2026 at 11:05 PM Michael Paquier <michael@paquier.xyz> wrote:
> >
> > On Tue, Feb 10, 2026 at 02:37:52PM +0900, Kyotaro Horiguchi wrote:
> > > I noticed that some error messages have issues such as non-standard
> > > wording, slightly odd output, and minor grammatical mistakes, and have
> > > attached patches addressing each of the items below.
> >
> > In the set of patches that Horiguchi-san has sent, I have taken care
> > of 0006 as it was my own business.
> >
> > Added in CC the two committers that own the other items reported.
> >
> > Sawada-san, could you take care of 0001 as effective_wal_level is your
> > thing?
>
> Sure, I'll take care of it.

Pushed. Thank you for the report!

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com