Re: [HACKERS] PG_DIAG_SEVERITY and a possible bug in pq_parse_errornotice()

2016-08-26 Thread Tom Lane
Robert Haas writes: > On Fri, Aug 26, 2016 at 11:26 AM, Tom Lane wrote: >> I'm not quite sure what to do about this. It feels a tad wrong to use >> ErrorContext as the active context during HandleParallelMessages, but >> what else should we use? Maybe

Re: [HACKERS] PG_DIAG_SEVERITY and a possible bug in pq_parse_errornotice()

2016-08-26 Thread Robert Haas
On Fri, Aug 26, 2016 at 11:26 AM, Tom Lane wrote: > Or in short, this has confused edata and newedata. Valid coding would > be > oldcontext = MemoryContextSwitchTo(newedata->assoc_context); > rather than what is there. Oh, right. >>> (Note that in the sole >>>

Re: [HACKERS] PG_DIAG_SEVERITY and a possible bug in pq_parse_errornotice()

2016-08-26 Thread Tom Lane
Alvaro Herrera writes: > Tom Lane wrote: >> Any objections? Anyone want to bikeshed the field name? I considered >> PG_DIAG_SEVERITY_NONLOCALIZED and PG_DIAG_SEVERITY_ENGLISH before settling >> on PG_DIAG_SEVERITY_ASCII, but I can't say I'm in love with that. > I

Re: [HACKERS] PG_DIAG_SEVERITY and a possible bug in pq_parse_errornotice()

2016-08-26 Thread Alvaro Herrera
Tom Lane wrote: > So far as I can find, the attached is all we need to do to introduce a > new message field. (This patch doesn't address the memory-context > questions, but it does fix the localization-driven failure demonstrated > upthread.) > > Any objections? Anyone want to bikeshed the

Re: [HACKERS] PG_DIAG_SEVERITY and a possible bug in pq_parse_errornotice()

2016-08-26 Thread Tom Lane
I wrote: > After sleeping on it, I think the right answer is to introduce the new > error-message field (and not worry about 9.5). Will work on a patch > for that, unless I hear objections pretty soon. So far as I can find, the attached is all we need to do to introduce a new message field.

Re: [HACKERS] PG_DIAG_SEVERITY and a possible bug in pq_parse_errornotice()

2016-08-26 Thread Tom Lane
Robert Haas writes: > On Fri, Aug 26, 2016 at 10:35 AM, Tom Lane wrote: >> BTW, while I'm looking at this: what on god's green earth is >> ThrowErrorData doing copying the supplied data into edata->assoc_context? >> Surely it should be putting the data

Re: [HACKERS] PG_DIAG_SEVERITY and a possible bug in pq_parse_errornotice()

2016-08-26 Thread Robert Haas
On Fri, Aug 26, 2016 at 10:35 AM, Tom Lane wrote: > I wrote: >> After sleeping on it, I think the right answer is to introduce the new >> error-message field (and not worry about 9.5). Will work on a patch >> for that, unless I hear objections pretty soon. > > BTW, while I'm

Re: [HACKERS] PG_DIAG_SEVERITY and a possible bug in pq_parse_errornotice()

2016-08-26 Thread Tom Lane
I wrote: > After sleeping on it, I think the right answer is to introduce the new > error-message field (and not worry about 9.5). Will work on a patch > for that, unless I hear objections pretty soon. BTW, while I'm looking at this: what on god's green earth is ThrowErrorData doing copying the

Re: [HACKERS] PG_DIAG_SEVERITY and a possible bug in pq_parse_errornotice()

2016-08-26 Thread Tom Lane
Robert Haas writes: > 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

Re: [HACKERS] PG_DIAG_SEVERITY and a possible bug in pq_parse_errornotice()

2016-08-25 Thread Tom Lane
BTW, this example also exposes another problem, in that the report is >> ERREUR: unknown error severity >> CONTEXT: parallel worker Since, in fact, this error was thrown from leader code, that CONTEXT report is outright misleading. It's easy to figure that out in this particular case because

Re: [HACKERS] PG_DIAG_SEVERITY and a possible bug in pq_parse_errornotice()

2016-08-25 Thread Robert Haas
On Thu, Aug 25, 2016 at 1:19 PM, Tom Lane 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

Re: [HACKERS] PG_DIAG_SEVERITY and a possible bug in pq_parse_errornotice()

2016-08-25 Thread Tom Lane
Robert Haas writes: > On Thu, Aug 25, 2016 at 11:43 AM, Tom Lane wrote: >> Ooops. Indeed, that is broken: >> postgres=# select stringu1::int2 from tenk1 where unique1 = 1; >> ERREUR: unknown error severity >> CONTEXT: parallel worker > Uggh.

Re: [HACKERS] PG_DIAG_SEVERITY and a possible bug in pq_parse_errornotice()

2016-08-25 Thread Robert Haas
On Thu, Aug 25, 2016 at 11:43 AM, Tom Lane wrote: > Ooops. Indeed, that is broken: > > postgres=# select 1/0; -- using French locale > ERREUR: division par zéro > postgres=# set force_parallel_mode=1; > SET > postgres=# select stringu1::int2 from tenk1 where unique1 = 1; >

Re: [HACKERS] PG_DIAG_SEVERITY and a possible bug in pq_parse_errornotice()

2016-08-25 Thread Tom Lane
Jakob Egger writes: > My PostgreSQL client checks the PG_DIAG_SEVERITY error field to determine the > error level. > However, I have now learned that this field is localized. This means that a > server configured with --enable-nls might for example return the string >

[HACKERS] PG_DIAG_SEVERITY and a possible bug in pq_parse_errornotice()

2016-08-25 Thread Jakob Egger
Hi, My PostgreSQL client checks the PG_DIAG_SEVERITY error field to determine the error level. However, I have now learned that this field is localized. This means that a server configured with --enable-nls might for example return the string ERREUR instead of ERROR. So if I want to