diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2023-07-13 17:30:14 -0400 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2023-07-14 11:41:20 -0400 |
commit | e08d74ca1342cb9e6047daad2019b550ecf54877 (patch) | |
tree | c9de9616844e56447c9f9a9c2857d462b4b97ea8 /src/backend/optimizer/plan/planner.c | |
parent | d0d44049d1262aed2eee906d26af852948206db0 (diff) |
Allow plan nodes with initPlans to be considered parallel-safe.
If the plan itself is parallel-safe, and the initPlans are too,
there's no reason anymore to prevent the plan from being marked
parallel-safe. That restriction (dating to commit ab77a5a45) 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.
We still forbid attaching initPlans to a Gather node that's
inserted pursuant to debug_parallel_query = regress. That's because,
when we hide the Gather from EXPLAIN output, we'd hide the initPlans
too, causing cosmetic regression diffs. It seems inadvisable to
kluge EXPLAIN to the extent required to make the output look the
same, so just don't do it in that case.
Along the way, this also takes care of some sloppiness about updating
path costs to match when we move initplans from one place to another
during createplan.c and setrefs.c. Since all the planning decisions
are already made by that point, this is just cosmetic; but it seems
good to keep EXPLAIN output consistent with where the initplans are.
The diff in query_planner() might be worth remarking on. I found that
one because after fixing things to allow parallel-safe initplans, one
partition_prune test case changed plans (as shown in the patch) ---
but only when debug_parallel_query was active. The reason proved to
be that we only bothered to mark Result nodes as potentially
parallel-safe when debug_parallel_query is on. This neglects the fact
that parallel-safety may be of interest for a sub-query even though
the Result itself doesn't parallelize.
Discussion: https://postgr.es/m/1129530.1681317832@sss.pgh.pa.us
Diffstat (limited to 'src/backend/optimizer/plan/planner.c')
-rw-r--r-- | src/backend/optimizer/plan/planner.c | 36 |
1 files changed, 28 insertions, 8 deletions
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 0e12fdeb602..44efb1f4ebc 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -432,16 +432,23 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions, /* * Optionally add a Gather node for testing purposes, provided this is * actually a safe thing to do. - */ - if (debug_parallel_query != DEBUG_PARALLEL_OFF && top_plan->parallel_safe) + * + * We can add Gather even when top_plan has parallel-safe initPlans, but + * then we have to move the initPlans to the Gather node because of + * SS_finalize_plan's limitations. That would cause cosmetic breakage of + * regression tests when debug_parallel_query = regress, because initPlans + * that would normally appear on the top_plan move to the Gather, causing + * them to disappear from EXPLAIN output. That doesn't seem worth kluging + * EXPLAIN to hide, so skip it when debug_parallel_query = regress. + */ + if (debug_parallel_query != DEBUG_PARALLEL_OFF && + top_plan->parallel_safe && + (top_plan->initPlan == NIL || + debug_parallel_query != DEBUG_PARALLEL_REGRESS)) { Gather *gather = makeNode(Gather); - - /* - * Top plan must not have any initPlans, else it shouldn't have been - * marked parallel-safe. - */ - Assert(top_plan->initPlan == NIL); + Cost initplan_cost; + bool unsafe_initplans; gather->plan.targetlist = top_plan->targetlist; gather->plan.qual = NIL; @@ -451,6 +458,10 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions, gather->single_copy = true; gather->invisible = (debug_parallel_query == DEBUG_PARALLEL_REGRESS); + /* Transfer any initPlans to the new top node */ + gather->plan.initPlan = top_plan->initPlan; + top_plan->initPlan = NIL; + /* * Since this Gather has no parallel-aware descendants to signal to, * we don't need a rescan Param. @@ -470,6 +481,15 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions, gather->plan.parallel_aware = false; gather->plan.parallel_safe = false; + /* + * Delete the initplans' cost from top_plan. We needn't add it to the + * Gather node, since the above coding already included it there. + */ + SS_compute_initplan_cost(gather->plan.initPlan, + &initplan_cost, &unsafe_initplans); + top_plan->startup_cost -= initplan_cost; + top_plan->total_cost -= initplan_cost; + /* use parallel mode for parallel plans. */ root->glob->parallelModeNeeded = true; |