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 the error could only have come from the leader
side, but errors reported from further down inside pq_parse_errornotice
(eg, out-of-memory in pstrdup) would not be so easy.

We could reduce this problem by narrowing the scope over which 
HandleParallelMessage activates the ParallelErrorContext callback.
But I'm inclined to get rid of that callback entirely and instead
do something like this:

                /* Parse ErrorResponse or NoticeResponse. */
                pq_parse_errornotice(msg, &edata);

                /* Death of a worker isn't enough justification for suicide. */
                edata.elevel = Min(edata.elevel, ERROR);

+               /* If desired, tag the message with context. */
+               if (force_parallel_mode != FORCE_PARALLEL_REGRESS)
+               {
+                   if (edata.context)
+                       edata.context = psprintf("%s\n%s", edata.context,
+                                                _("parallel worker"));
+                   else
+                       edata.context = pstrdup(_("parallel worker"));
+               }
+
                /* Rethrow error or notice. */
                ThrowErrorData(&edata);

This knows a little bit more than before about the conventions for
combining context strings, but we can be sure that "parallel worker"
will be appended to exactly the messages that come back from the
parallel worker, not anything else.

Barring objections I'll push something like this tomorrow.

                        regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to