Обсуждение: Miscellaneous message fixes
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
Вложения
- 0001-Use-third-person-singular-Shows-for-reporting-variab.patch
- 0002-Consolidate-target-only-message-variants.patch
- 0003-Fix-grammar-in-a-partition-related-message.patch
- 0004-Add-omitted-that-for-clarity-in-some-messages.patch
- 0005-Unify-wording-of-timeout-error-messages.patch
- 0006-Fix-double-quoting-in-extended-statistics-error-mess.patch
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
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
Вложения
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/
=?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
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
Вложения
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
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
Вложения
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
Вложения
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
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