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-13 08:23:06 |
Message-ID: | CAMbWs4_7hFTDjYzJs6sYpceY_0MFRA+DhdKxAMOREP084vr2pA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Apr 13, 2023 at 12:43 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Pursuant to the discussion at [1], here's a patch that removes our
> old restriction that a plan node having initPlans can't be marked
> parallel-safe (dating to commit ab77a5a45). That was really a special
> case of the fact that we couldn't transmit subplans to parallel
> workers at all. We fixed that in commit 5e6d8d2bb and follow-ons,
> but this case never got addressed.
The patch looks good to me. Some comments from me:
* For the diff in standard_planner, I was wondering why not move the
initPlans up to the Gather node, just as we did before. So I tried that
way but did not notice the breakage of regression tests as stated in the
comments. Would you please confirm that?
* Not related to this patch. In SS_make_initplan_from_plan, the comment
says that the node's parParam and args lists remain empty. I wonder if
we need to explicitly set node->parParam and node->args to NIL before
that comment, or can we depend on makeNode to initialize them to NIL?
> There's only one existing test case that visibly changes plan with
> these changes. The new plan is clearly saner-looking than before,
> and testing with some data loaded into the table confirms that it
> is faster. I'm not sure if it's worth devising more test cases.
I also think it's better to have more test cases covering this change.
Thanks
Richard
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Eisentraut | 2023-04-13 08:31:43 | Re: doc: add missing "id" attributes to extension packaging page |
Previous Message | Bruce Momjian | 2023-04-13 06:50:36 | Re: Partial aggregates pushdown |