From: | Richard Guo <guofenglinux(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: Allowing parallel-safe initplans |
Date: | 2023-04-18 07:14:01 |
Message-ID: | CAMbWs4-ED_ga2v0SbjKWV4Ay2GSVqtK_6xiG3iHKH-sHcCDs3A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Apr 17, 2023 at 11:04 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Richard Guo <guofenglinux(at)gmail(dot)com> writes:
> > 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.
>
> I wondered about that too, but how come neither of us saw non-cosmetic
> failures (ie, actual query output changes not just EXPLAIN changes)
> when we tried this? Maybe the case is somehow not exercised, but if
> so I'm more worried about adding regression tests than comments.
Sorry I forgot to mention that I did see query output changes after
moving the initPlans to the Gather node. First of all let me make sure
I was doing it in the right way. On the base of the patch, I was using
the diff as below
if (debug_parallel_query != DEBUG_PARALLEL_OFF &&
- top_plan->parallel_safe && top_plan->initPlan == NIL)
+ top_plan->parallel_safe)
{
Gather *gather = makeNode(Gather);
+ gather->plan.initPlan = top_plan->initPlan;
+ top_plan->initPlan = NIL;
+
gather->plan.targetlist = top_plan->targetlist;
And then I changed the default value of debug_parallel_query to
DEBUG_PARALLEL_REGRESS. And then I just ran 'make installcheck' and saw
the query output changes.
> I think actually that it does work beyond the EXPLAIN weirdness,
> because since e89a71fb4 the Gather machinery knows how to transmit
> the values of Params listed in Gather.initParam to workers, and that
> is filled in setrefs.c in a way that looks like it'd work regardless
> of whether the Gather appeared organically or was stuck on by the
> debug_parallel_query hackery. I've not tried to verify that
> directly though.
It seems that in this case the top_plan does not have any extParam, so
the Gather node that is added atop the top_plan does not have a chance
to get its initParam filled in set_param_references().
Thanks
Richard
From | Date | Subject | |
---|---|---|---|
Next Message | sirisha chamarthi | 2023-04-18 08:46:21 | Re: Fix documentation for max_wal_size and min_wal_size |
Previous Message | Peter Geoghegan | 2023-04-18 06:53:17 | Re: Should we put command options in alphabetical order in the doc? |