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