Re: PG_DIAG_SEVERITY and a possible bug in pq_parse_errornotice()

On Thu, Aug 25, 2016 at 1:19 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> 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.

Oh, right.

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

I don't have strong feelings about this. Technically, this issue
affects 9.5 also, because pqmq.c was introduced in that release. I
don't think we want to add another error field in a released branch.
However, since there's no parallel query in 9.5, only people who are
accessing that functionality via extension code would be affected,
which might be nobody and certainly isn't a lot of people, so we could
just leave this unfixed in 9.5.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company