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

Commit6ee41a3

Browse files
committed
Fix mis-planning of repeated application of a projection.
create_projection_plan contains a hidden assumption (here madeexplicit by an Assert) that a projection-capable Path will yield aprojection-capable Plan. Unfortunately, that assumption is violatedonly a few lines away, by create_projection_plan itself. This meansthat two stacked ProjectionPaths can yield an outcome where we try tojam the upper path's tlist into a non-projection-capable child node,resulting in an invalid plan.There isn't any good reason to have stacked ProjectionPaths; indeed thewhole concept is faulty, since the set of Vars/Aggs/etc needed by theupper one wouldn't necessarily be available in the output of the lowerone, nor could the lower one create such values if they weren'tavailable from its input. Hence, we can fix this by adjustingcreate_projection_path to strip any top-level ProjectionPath from thesubpath it's given. (This amounts to saying "oh, we changed ourminds about what we need to project here".)The test case added here only fails in v13 and HEAD; before that, wedon't attempt to shove the Sort into the parallel part of the plan,for reasons that aren't entirely clear to me. However, all thedirectly-related code looks generally the same as far back as v11,where the hazard was introduced (byd7c19e6). So I've got no faiththat the same type of bug doesn't exist in v11 and v12, given theright test case. Hence, back-patch the code changes, but not theirrelevant test case, into those branches.Per report from Bas Poot.Discussion:https://postgr.es/m/534fca83789c4a378c7de379e9067d4f@politie.nl
1 parentd03eeab commit6ee41a3

File tree

4 files changed

+46
-1
lines changed

4 files changed

+46
-1
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1976,6 +1976,7 @@ create_projection_plan(PlannerInfo *root, ProjectionPath *best_path, int flags)
19761976
*/
19771977
subplan=create_plan_recurse(root,best_path->subpath,
19781978
CP_IGNORE_TLIST);
1979+
Assert(is_projection_capable_plan(subplan));
19791980
tlist=build_path_tlist(root,&best_path->path);
19801981
}
19811982
else

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2632,7 +2632,23 @@ create_projection_path(PlannerInfo *root,
26322632
PathTarget*target)
26332633
{
26342634
ProjectionPath*pathnode=makeNode(ProjectionPath);
2635-
PathTarget*oldtarget=subpath->pathtarget;
2635+
PathTarget*oldtarget;
2636+
2637+
/*
2638+
* We mustn't put a ProjectionPath directly above another; it's useless
2639+
* and will confuse create_projection_plan. Rather than making sure all
2640+
* callers handle that, let's implement it here, by stripping off any
2641+
* ProjectionPath in what we're given. Given this rule, there won't be
2642+
* more than one.
2643+
*/
2644+
if (IsA(subpath,ProjectionPath))
2645+
{
2646+
ProjectionPath*subpp= (ProjectionPath*)subpath;
2647+
2648+
Assert(subpp->path.parent==rel);
2649+
subpath=subpp->subpath;
2650+
Assert(!IsA(subpath,ProjectionPath));
2651+
}
26362652

26372653
pathnode->path.pathtype=T_Result;
26382654
pathnode->path.parent=rel;
@@ -2658,6 +2674,7 @@ create_projection_path(PlannerInfo *root,
26582674
* Note: in the latter case, create_projection_plan has to recheck our
26592675
* conclusion; see comments therein.
26602676
*/
2677+
oldtarget=subpath->pathtarget;
26612678
if (is_projection_capable_path(subpath)||
26622679
equal(oldtarget->exprs,target->exprs))
26632680
{

‎src/test/regress/expected/select_parallel.out

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1126,6 +1126,29 @@ ORDER BY 1, 2, 3;
11261126
------------------------------+---------------------------+-------------+--------------
11271127
(0 rows)
11281128

1129+
EXPLAIN (VERBOSE, COSTS OFF)
1130+
SELECT generate_series(1, two), array(select generate_series(1, two))
1131+
FROM tenk1 ORDER BY tenthous;
1132+
QUERY PLAN
1133+
----------------------------------------------------------------------
1134+
ProjectSet
1135+
Output: generate_series(1, tenk1.two), (SubPlan 1), tenk1.tenthous
1136+
-> Gather Merge
1137+
Output: tenk1.two, tenk1.tenthous
1138+
Workers Planned: 4
1139+
-> Result
1140+
Output: tenk1.two, tenk1.tenthous
1141+
-> Sort
1142+
Output: tenk1.tenthous, tenk1.two
1143+
Sort Key: tenk1.tenthous
1144+
-> Parallel Seq Scan on public.tenk1
1145+
Output: tenk1.tenthous, tenk1.two
1146+
SubPlan 1
1147+
-> ProjectSet
1148+
Output: generate_series(1, tenk1.two)
1149+
-> Result
1150+
(16 rows)
1151+
11291152
-- test passing expanded-value representations to workers
11301153
CREATE FUNCTION make_some_array(int,int) returns int[] as
11311154
$$declare x int[];

‎src/test/regress/sql/select_parallel.sql

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,10 @@ ORDER BY 1;
429429
SELECT*FROMinformation_schema.foreign_data_wrapper_options
430430
ORDER BY1,2,3;
431431

432+
EXPLAIN (VERBOSE, COSTS OFF)
433+
SELECT generate_series(1, two), array(select generate_series(1, two))
434+
FROM tenk1ORDER BY tenthous;
435+
432436
-- test passing expanded-value representations to workers
433437
CREATEFUNCTIONmake_some_array(int,int) returnsint[]as
434438
$$declare xint[];

0 commit comments

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