Re: PG_DIAG_SEVERITY and a possible bug in pq_parse_errornotice()

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Thu, Aug 25, 2016 at 11:43 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Ooops. Indeed, that is broken:
>> postgres=# select stringu1::int2 from tenk1 where unique1 = 1;
>> ERREUR: unknown error severity
>> CONTEXT: parallel worker

> Uggh. Obviously, I failed to realize that those strings were
> localized. Leaving aside the question of this particular matching
> problem, I wonder if we are localizing everything twice right now,
> once in the worker and once in the leader.

Hm. It's possible we're passing already-localized strings through
gettext a second time in the leader, but that basically does nothing
(unless somehow you got a match, but the probability seems tiny).
I rather doubt that is happening though, because of the next point:

> It's probably best to try
> to hack things somehow so that the worker localizes nothing and the
> leader localizes everything.

No way that's gonna work. For example, the expected report in
English for the example above is
ERROR: invalid input syntax for integer: "BAAAAA"
That doesn't match anything in gettext's database, because we
already substituted something for the %s. Basically, localization
always has to happen before error message construction, not later.

> Or we could add another field to the
> message the worker sends that includes the error level as an integer.

The two alternatives that seem reasonable to me after a bit of reflection
are:

1. Hack elog.c to not localize the severity field when in a worker
process. (It'd still have to localize all the other fields, because
of the %s problem.) This would be a very localized fix, but it still
seems mighty ugly.

2. Add another field to error messages, which would be a never-localized
copy of the severity string. This might be the best long-run solution,
especially if Jakob can present a convincing argument why clients might
want it. We would be taking some small chance of breaking existing
clients, but if it only happens in a new major release (ie 9.6) then
that doesn't seem like a problem. And anyway the protocol spec has
always counseled clients to be prepared to ignore unrecognized fields
in an error message.

We could do a variant 2a in which we invent an additional field but
only allow workers to send it, which is more or less the same as your
idea (though I'd still prefer string to integer). I don't find that
very attractive though. If we're going to hack elog.c to have different
sending behavior in a worker, we might as well do #1.

regards, tom lane