Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitf4badbc

Browse files
committed
Fix parallel-safety marking when moving initplans to another node.
Our policy since commitab77a5a has been that a plan node havingany initplans is automatically not parallel-safe. (This could berelaxed, but not today.) clean_up_removed_plan_level neglectedthis, and could attach initplans to a parallel-safe child plannode without clearing the plan's parallel-safe flag. That couldlead to "subplan was not initialized" errors at runtime, in casean initplan referenced another one and only the referencing onegot transmitted to parallel workers.The fix in clean_up_removed_plan_level is trivial enough.materialize_finished_plan also moves initplans from one nodeto another, but it's okay because it already copies the sourcenode's parallel_safe flag. The other place that does this kindof thing is standard_planner's hack to inject a top-level Gatherwhen debug_parallel_query is active. But that's actually deadcode given that we're correctly enforcing the "initplans aren'tparallel safe" rule, so just replace it with an Assert thatthere are no initplans.Also improve some related comments.Normally we'd add a regression test case for this sort of bug.The mistake itself is already reached by existing tests, but thereis accidentally no visible problem. The only known test case thatcreates an actual failure seems too indirect and fragile to justifykeeping it as a regression test (not least because it fails to failin v11, though the bug is clearly present there too).Per report from Justin Pryzby. Back-patch to all supported branches.Discussion:https://postgr.es/m/ZDVt6MaNWkRDO1LQ@telsasoft.com
1 parent5c32549 commitf4badbc

File tree

4 files changed

+22
-10
lines changed

4 files changed

+22
-10
lines changed

‎src/backend/optimizer/plan/planner.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -431,12 +431,10 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions,
431431
Gather*gather=makeNode(Gather);
432432

433433
/*
434-
* If there are any initPlans attached to the formerly-top plan node,
435-
* move them up to the Gather node; same as we do for Material node in
436-
* materialize_finished_plan.
434+
* Top plan must not have any initPlans, else it shouldn't have been
435+
* marked parallel-safe.
437436
*/
438-
gather->plan.initPlan=top_plan->initPlan;
439-
top_plan->initPlan=NIL;
437+
Assert(top_plan->initPlan==NIL);
440438

441439
gather->plan.targetlist=top_plan->targetlist;
442440
gather->plan.qual=NIL;

‎src/backend/optimizer/plan/setrefs.c

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1440,7 +1440,19 @@ trivial_subqueryscan(SubqueryScan *plan)
14401440
staticPlan*
14411441
clean_up_removed_plan_level(Plan*parent,Plan*child)
14421442
{
1443-
/* We have to be sure we don't lose any initplans */
1443+
/*
1444+
* We have to be sure we don't lose any initplans, so move any that were
1445+
* attached to the parent plan to the child. If we do move any, the child
1446+
* is no longer parallel-safe.
1447+
*/
1448+
if (parent->initPlan)
1449+
child->parallel_safe= false;
1450+
1451+
/*
1452+
* Attach plans this way so that parent's initplans are processed before
1453+
* any pre-existing initplans of the child. Probably doesn't matter, but
1454+
* let's preserve the ordering just in case.
1455+
*/
14441456
child->initPlan=list_concat(parent->initPlan,
14451457
child->initPlan);
14461458

‎src/backend/optimizer/plan/subselect.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2121,7 +2121,7 @@ SS_identify_outer_params(PlannerInfo *root)
21212121
* This is separate from SS_attach_initplans because we might conditionally
21222122
* create more initPlans during create_plan(), depending on which Path we
21232123
* select. However, Paths that would generate such initPlans are expected
2124-
* to have included their cost already.
2124+
* to have included their costand parallel-safety effectsalready.
21252125
*/
21262126
void
21272127
SS_charge_for_initplans(PlannerInfo*root,RelOptInfo*final_rel)
@@ -2177,8 +2177,10 @@ SS_charge_for_initplans(PlannerInfo *root, RelOptInfo *final_rel)
21772177
* (In principle the initPlans could go in any node at or above where they're
21782178
* referenced; but there seems no reason to put them any lower than the
21792179
* topmost node, so we don't bother to track exactly where they came from.)
2180-
* We do not touch the plan node's cost; the initplans should have been
2181-
* accounted for in path costing.
2180+
*
2181+
* We do not touch the plan node's cost or parallel_safe flag. The initplans
2182+
* must have been accounted for in SS_charge_for_initplans, or by any later
2183+
* code that adds initplans via SS_make_initplan_from_plan.
21822184
*/
21832185
void
21842186
SS_attach_initplans(PlannerInfo*root,Plan*plan)

‎src/backend/optimizer/util/pathnode.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3339,7 +3339,7 @@ create_minmaxagg_path(PlannerInfo *root,
33393339
/* For now, assume we are above any joins, so no parameterization */
33403340
pathnode->path.param_info=NULL;
33413341
pathnode->path.parallel_aware= false;
3342-
/* A MinMaxAggPath implies use ofsubplans, so cannot be parallel-safe */
3342+
/* A MinMaxAggPath implies use ofinitplans, so cannot be parallel-safe */
33433343
pathnode->path.parallel_safe= false;
33443344
pathnode->path.parallel_workers=0;
33453345
/* Result is one unordered row */

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp