Skip to content

Commit 26fa446

Browse files
tglsfdctglsfdc
committed
Add a nonlocalized version of the severity field to client error messages.
This has been requested a few times, but the use-case for it was never entirely clear. The reason for adding it now is that transmission of error reports from parallel workers fails when NLS is active, because pq_parse_errornotice() wrongly assumes that the existing severity field is nonlocalized. There are other ways we could have fixed that, but the other options were basically kluges, whereas this way provides something that's at least arguably a useful feature along with the bug fix. Per report from Jakob Egger. Back-patch into 9.6, because otherwise parallel query is essentially unusable in non-English locales. The problem exists in 9.5 as well, but we don't want to risk changing on-the-wire behavior in 9.5 (even though the possibility of new error fields is specifically called out in the protocol document). It may be sufficient to leave the issue unfixed in 9.5, given the very limited usefulness of pq_parse_errornotice in that version. Discussion: <A88E0006-13CB-49C6-95CC-1A77D717213C@eggerapps.at>
1 parent 78dcd02 commit 26fa446

File tree

6 files changed

+78
-18
lines changed

6 files changed

+78
-18
lines changed

‎doc/src/sgml/libpq.sgml

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2767,6 +2767,22 @@ char *PQresultErrorField(const PGresult *res, int fieldcode);
27672767
</listitem>
27682768
</varlistentry>
27692769

2770+
<varlistentry id="libpq-pg-diag-severity-nonlocalized">
2771+
<term><symbol>PG_DIAG_SEVERITY_NONLOCALIZED</></term>
2772+
<listitem>
2773+
<para>
2774+
The severity; the field contents are <literal>ERROR</>,
2775+
<literal>FATAL</>, or <literal>PANIC</> (in an error message),
2776+
or <literal>WARNING</>, <literal>NOTICE</>, <literal>DEBUG</>,
2777+
<literal>INFO</>, or <literal>LOG</> (in a notice message).
2778+
This is identical to the <symbol>PG_DIAG_SEVERITY</> field except
2779+
that the contents are never localized. This is present only in
2780+
reports generated by <productname>PostgreSQL</> versions 9.6
2781+
and later.
2782+
</para>
2783+
</listitem>
2784+
</varlistentry>
2785+
27702786
<varlistentry id="libpq-pg-diag-sqlstate">
27712787
<term>
27722788
<symbol>PG_DIAG_SQLSTATE</>

‎doc/src/sgml/protocol.sgml

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4882,6 +4882,25 @@ message.
48824882
</listitem>
48834883
</varlistentry>
48844884

4885+
<varlistentry>
4886+
<term>
4887+
<literal>V</>
4888+
</term>
4889+
<listitem>
4890+
<para>
4891+
Severity: the field contents are
4892+
<literal>ERROR</>, <literal>FATAL</>, or
4893+
<literal>PANIC</> (in an error message), or
4894+
<literal>WARNING</>, <literal>NOTICE</>, <literal>DEBUG</>,
4895+
<literal>INFO</>, or <literal>LOG</> (in a notice message).
4896+
This is identical to the <literal>S</> field except
4897+
that the contents are never localized. This is present only in
4898+
messages generated by <productname>PostgreSQL</> versions 9.6
4899+
and later.
4900+
</para>
4901+
</listitem>
4902+
</varlistentry>
4903+
48854904
<varlistentry>
48864905
<term>
48874906
<literal>C</>

‎src/backend/libpq/pqmq.c

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -237,10 +237,26 @@ pq_parse_errornotice(StringInfo msg, ErrorData *edata)
237237
switch (code)
238238
{
239239
case PG_DIAG_SEVERITY:
240+
/* ignore, trusting we'll get a nonlocalized version */
241+
break;
242+
case PG_DIAG_SEVERITY_NONLOCALIZED:
240243
if (strcmp(value, "DEBUG") == 0)
241-
edata->elevel = DEBUG1; /* or some other DEBUG level */
244+
{
245+
/*
246+
* We can't reconstruct the exact DEBUG level, but
247+
* presumably it was >= client_min_messages, so select
248+
* DEBUG1 to ensure we'll pass it on to the client.
249+
*/
250+
edata->elevel = DEBUG1;
251+
}
242252
else if (strcmp(value, "LOG") == 0)
243-
edata->elevel = LOG; /* can't be COMMERROR */
253+
{
254+
/*
255+
* It can't be LOG_SERVER_ONLY, or the worker wouldn't
256+
* have sent it to us; so LOG is the correct value.
257+
*/
258+
edata->elevel = LOG;
259+
}
244260
else if (strcmp(value, "INFO") == 0)
245261
edata->elevel = INFO;
246262
else if (strcmp(value, "NOTICE") == 0)
@@ -254,11 +270,11 @@ pq_parse_errornotice(StringInfo msg, ErrorData *edata)
254270
else if (strcmp(value, "PANIC") == 0)
255271
edata->elevel = PANIC;
256272
else
257-
elog(ERROR, "unknown error severity");
273+
elog(ERROR, "unrecognized error severity: \"%s\"", value);
258274
break;
259275
case PG_DIAG_SQLSTATE:
260276
if (strlen(value) != 5)
261-
elog(ERROR, "malformed sql state");
277+
elog(ERROR, "invalid SQLSTATE: \"%s\"", value);
262278
edata->sqlerrcode = MAKE_SQLSTATE(value[0], value[1], value[2],
263279
value[3], value[4]);
264280
break;
@@ -308,7 +324,7 @@ pq_parse_errornotice(StringInfo msg, ErrorData *edata)
308324
edata->funcname = pstrdup(value);
309325
break;
310326
default:
311-
elog(ERROR, "unknown error field: %d", (int) code);
327+
elog(ERROR, "unrecognized error field code: %d", (int) code);
312328
break;
313329
}
314330
}

