Skip to content

Commit 76d4abf

Browse files
tglsfdctglsfdc
committed
Improve the recently-added support for properly pluralized error messages
by extending the ereport() API to cater for pluralization directly. This is better than the original method of calling ngettext outside the elog.c code because (1) it avoids double translation, which wastes cycles and in the worst case could give a wrong result; and (2) it avoids having to use a different coding method in PL code than in the core backend. The client-side uses of ngettext are not touched since neither of these concerns is very pressing in the client environment. Per my proposal of yesterday.
1 parent fd416db commit 76d4abf

File tree

17 files changed

+292
-102
lines changed

17 files changed

+292
-102
lines changed

‎doc/src/sgml/nls.sgml

Lines changed: 45 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<!-- $PostgreSQL: pgsql/doc/src/sgml/nls.sgml,v 1.17 2009/01/09 10:54:07 petere Exp $ -->
1+
<!-- $PostgreSQL: pgsql/doc/src/sgml/nls.sgml,v 1.18 2009/06/04 18:33:06 tgl Exp $ -->
22

33
<chapter id="nls">
44
<chapterinfo>
@@ -46,7 +46,7 @@
4646
<filename>msgmerge</filename>, respectively, in a GNU-compatible
4747
implementation. Later, we will try to arrange it so that if you
4848
use a packaged source distribution, you won't need
49-
<filename>xgettext</filename>. (From CVS, you will still need
49+
<filename>xgettext</filename>. (If working from CVS, you will still need
5050
it.) <application>GNU Gettext 0.10.36</application> or later is currently recommended.
5151
</para>
5252

@@ -152,7 +152,7 @@ msgstr "another translated"
152152
If there are already some <filename>.po</filename> files, then
153153
someone has already done some translation work. The files are
154154
named <filename><replaceable>language</replaceable>.po</filename>,
155-
where <replaceable>language</replaceable> is the
155+
where <replaceable>language</replaceable> is the
156156
<ulink url="http://lcweb.loc.gov/standards/iso639-2/englangn.html">
157157
ISO 639-1 two-letter language code (in lower case)</ulink>, e.g.,
158158
<filename>fr.po</filename> for French. If there is really a need
@@ -224,7 +224,7 @@ gmake update-po
224224
that gives room for other people to pick up your work. However,
225225
you are encouraged to give priority to removing fuzzy entries
226226
after doing a merge. Remember that fuzzy entries will not be
227-
installed; they only serve as reference what might be the right
227+
installed; they only serve as reference for what might be the right
228228
translation.
229229
</para>
230230

@@ -347,8 +347,8 @@ fprintf(stderr, "panic level %d\n", lvl);
347347
<programlisting>
348348
fprintf(stderr, gettext("panic level %d\n"), lvl);
349349
</programlisting>
350-
(<symbol>gettext</symbol> is defined as a no-op if no NLS is
351-
configured.)
350+
(<symbol>gettext</symbol> is defined as a no-op if NLS support is
351+
not configured.)
352352
</para>
353353

354354
<para>
@@ -421,6 +421,9 @@ fprintf(stderr, gettext("panic level %d\n"), lvl);
421421
them here. If the translatable string is not the first
422422
argument, the item needs to be of the form
423423
<literal>func:2</literal> (for the second argument).
424+
If you have a function that supports pluralized messages,
425+
the item should look like <literal>func:1,2</literal>
426+
(identifying the singular and plural message arguments).
424427
</para>
425428
</listitem>
426429
</varlistentry>
@@ -451,8 +454,8 @@ fprintf(stderr, gettext("panic level %d\n"), lvl);
451454
printf("Files were %s.\n", flag ? "copied" : "removed");
452455
</programlisting>
453456
The word order within the sentence might be different in other
454-
languages. Also, even if you remember to call gettext() on each
455-
fragment, the fragments might not translate well separately. It's
457+
languages. Also, even if you remember to call <function>gettext()</> on
458+
each fragment, the fragments might not translate well separately. It's
456459
better to duplicate a little code so that each message to be
457460
translated is a coherent whole. Only numbers, file names, and
458461
such-like run-time variables should be inserted at run time into
@@ -475,13 +478,44 @@ else
475478
printf("copied %d files", n):
476479
</programlisting>
477480
then be disappointed. Some languages have more than two forms,
478-
with some peculiar rules. We might have a solution for this in
479-
the future, but for now the matter is best avoided altogether.
480-
You could write:
481+
with some peculiar rules. It's often best to design the message
482+
to avoid the issue altogether, for instance like this:
481483
<programlisting>
482484
printf("number of copied files: %d", n);
483485
</programlisting>
484486
</para>
487+
488+
<para>
489+
If you really want to construct a properly pluralized message,
490+
there is support for this, but it's a bit awkward. When generating
491+
a primary or detail error message in <function>ereport()</>, you can
492+
write something like this:
493+
<programlisting>
494+
errmsg_plural("copied %d file",
495+
"copied %d files",
496+
n,
497+
n)
498+
</programlisting>
499+
The first argument is the format string appropriate for English
500+
singular form, the second is the format string appropriate for
501+
English plural form, and the third is the integer control value
502+
that determines which plural form to use. Subsequent arguments
503+
are formatted per the format string as usual. (Normally, the
504+
pluralization control value will also be one of the values to be
505+
formatted, so it has to be written twice.) In English it only
506+
matters whether <replaceable>n</> is 1 or not 1, but in other
507+
languages there can be many different plural forms. The translator
508+
sees the two English forms as a group and has the opportunity to
509+
supply multiple substitute strings, with the appropriate one being
510+
selected based on the run-time value of <replaceable>n</>.
511+
</para>
512+
513+
<para>
514+
If you need to pluralize a message that isn't going directly to an
515+
<function>errmsg</> or <function>errdetail</> report, you have to use
516+
the underlying function <function>ngettext</>. See the gettext
517+
documentation.
518+
</para>
485519
</listitem>
486520

487521
<listitem>

‎doc/src/sgml/sources.sgml

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<!-- $PostgreSQL: pgsql/doc/src/sgml/sources.sgml,v 2.33 2009/04/27 16:27:36 momjian Exp $ -->
1+
<!-- $PostgreSQL: pgsql/doc/src/sgml/sources.sgml,v 2.34 2009/06/04 18:33:06 tgl Exp $ -->
22

33
<chapter id="source">
44
<title>PostgreSQL Coding Conventions</title>
@@ -181,6 +181,19 @@ ereport(ERROR,
181181
not worth expending translation effort on.
182182
</para>
183183
</listitem>
184+
<listitem>
185+
<para>
186+
<function>errmsg_plural(const char *fmt_singular, const char *fmt_plural,
187+
unsigned long n, ...)</function> is like <function>errmsg</>, but with
188+
support for various plural forms of the message.
189+
<replaceable>fmt_singular</> is the English singular format,
190+
<replaceable>fmt_plural</> is the English plural format,
191+
<replaceable>n</> is the integer value that determines which plural
192+
form is needed, and the remaining arguments are formatted according
193+
to the selected format string. For more information see
194+
<xref linkend="nls-guidelines">.
195+
</para>
196+
</listitem>
184197
<listitem>
185198
<para>
186199
<function>errdetail(const char *msg, ...)</function> supplies an optional
@@ -201,6 +214,14 @@ ereport(ERROR,
201214
sent to the client.
202215
</para>
203216
</listitem>
217+
<listitem>
218+
<para>
219+
<function>errdetail_plural(const char *fmt_singular, const char *fmt_plural,
220+
unsigned long n, ...)</function> is like <function>errdetail</>, but with
221+
support for various plural forms of the message.
222+
For more information see <xref linkend="nls-guidelines">.
223+
</para>
224+
</listitem>
204225
<listitem>
205226
<para>
206227
<function>errhint(const char *msg, ...)</function> supplies an optional
@@ -390,14 +411,14 @@ Hint: the addendum
390411
<para>
391412
There are functions in the backend that will double-quote their own output
392413
at need (for example, <function>format_type_be</>()). Do not put
393-
additional quotes around the output of such functions.
414+
additional quotes around the output of such functions.
394415
</para>
395416

396417
<para>
397418
Rationale: Objects can have names that create ambiguity when embedded in a
398419
message. Be consistent about denoting where a plugged-in name starts and
399420
ends. But don't clutter messages with unnecessary or duplicate quote
400-
marks.
421+
marks.
401422
</para>
402423

403424
</simplesect>
@@ -413,7 +434,7 @@ Hint: the addendum
413434
<para>
414435
Primary error messages: Do not capitalize the first letter. Do not end a
415436
message with a period. Do not even think about ending a message with an
416-
exclamation point.
437+
exclamation point.
417438
</para>
418439

419440
<para>
@@ -430,7 +451,7 @@ Hint: the addendum
430451
long enough to be more than one sentence, they should be split into
431452
primary and detail parts.) However, detail and hint messages are longer
432453
and might need to include multiple sentences. For consistency, they should
433-
follow complete-sentence style even when there's only one sentence.
454+
follow complete-sentence style even when there's only one sentence.
434455
</para>
435456

436457
</simplesect>
@@ -473,7 +494,7 @@ Hint: the addendum
473494
<para>
474495
Use past tense if an attempt to do something failed, but could perhaps
475496
succeed next time (perhaps after fixing some problem). Use present tense
476-
if the failure is certainly permanent.
497+
if the failure is certainly permanent.
477498
</para>
478499

479500
<para>
@@ -489,20 +510,20 @@ cannot open file "%s"
489510
message should give a reason, such as <quote>disk full</quote> or
490511
<quote>file doesn't exist</quote>. The past tense is appropriate because
491512
next time the disk might not be full anymore or the file in question might
492-
exist.
513+
exist.
493514
</para>
494515

495516
<para>
496517
The second form indicates that the functionality of opening the named file
497518
does not exist at all in the program, or that it's conceptually
498519
impossible. The present tense is appropriate because the condition will
499-
persist indefinitely.
520+
persist indefinitely.
500521
</para>
501522

502523
<para>
503524
Rationale: Granted, the average user will not be able to draw great
504525
conclusions merely from the tense of the message, but since the language
505-
provides us with a grammar we should use it correctly.
526+
provides us with a grammar we should use it correctly.
506527
</para>
507528

508529
</simplesect>
@@ -552,7 +573,7 @@ could not open file %s: %m
552573
to paste this into a single smooth sentence, so some sort of punctuation
553574
is needed. Putting the embedded text in parentheses has also been
554575
suggested, but it's unnatural if the embedded text is likely to be the
555-
most important part of the message, as is often the case.
576+
most important part of the message, as is often the case.
556577
</para>
557578

558579
</simplesect>
@@ -579,7 +600,7 @@ BETTER: could not open file %s (I/O failure)
579600
Don't include the name of the reporting routine in the error text. We have
580601
other mechanisms for finding that out when needed, and for most users it's
581602
not helpful information. If the error text doesn't make as much sense
582-
without the function name, reword it.
603+
without the function name, reword it.
583604
<programlisting>
584605
BAD: pg_atoi: error in "z": cannot parse "z"
585606
BETTER: invalid input syntax for integer: "z"
@@ -620,7 +641,7 @@ BETTER: could not open file %s: %m
620641
<para>
621642
Error messages like <quote>bad result</quote> are really hard to interpret
622643
intelligently. It's better to write why the result is <quote>bad</quote>,
623-
e.g., <quote>invalid format</quote>.
644+
e.g., <quote>invalid format</quote>.
624645
</para>
625646
</formalpara>
626647

@@ -638,7 +659,7 @@ BETTER: could not open file %s: %m
638659
Try to avoid <quote>unknown</quote>. Consider <quote>error: unknown
639660
response</quote>. If you don't know what the response is, how do you know
640661
it's erroneous? <quote>Unrecognized</quote> is often a better choice.
641-
Also, be sure to include the value being complained of.
662+
Also, be sure to include the value being complained of.
642663
<programlisting>
643664
BAD: unknown node type
644665
BETTER: unrecognized node type: 42
@@ -654,7 +675,7 @@ BETTER: unrecognized node type: 42
654675
couldn't <quote>find</quote> the resource. If, on the other hand, the
655676
expected location of the resource is known but the program cannot access
656677
it there then say that the resource doesn't <quote>exist</quote>. Using
657-
<quote>find</quote> in this case sounds weak and confuses the issue.
678+
<quote>find</quote> in this case sounds weak and confuses the issue.
658679
</para>
659680
</formalpara>
660681

‎src/backend/catalog/dependency.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/catalog/dependency.c,v 1.87 2009/03/26 22:26:06 petere Exp $
11+
* $PostgreSQL: pgsql/src/backend/catalog/dependency.c,v 1.88 2009/06/04 18:33:06 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -914,10 +914,10 @@ reportDependentObjects(const ObjectAddresses *targetObjects,
914914
{
915915
ereport(msglevel,
916916
/* translator: %d always has a value larger than 1 */
917-
(errmsg(ngettext("drop cascades to %d other object",
918-
"drop cascades to %d other objects",
919-
numReportedClient + numNotReportedClient),
920-
numReportedClient + numNotReportedClient),
917+
(errmsg_plural("drop cascades to %d other object",
918+
"drop cascades to %d other objects",
919+
numReportedClient + numNotReportedClient,
920+
numReportedClient + numNotReportedClient),
921921
errdetail("%s", clientdetail.data),
922922
errdetail_log("%s", logdetail.data)));
923923
}

‎src/backend/catalog/pg_proc.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/catalog/pg_proc.c,v 1.162 2009/03/26 22:26:06 petere Exp $
11+
* $PostgreSQL: pgsql/src/backend/catalog/pg_proc.c,v 1.163 2009/06/04 18:33:06 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -112,10 +112,10 @@ ProcedureCreate(const char *procedureName,
112112
if (parameterCount < 0 || parameterCount > FUNC_MAX_ARGS)
113113
ereport(ERROR,
114114
(errcode(ERRCODE_TOO_MANY_ARGUMENTS),
115-
errmsg(ngettext("functions cannot have more than %d argument",
116-
"functions cannot have more than %d arguments",
117-
FUNC_MAX_ARGS),
118-
FUNC_MAX_ARGS)));
115+
errmsg_plural("functions cannot have more than %d argument",
116+
"functions cannot have more than %d arguments",
117+
FUNC_MAX_ARGS,
118+
FUNC_MAX_ARGS)));
119119
/* note: the above is correct, we do NOT count output arguments */
120120

121121
if (allParameterTypes != PointerGetDatum(NULL))

‎src/backend/catalog/pg_shdepend.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/catalog/pg_shdepend.c,v 1.32 2009/03/26 22:26:06 petere Exp $
11+
* $PostgreSQL: pgsql/src/backend/catalog/pg_shdepend.c,v 1.33 2009/06/04 18:33:06 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -1049,7 +1049,10 @@ storeObjectDescription(StringInfo descs, objectType type,
10491049

10501050
case REMOTE_OBJECT:
10511051
/* translator: %s will always be "database %s" */
1052-
appendStringInfo(descs, ngettext("%d object in %s", "%d objects in %s", count), count, objdesc);
1052+
appendStringInfo(descs, ngettext("%d object in %s",
1053+
"%d objects in %s",
1054+
count),
1055+
count, objdesc);
10531056
break;
10541057

10551058
default:

‎src/backend/executor/execQual.c

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/executor/execQual.c,v 1.246 2009/04/08 21:51:38 petere Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/execQual.c,v 1.247 2009/06/04 18:33:07 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -616,10 +616,11 @@ ExecEvalVar(ExprState *exprstate, ExprContext *econtext,
616616
ereport(ERROR,
617617
(errcode(ERRCODE_DATATYPE_MISMATCH),
618618
errmsg("table row type and query-specified row type do not match"),
619-
errdetail(ngettext("Table row contains %d attribute, but query expects %d.",
620-
"Table row contains %d attributes, but query expects %d.",
621-
slot_tupdesc->natts),
622-
slot_tupdesc->natts, var_tupdesc->natts)));
619+
errdetail_plural("Table row contains %d attribute, but query expects %d.",
620+
"Table row contains %d attributes, but query expects %d.",
621+
slot_tupdesc->natts,
622+
slot_tupdesc->natts,
623+
var_tupdesc->natts)));
623624
else if (var_tupdesc->natts < slot_tupdesc->natts)
624625
needslow = true;
625626

@@ -1043,10 +1044,10 @@ init_fcache(Oid foid, FuncExprState *fcache,
10431044
if (list_length(fcache->args) > FUNC_MAX_ARGS)
10441045
ereport(ERROR,
10451046
(errcode(ERRCODE_TOO_MANY_ARGUMENTS),
1046-
errmsg(ngettext("cannot pass more than %d argument to a function",
1047-
"cannot pass more than %d arguments to a function",
1048-
FUNC_MAX_ARGS),
1049-
FUNC_MAX_ARGS)));
1047+
errmsg_plural("cannot pass more than %d argument to a function",
1048+
"cannot pass more than %d arguments to a function",
1049+
FUNC_MAX_ARGS,
1050+
FUNC_MAX_ARGS)));
10501051

10511052
/* Set up the primary fmgr lookup information */
10521053
fmgr_info_cxt(foid, &(fcache->func), fcacheCxt);
@@ -1314,10 +1315,10 @@ tupledesc_match(TupleDesc dst_tupdesc, TupleDesc src_tupdesc)
13141315
ereport(ERROR,
13151316
(errcode(ERRCODE_DATATYPE_MISMATCH),
13161317
errmsg("function return row and query-specified return row do not match"),
1317-
errdetail(ngettext("Returned row contains %d attribute, but query expects %d.",
1318-
"Returned row contains %d attributes, but query expects %d.",
1319-
src_tupdesc->natts),
1320-
src_tupdesc->natts, dst_tupdesc->natts)));
1318+
errdetail_plural("Returned row contains %d attribute, but query expects %d.",
1319+
"Returned row contains %d attributes, but query expects %d.",
1320+
src_tupdesc->natts,
1321+
src_tupdesc->natts, dst_tupdesc->natts)));
13211322

13221323
for (i = 0; i < dst_tupdesc->natts; i++)
13231324
{

0 commit comments

Comments
 (0)