Remove an unnecessary errmsg_plural in dependency.c
Hi,
It looks like the following errmsg_plural in dependency.c is
unnecessary as numReportedClient > 1 always and numNotReportedClient
can never be < 0. Therefore plural version of the error message is
sufficient. Attached a patch to fix it.
@@ -1200,10 +1200,8 @@ reportDependentObjects(const ObjectAddresses
*targetObjects,
{
ereport(msglevel,
/* translator: %d always has a value larger than 1 */
- (errmsg_plural("drop cascades to %d
other object",
- "drop
cascades to %d other objects",
-
numReportedClient + numNotReportedClient,
-
numReportedClient + numNotReportedClient),
+ (errmsg("drop cascades to %d other objects",
+ numReportedClient +
numNotReportedClient),
errdetail("%s", clientdetail.data),
errdetail_log("%s", logdetail.data)));
Regards,
Bharath Rupireddy.
Attachments:
v1-0001-Remove-an-unnecessary-errmsg_plural-in-dependency.patchapplication/x-patch; name=v1-0001-Remove-an-unnecessary-errmsg_plural-in-dependency.patchDownload
From 50328d92744f279bf26e72b6dc18c4dd9679e4aa Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Wed, 23 Mar 2022 16:20:56 +0000
Subject: [PATCH v1] Remove an unnecessary errmsg_plural in dependency.c
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The errmsg_plural for "drop cascades to %d other objects" is
unnecessary as numReportedClient > 1 always and
numNotReportedClient can never be < 0. Therefore plural version of
the error message is sufficient.
---
src/backend/catalog/dependency.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index ab9e42d7d1..2ca4492620 100644
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -1200,10 +1200,8 @@ reportDependentObjects(const ObjectAddresses *targetObjects,
{
ereport(msglevel,
/* translator: %d always has a value larger than 1 */
- (errmsg_plural("drop cascades to %d other object",
- "drop cascades to %d other objects",
- numReportedClient + numNotReportedClient,
- numReportedClient + numNotReportedClient),
+ (errmsg("drop cascades to %d other objects",
+ numReportedClient + numNotReportedClient),
errdetail("%s", clientdetail.data),
errdetail_log("%s", logdetail.data)));
}
--
2.25.1
On 23.03.22 17:33, Bharath Rupireddy wrote:
It looks like the following errmsg_plural in dependency.c is
unnecessary as numReportedClient > 1 always and numNotReportedClient
can never be < 0. Therefore plural version of the error message is
sufficient. Attached a patch to fix it.
Some languages have more than two forms, so we still need to keep this
to handle those.
At Wed, 23 Mar 2022 17:39:52 +0100, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote in
On 23.03.22 17:33, Bharath Rupireddy wrote:
It looks like the following errmsg_plural in dependency.c is
unnecessary as numReportedClient > 1 always and numNotReportedClient
can never be < 0. Therefore plural version of the error message is
sufficient. Attached a patch to fix it.Some languages have more than two forms, so we still need to keep this
to handle those.
The point seems to be that numReportedClient + numNotReportedClient >=
2 (not 1) there. So the singular form is never used. It doesn't harm
as-is but translation burden decreases a bit by fixing it.
By the way it has a translator-note as follows.
else if (numReportedClient > 1)
{
ereport(msglevel,
/* translator: %d always has a value larger than 1 */
(errmsg_plural("drop cascades to %d other object",
"drop cascades to %d other objects",
numReportedClient + numNotReportedClient,
numReportedClient + numNotReportedClient),
The comment and errmsg_plural don't seem to be consistent. When the
code was added by c4f2a0458d, it had only singular form and already
had the comment. After that 8032d76b5 turned it to errmsg_plural
ignoring the comment. It seems like a thinko of 8032d76b5.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
On 24 Mar 2022, at 06:17, Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
The comment and errmsg_plural don't seem to be consistent. When the
code was added by c4f2a0458d, it had only singular form and already
had the comment. After that 8032d76b5 turned it to errmsg_plural
ignoring the comment. It seems like a thinko of 8032d76b5.
Following the bouncing ball, that seems like a reasonable conclusion, and
removing the plural form should be fine to reduce translator work.
--
Daniel Gustafsson https://vmware.com/
On Thu, Mar 24, 2022 at 2:34 PM Daniel Gustafsson <daniel@yesql.se> wrote:
On 24 Mar 2022, at 06:17, Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
The comment and errmsg_plural don't seem to be consistent. When the
code was added by c4f2a0458d, it had only singular form and already
had the comment. After that 8032d76b5 turned it to errmsg_plural
ignoring the comment. It seems like a thinko of 8032d76b5.Following the bouncing ball, that seems like a reasonable conclusion, and
removing the plural form should be fine to reduce translator work.
Yes, the singular version of the message isn't required at all as
numReportedClient > 1. Hence I proposed to remove errmsg_plural and
singular version.
Regards,
Bharath Rupireddy.
On 24.03.22 13:48, Bharath Rupireddy wrote:
Yes, the singular version of the message isn't required at all as
numReportedClient > 1. Hence I proposed to remove errmsg_plural and
singular version.
The issue is that n == 1 and n != 1 are not the only cases that
errmsg_plural() handles. Some languages have different forms for n ==
1, n == 2, and n >= 5, for example. So while it is true that in
errmsg_plural("drop cascades to %d other object",
"drop cascades to %d other objects",
the English singular string will never be used, you have to keep the
errmsg_plural() call so that it can handle variants like the above for
other languages.
You could write
errmsg_plural("DUMMY NOT USED %d",
"drop cascades to %d other objects",
but I don't think that is better.
On 24.03.22 06:17, Kyotaro Horiguchi wrote:
The comment and errmsg_plural don't seem to be consistent. When the
code was added by c4f2a0458d, it had only singular form and already
had the comment. After that 8032d76b5 turned it to errmsg_plural
ignoring the comment. It seems like a thinko of 8032d76b5.
I have removed the comment.
On 24 Mar 2022, at 14:07, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote:
On 24.03.22 06:17, Kyotaro Horiguchi wrote:
The comment and errmsg_plural don't seem to be consistent. When the
code was added by c4f2a0458d, it had only singular form and already
had the comment. After that 8032d76b5 turned it to errmsg_plural
ignoring the comment. It seems like a thinko of 8032d76b5.I have removed the comment.
I was just typing a reply to your upthread answer that we should just remove
the comment then, so a retroactive +1 on this =)
--
Daniel Gustafsson https://vmware.com/
On Thu, Mar 24, 2022 at 6:35 PM Peter Eisentraut
<peter.eisentraut@enterprisedb.com> wrote:
On 24.03.22 13:48, Bharath Rupireddy wrote:
Yes, the singular version of the message isn't required at all as
numReportedClient > 1. Hence I proposed to remove errmsg_plural and
singular version.The issue is that n == 1 and n != 1 are not the only cases that
errmsg_plural() handles. Some languages have different forms for n ==
1, n == 2, and n >= 5, for example. So while it is true that inerrmsg_plural("drop cascades to %d other object",
"drop cascades to %d other objects",
Thanks. I think I get the point - is it dngettext doing things
differently for different languages?
#define EVALUATE_MESSAGE_PLURAL(domain, targetfield, appendval) \
{ \
const char *fmt; \
StringInfoData buf; \
/* Internationalize the error format string */ \
if (!in_error_recursion_trouble()) \
fmt = dngettext((domain), fmt_singular, fmt_plural, n); \
else \
fmt = (n == 1 ? fmt_singular : fmt_plural); \
initStringInfo(&buf); \
Regards,
Bharath Rupireddy.
Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> writes:
Thanks. I think I get the point - is it dngettext doing things
differently for different languages?
Yeah. To be concrete, have a look in ru.po:
#: catalog/dependency.c:1208
#, c-format
msgid "drop cascades to %d other object"
msgid_plural "drop cascades to %d other objects"
msgstr[0] "удаление распространяется на ещё %d объект"
msgstr[1] "удаление распространяется на ещё %d объекта"
msgstr[2] "удаление распространяется на ещё %d объектов"
I don't know Russian, so I don't know exactly what's going
on there, but there's evidently three different forms in
that language. Probably one of them is not reachable given
that n > 1, but I doubt we're saving the translator any time
with that info. Besides, gettext might require all three
forms to be provided anyway in order to work correctly.
regards, tom lane
On 2022-Mar-24, Bharath Rupireddy wrote:
Thanks. I think I get the point - is it dngettext doing things
differently for different languages?
Yes. The dngettext() rules are embedded in each translation's catalog
file:
$ git grep 'Plural-Forms' src/backend/po/*.po
de.po:"Plural-Forms: nplurals=2; plural=(n != 1);\n"
es.po:"Plural-Forms: nplurals=2; plural=n != 1;\n"
fr.po:"Plural-Forms: nplurals=2; plural=(n > 1);\n"
id.po:"Plural-Forms: nplurals=2; plural=(n > 1);\n"
it.po:"Plural-Forms: nplurals=2; plural=n != 1;\n"
ja.po:"Plural-Forms: nplurals=2; plural=n != 1;\n"
ko.po:"Plural-Forms: nplurals=1; plural=0;\n"
pl.po:"Plural-Forms: nplurals=3; plural=(n==1 ? 0 : n%10>=2 && n%10<=4 && (n%100<10 "
pt_BR.po:"Plural-Forms: nplurals=2; plural=(n>1);\n"
ru.po:"Plural-Forms: nplurals=3; plural=(n%10==1 && n%100!=11 ? 0 : n%10>=2 && n"
sv.po:"Plural-Forms: nplurals=2; plural=(n != 1);\n"
tr.po:"Plural-Forms: nplurals=2; plural=(n != 1);\n"
uk.po:"Plural-Forms: nplurals=4; plural=((n%10==1 && n%100!=11) ? 0 : ((n%10 >= 2 && n%10 <=4 && (n%100 < 12 || n%100 > 14)) ? 1 : ((n%10 == 0 || (n%10 >= 5 && n%10 <=9)) || (n%100 >= 11 && n%100 <= 14)) ? 2 : 3));\n"
zh_CN.po:"Plural-Forms: nplurals=1; plural=0;\n"
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
Thou shalt study thy libraries and strive not to reinvent them without
cause, that thy code may be short and readable and thy days pleasant
and productive. (7th Commandment for C Programmers)
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
$ git grep 'Plural-Forms' src/backend/po/*.po
ru.po:"Plural-Forms: nplurals=3; plural=(n%10==1 && n%100!=11 ? 0 : n%10>=2 && n"
Oh, interesting: if I'm reading that right, all three Russian
forms are reachable, even with the knowledge that n > 1.
(But isn't the last "&& n" test redundant?)
regards, tom lane
On 2022-Mar-24, Tom Lane wrote:
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
$ git grep 'Plural-Forms' src/backend/po/*.po
ru.po:"Plural-Forms: nplurals=3; plural=(n%10==1 && n%100!=11 ? 0 : n%10>=2 && n"Oh, interesting: if I'm reading that right, all three Russian
forms are reachable, even with the knowledge that n > 1.
(But isn't the last "&& n" test redundant?)
I wondered about that trailing 'n' and it turns out that the grep was
too simplistic, so it's incomplete. The full rule is:
"Plural-Forms: nplurals=3; plural=(n%10==1 && n%100!=11 ? 0 : n%10>=2 && n"
"%10<=4 && (n%100<10 || n%100>=20) ? 1 : 2);\n"
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
At Thu, 24 Mar 2022 10:19:18 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in
Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> writes:
Thanks. I think I get the point - is it dngettext doing things
differently for different languages?Yeah. To be concrete, have a look in ru.po:
I wondered why it takes two forms of format string but I now
understand it is the fall-back texts used when translation is not
found.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
At Thu, 24 Mar 2022 16:00:58 +0100, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote in
On 2022-Mar-24, Tom Lane wrote:
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
$ git grep 'Plural-Forms' src/backend/po/*.po
ru.po:"Plural-Forms: nplurals=3; plural=(n%10==1 && n%100!=11 ? 0 : n%10>=2 && n"Oh, interesting: if I'm reading that right, all three Russian
forms are reachable, even with the knowledge that n > 1.
(But isn't the last "&& n" test redundant?)I wondered about that trailing 'n' and it turns out that the grep was
too simplistic, so it's incomplete. The full rule is:"Plural-Forms: nplurals=3; plural=(n%10==1 && n%100!=11 ? 0 : n%10>=2 && n"
"%10<=4 && (n%100<10 || n%100>=20) ? 1 : 2);\n"
FWIW just for fun, I saw the first form.
postgres=# drop table t cascade;
ЗАМЕЧАНИЕ: удаление распространяется на ещё 21 объект
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Import Notes
Reply to msg id not found: 202203241500.usbymbyx4fbm@alvherre.pgsql74ce9862-c723-45fd-d3bb-f4901515320e@enterprisedb.com