‎src/backend/utils/error/elog.c

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2753,7 +2753,7 @@ write_csvlog(ErrorData *edata)
27532753
appendStringInfoChar(&buf, ',');
27542754

27552755
/* Error severity */
2756-
appendStringInfoString(&buf, error_severity(edata->elevel));
2756+
appendStringInfoString(&buf, _(error_severity(edata->elevel)));
27572757
appendStringInfoChar(&buf, ',');
27582758

27592759
/* SQL state code */
@@ -2870,7 +2870,7 @@ send_message_to_server_log(ErrorData *edata)
28702870
formatted_log_time[0] = '\0';
28712871

28722872
log_line_prefix(&buf, edata);
2873-
appendStringInfo(&buf, "%s: ", error_severity(edata->elevel));
2873+
appendStringInfo(&buf, "%s: ", _(error_severity(edata->elevel)));
28742874

28752875
if (Log_error_verbosity >= PGERROR_VERBOSE)
28762876
appendStringInfo(&buf, "%s: ", unpack_sql_state(edata->sqlerrcode));
@@ -3153,12 +3153,16 @@ send_message_to_frontend(ErrorData *edata)
31533153
if (PG_PROTOCOL_MAJOR(FrontendProtocol) >= 3)
31543154
{
31553155
/* New style with separate fields */
3156+
const char *sev;
31563157
char tbuf[12];
31573158
int ssval;
31583159
int i;
31593160

3161+
sev = error_severity(edata->elevel);
31603162
pq_sendbyte(&msgbuf, PG_DIAG_SEVERITY);
3161-
err_sendstring(&msgbuf, error_severity(edata->elevel));
3163+
err_sendstring(&msgbuf, _(sev));
3164+
pq_sendbyte(&msgbuf, PG_DIAG_SEVERITY_NONLOCALIZED);
3165+
err_sendstring(&msgbuf, sev);
31623166

31633167
/* unpack MAKE_SQLSTATE code */
31643168
ssval = edata->sqlerrcode;
@@ -3277,7 +3281,7 @@ send_message_to_frontend(ErrorData *edata)
32773281

32783282
initStringInfo(&buf);
32793283

3280-
appendStringInfo(&buf, "%s: ", error_severity(edata->elevel));
3284+
appendStringInfo(&buf, "%s: ", _(error_severity(edata->elevel)));
32813285

32823286
if (edata->show_funcname && edata->funcname)
32833287
appendStringInfo(&buf, "%s: ", edata->funcname);
@@ -3587,7 +3591,10 @@ get_errno_symbol(int errnum)
35873591

35883592

35893593
/*
3590-
* error_severity --- get localized string representing elevel
3594+
* error_severity --- get string representing elevel
3595+
*
3596+
* The string is not localized here, but we mark the strings for translation
3597+
* so that callers can invoke _() on the result.
35913598
*/
35923599
static const char *
35933600
error_severity(int elevel)
@@ -3601,29 +3608,29 @@ error_severity(int elevel)
36013608
case DEBUG3:
36023609
case DEBUG4:
36033610
case DEBUG5:
3604-
prefix = _("DEBUG");
3611+
prefix = gettext_noop("DEBUG");
36053612
break;
36063613
case LOG:
36073614
case LOG_SERVER_ONLY:
3608-
prefix = _("LOG");
3615+
prefix = gettext_noop("LOG");
36093616
break;
36103617
case INFO:
3611-
prefix = _("INFO");
3618+
prefix = gettext_noop("INFO");
36123619
break;
36133620
case NOTICE:
3614-
prefix = _("NOTICE");
3621+
prefix = gettext_noop("NOTICE");
36153622
break;
36163623
case WARNING:
3617-
prefix = _("WARNING");
3624+
prefix = gettext_noop("WARNING");
36183625
break;
36193626
case ERROR:
3620-
prefix = _("ERROR");
3627+
prefix = gettext_noop("ERROR");
36213628
break;
36223629
case FATAL:
3623-
prefix = _("FATAL");
3630+
prefix = gettext_noop("FATAL");
36243631
break;
36253632
case PANIC:
3626-
prefix = _("PANIC");
3633+
prefix = gettext_noop("PANIC");
36273634
break;
36283635
default:
36293636
prefix = "???";

‎src/include/postgres_ext.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ typedef PG_INT64_TYPE pg_int64;
4949
* applications.
5050
*/
5151
#define PG_DIAG_SEVERITY 'S'
52+
#define PG_DIAG_SEVERITY_NONLOCALIZED 'V'
5253
#define PG_DIAG_SQLSTATE 'C'
5354
#define PG_DIAG_MESSAGE_PRIMARY 'M'
5455
#define PG_DIAG_MESSAGE_DETAIL 'D'

‎src/interfaces/libpq/fe-exec.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -824,6 +824,7 @@ pqInternalNotice(const PGNoticeHooks *hooks, const char *fmt,...)
824824
*/
825825
pqSaveMessageField(res, PG_DIAG_MESSAGE_PRIMARY, msgBuf);
826826
pqSaveMessageField(res, PG_DIAG_SEVERITY, libpq_gettext("NOTICE"));
827+
pqSaveMessageField(res, PG_DIAG_SEVERITY_NONLOCALIZED, "NOTICE");
827828
/* XXX should provide a SQLSTATE too? */
828829

829830
/*

0 commit comments

Comments
 (0)