Horton

Re: Allowing parallel-safe initplans

Richard Guo <guofenglinux@gmail.com>

View thread View on postgr.es
From: Richard Guo <guofenglinux@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@lists.postgresql.org
Date: 2023-04-17T07:26:46Z
Lists: pgsql-hackers 
On Mon, Apr 17, 2023 at 10:57 AM Richard Guo <guofenglinux@gmail.com> wrote:

> The initPlan has been moved from the Result node to the Gather node.  As
> a result, when doing tuple projection for the Result node, we'd get a
> ParamExecData entry with NULL execPlan.  So the initPlan does not get
> chance to be executed.  And we'd get the output as the default value
> from the ParamExecData entry, which is zero as shown.
>
> So now I begin to wonder if this wrong result issue is possible to exist
> in other places where we move initPlans.  But I haven't tried hard to
> verify that.
>

I looked further into this issue and I believe other places are good.
The problem with this query is that the es/ecxt_param_exec_vals used to
store info about the initplan is not the same one as in the Result
node's expression context for projection, because we've forked a new
process for the parallel worker and then created and initialized a new
EState node, and allocated a new es_param_exec_vals array for the new
EState.  When doing projection for the Result node, the current code
just goes ahead and accesses the new es_param_exec_vals, thus fails to
retrieve the info about the initplan.  Hmm, I doubt this is sensible.

So now it seems that the breakage of regression tests is more severe
than being cosmetic.  I wonder if we need to update the comments to
indicate the potential wrong results issue if we move the initPlans to
the Gather node.

Thanks
Richard

Commits

  1. Account for optimized MinMax aggregates during SS_finalize_plan.

  2. Add test case showing that commit d0d44049d fixed a live bug.

  3. Allow plan nodes with initPlans to be considered parallel-safe.

  4. Allow parallel workers to execute subplans.

  5. Mark a query's topmost Paths parallel-unsafe if they will have initPlans.