RE: Incorrect CONTEXT reported for errors from parallel apply worker in logical replication

Dear Vignesh,

> While reviewing another thread at [1], I found an issue where an error
> relayed from a parallel apply worker can get an unrelated CONTEXT line
> from the leader process.
> ProcessParallelApplyMessage() in applyparallelworker.c sets
> error_context_stack to the leader's apply_error_context_stack before
> calling ereport(ERROR). As a result, when errfinish() processes the
> error, the leader's error context callback runs again and adds the
> leader's current replication context. This can be incorrect if the
> leader is processing a different transaction when it receives the
> error from the parallel worker.

Good catch and agreed your analysis.

I checked other examples in core, and parallel query seems to handle correctly.
It has an attribute ParallelContext::error_context_stack which preserves an error
context at that time, see CreateParallelContext(). When the parallel worker raises
an ERROR, the leader backend can consume its message and raise it again, and at
that time the error context is restored from the ParallelContext.

I think the easiest fix is to use apply_error_context_stack for preserving the
error context before entering the loop, attached patch implements the idea.
IIUC no contexts can be stacked at the begining of the worker, i.e.,
apply_error_context_stack can be NULL in any case. So the global variable can be
removed if we clean up more aggressively. I also felt the name can be changed,
but it's retained for now because the variable has been exposed.

BTW, I also checked the REPACK CONCURRENTLY, but it seemed to have the same issue
as the parallel apply. I locally found a reproducer and wrote a fix patch,
I can share here or another place.

Best regards,
Hayato Kuroda
FUJITSU LIMITED