Re: Allowing parallel-safe initplans
| От | Richard Guo |
|---|---|
| Тема | Re: Allowing parallel-safe initplans |
| Дата | |
| Msg-id | CAMbWs4-ED_ga2v0SbjKWV4Ay2GSVqtK_6xiG3iHKH-sHcCDs3A@mail.gmail.com обсуждение исходный текст |
| Ответ на | Re: Allowing parallel-safe initplans (Tom Lane <tgl@sss.pgh.pa.us>) |
| Ответы |
Re: Allowing parallel-safe initplans
|
| Список | pgsql-hackers |
On Mon, Apr 17, 2023 at 11:04 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Richard Guo <guofenglinux@gmail.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.
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
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
В списке pgsql-hackers по дате отправления: