Thread

Commits

Remove unnecessary translator comment
- a47651447f01 15.0 landed

Remove an unnecessary errmsg_plural in dependency.c

Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2022-03-23T16:33:55Z

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.

Re: Remove an unnecessary errmsg_plural in dependency.c

Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2022-03-23T16:39:52Z

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.

Re: Remove an unnecessary errmsg_plural in dependency.c

Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2022-03-24T05:17:03Z

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

Re: Remove an unnecessary errmsg_plural in dependency.c

Daniel Gustafsson <daniel@yesql.se> — 2022-03-24T09:04:28Z

> 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/

Re: Remove an unnecessary errmsg_plural in dependency.c

Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2022-03-24T12:48:38Z

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.

Re: Remove an unnecessary errmsg_plural in dependency.c

Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2022-03-24T13:05:27Z

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.

Re: Remove an unnecessary errmsg_plural in dependency.c

Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2022-03-24T13:07:35Z

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.

Re: Remove an unnecessary errmsg_plural in dependency.c

Daniel Gustafsson <daniel@yesql.se> — 2022-03-24T13:11:01Z

> 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/

Re: Remove an unnecessary errmsg_plural in dependency.c

Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2022-03-24T13:34:20Z

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 in
>
>      errmsg_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.

Re: Remove an unnecessary errmsg_plural in dependency.c

Tom Lane <tgl@sss.pgh.pa.us> — 2022-03-24T14:19:18Z

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

Re: Remove an unnecessary errmsg_plural in dependency.c

Alvaro Herrera <alvherre@alvh.no-ip.org> — 2022-03-24T14:28:57Z

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)

Re: Remove an unnecessary errmsg_plural in dependency.c

Tom Lane <tgl@sss.pgh.pa.us> — 2022-03-24T14:49:18Z

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

Re: Remove an unnecessary errmsg_plural in dependency.c

Alvaro Herrera <alvherre@alvh.no-ip.org> — 2022-03-24T15:00:58Z

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/

Re: Remove an unnecessary errmsg_plural in dependency.c

Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2022-03-25T01:45:54Z

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

Re: Remove an unnecessary errmsg_plural in dependency.c

Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2022-03-25T01:52:46Z

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