Fix logic for adding "parallel worker" context line to worker errors.

The previous coding here was capable of adding a "parallel worker" context
line to errors that were not, in fact, returned from a parallel worker.
Instead of using an errcontext callback to add that annotation, just paste
it onto the message by hand; this looks uglier but is more reliable.

Discussion: <19757.1472151987@sss.pgh.pa.us>

diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c
index a47eba647bcba5e5b8ce7841cf376e7921c369b3..ec6e1c5e6dc20844e5f8d9a463133b1a2ccd75b9 100644 (file)
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -108,7 +108,6 @@ static dlist_head pcxt_list = DLIST_STATIC_INIT(pcxt_list);
 
 /* Private functions. */
 static void HandleParallelMessage(ParallelContext *pcxt, int i, StringInfo msg);
-static void ParallelErrorContext(void *arg);
 static void ParallelExtensionTrampoline(dsm_segment *seg, shm_toc *toc);
 static void ParallelWorkerMain(Datum main_arg);
 static void WaitForParallelWorkersToExit(ParallelContext *pcxt);
@@ -788,30 +787,43 @@ HandleParallelMessage(ParallelContext *pcxt, int i, StringInfo msg)
        case 'N':               /* NoticeResponse */
            {
                ErrorData   edata;
-               ErrorContextCallback errctx;
                ErrorContextCallback *save_error_context_stack;
 
-               /*
-                * Rethrow the error using the error context callbacks that
-                * were in effect when the context was created, not the
-                * current ones.
-                */
-               save_error_context_stack = error_context_stack;
-               errctx.callback = ParallelErrorContext;
-               errctx.arg = NULL;
-               errctx.previous = pcxt->error_context_stack;
-               error_context_stack = &errctx;
-
                /* 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);
 
-               /* Rethrow error or notice. */
+               /*
+                * If desired, add a context line to show that this is a
+                * message propagated from a parallel worker.  Otherwise, it
+                * can sometimes be confusing to understand what actually
+                * happened.  (We don't do this in FORCE_PARALLEL_REGRESS mode
+                * because it causes test-result instability depending on
+                * whether a parallel worker is actually used or not.)
+                */
+               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"));
+               }
+
+               /*
+                * Context beyond that should use the error context callbacks
+                * that were in effect when the ParallelContext was created,
+                * not the current ones.
+                */
+               save_error_context_stack = error_context_stack;
+               error_context_stack = pcxt->error_context_stack;
+
+               /* Rethrow error or print notice. */
                ThrowErrorData(&edata);
 
-               /* Restore previous context. */
+               /* Not an error, so restore previous context stack. */
                error_context_stack = save_error_context_stack;
 
                break;
@@ -1112,18 +1124,6 @@ ParallelExtensionTrampoline(dsm_segment *seg, shm_toc *toc)
    entrypt(seg, toc);
 }
 
-/*
- * Give the user a hint that this is a message propagated from a parallel
- * worker.  Otherwise, it can sometimes be confusing to understand what
- * actually happened.
- */
-static void
-ParallelErrorContext(void *arg)
-{
-   if (force_parallel_mode != FORCE_PARALLEL_REGRESS)
-       errcontext("parallel worker");
-}
-
 /*
  * Update shared memory with the ending location of the last WAL record we
  * wrote, if it's greater than the value already stored there.