On Thu, Apr 13, 2023 at 12:43 AM Tom Lane <t...@sss.pgh.pa.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
