Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit ab77a5a

Browse files
committed
Mark a query's topmost Paths parallel-unsafe if they will have initPlans.
Andreas Seltenreich found another case where we were being too optimistic about allowing a plan to be considered parallelizable despite it containing initPlans. It seems like the real issue here is that if we know we are going to tack initPlans onto the topmost Plan node for a subquery, we had better mark that subquery's result Paths as not-parallel-safe. That fixes this problem and allows reversion of a kluge (added in commit 7b67a0a and extended in f24cf96) to not trust the parallel_safe flag at top level. Discussion: <874m2w4k5d.fsf@ex.ansel.ydns.eu>
1 parent 4e026b3 commit ab77a5a

File tree

2 files changed

+12
-12
lines changed

2 files changed

+12
-12
lines changed

src/backend/optimizer/plan/planner.c

+6-9
Original file line numberDiff line numberDiff line change
@@ -341,12 +341,9 @@ standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams)
341341
* Optionally add a Gather node for testing purposes, provided this is
342342
* actually a safe thing to do. (Note: we assume adding a Material node
343343
* above did not change the parallel safety of the plan, so we can still
344-
* rely on best_path->parallel_safe. However, that flag doesn't account
345-
* for subplans, which we are unable to transmit to workers presently.)
344+
* rely on best_path->parallel_safe.)
346345
*/
347-
if (force_parallel_mode != FORCE_PARALLEL_OFF &&
348-
best_path->parallel_safe &&
349-
glob->subplans == NIL)
346+
if (force_parallel_mode != FORCE_PARALLEL_OFF && best_path->parallel_safe)
350347
{
351348
Gather *gather = makeNode(Gather);
352349

@@ -801,10 +798,10 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
801798
SS_identify_outer_params(root);
802799

803800
/*
804-
* If any initPlans were created in this query level, increment the
805-
* surviving Paths' costs to account for them. They won't actually get
806-
* attached to the plan tree till create_plan() runs, but we want to be
807-
* sure their costs are included now.
801+
* If any initPlans were created in this query level, adjust the surviving
802+
* Paths' costs and parallel-safety flags to account for them. The
803+
* initPlans won't actually get attached to the plan tree till
804+
* create_plan() runs, but we must include their effects now.
808805
*/
809806
final_rel = fetch_upper_rel(root, UPPERREL_FINAL, NULL);
810807
SS_charge_for_initplans(root, final_rel);

src/backend/optimizer/plan/subselect.c

+6-3
Original file line numberDiff line numberDiff line change
@@ -2128,11 +2128,13 @@ SS_identify_outer_params(PlannerInfo *root)
21282128
}
21292129

21302130
/*
2131-
* SS_charge_for_initplans - account for cost of initplans in Path costs
2131+
* SS_charge_for_initplans - account for initplans in Path costs & parallelism
21322132
*
21332133
* If any initPlans have been created in the current query level, they will
21342134
* get attached to the Plan tree created from whichever Path we select from
2135-
* the given rel; so increment all the rel's Paths' costs to account for them.
2135+
* the given rel. Increment all that rel's Paths' costs to account for them,
2136+
* and make sure the paths get marked as parallel-unsafe, since we can't
2137+
* currently transmit initPlans to parallel workers.
21362138
*
21372139
* This is separate from SS_attach_initplans because we might conditionally
21382140
* create more initPlans during create_plan(), depending on which Path we
@@ -2164,14 +2166,15 @@ SS_charge_for_initplans(PlannerInfo *root, RelOptInfo *final_rel)
21642166
}
21652167

21662168
/*
2167-
* Now adjust the costs.
2169+
* Now adjust the costs and parallel_safe flags.
21682170
*/
21692171
foreach(lc, final_rel->pathlist)
21702172
{
21712173
Path *path = (Path *) lfirst(lc);
21722174

21732175
path->startup_cost += initplan_cost;
21742176
path->total_cost += initplan_cost;
2177+
path->parallel_safe = false;
21752178
}
21762179

21772180
/* We needn't do set_cheapest() here, caller will do it */

0 commit comments

Comments
 (0)