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

Commit19ff710

Browse files
committed
Fix handling of targetlist SRFs when scan/join relation is known empty.
When we introduced separate ProjectSetPath nodes for application ofset-returning functions in v10, we inadvertently broke some cases wherewe're supposed to recognize that the result of a subquery is known to beempty (contain zero rows). That's because IS_DUMMY_REL was just lookingfor a childless AppendPath without allowing for a ProjectSetPath beingpossibly stuck on top. In itself, this didn't do anything much worsethan produce slightly worse plans for some corner cases.Then in v11, commit11cf92f rearranged things to allow the scan/jointargetlist to be applied directly to partial paths before they getgathered. But it inserted a short-circuit path for dummy relationsthat was a little too short: it failed to insert a ProjectSetPath nodeat all for a targetlist containing set-returning functions, resulting inbogus "set-valued function called in context that cannot accept a set"errors, as reported in bug #15669 from Madelaine Thibaut.The best way to fix this mess seems to be to reimplement IS_DUMMY_RELso that it drills down through any ProjectSetPath nodes that might bethere (and it seems like we'd better allow for ProjectionPath as well).While we're at it, make it look at rel->pathlist not cheapest_total_path,so that it gives the right answer independently of whether set_cheapesthas been done lately. That dependency looks pretty shaky in the contextof code like apply_scanjoin_target_to_paths, and even if it's not brokentoday it'd certainly bite us at some point. (Nastily, unsafe use of theold coding would almost always work; the hazard comes down to possiblylooking through a dangling pointer, and only once in a blue moon wouldyou find something there that resulted in the wrong answer.)It now looks like it was a mistake for IS_DUMMY_REL to be a macro: ifthere are any extensions using it, they'll continue to use the oldinadequate logic until they're recompiled, after which they'll failto load into server versions predating this fix. Hopefully there arefew such extensions.Having fixed IS_DUMMY_REL, the special path for dummy rels inapply_scanjoin_target_to_paths is unnecessary as well as being wrong,so we can just drop it.Also change a few places that were testing for partitioned-ness of aplanner relation but not using IS_PARTITIONED_REL for the purpose; thatseems unsafe as well as inconsistent, plus it required an ugly hack inapply_scanjoin_target_to_paths.In passing, save a few cycles in apply_scanjoin_target_to_paths byskipping processing of pre-existing paths for partitioned rels,and do some cosmetic cleanup and comment adjustment in that function.I renamed IS_DUMMY_PATH to IS_DUMMY_APPEND with the intention of breakingany code that might be using it, since in almost every case that wouldbe wrong; IS_DUMMY_REL is what to be using instead.In HEAD, also make set_dummy_rel_pathlist static (since it's no longerused from outside allpaths.c), and delete is_dummy_plan, since it's nolonger used anywhere.Back-patch as appropriate into v11 and v10.Tom Lane and Julien RouhaudDiscussion:https://postgr.es/m/15669-02fb3296cca26203@postgresql.org
1 parentc2c937c commit19ff710

File tree

7 files changed

+102
-22
lines changed

7 files changed

+102
-22
lines changed

‎src/backend/optimizer/path/allpaths.c

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1453,7 +1453,7 @@ add_paths_to_append_rel(PlannerInfo *root, RelOptInfo *rel,
14531453
/*
14541454
* Consider an append of partial unordered, unparameterized partial paths.
14551455
*/
1456-
if (partial_subpaths_valid)
1456+
if (partial_subpaths_valid&&partial_subpaths!=NIL)
14571457
{
14581458
AppendPath*appendpath;
14591459
ListCell*lc;
@@ -1750,9 +1750,11 @@ accumulate_append_subpath(List *subpaths, Path *path)
17501750
* Build a dummy path for a relation that's been excluded by constraints
17511751
*
17521752
* Rather than inventing a special "dummy" path type, we represent this as an
1753-
* AppendPath with no members (see alsoIS_DUMMY_PATH/IS_DUMMY_REL macros).
1753+
* AppendPath with no members (see alsoIS_DUMMY_APPEND/IS_DUMMY_REL macros).
17541754
*
1755-
* This is exported because inheritance_planner() has need for it.
1755+
* (See also mark_dummy_rel, which does basically the same thing, but is
1756+
* typically used to change a rel into dummy state after we already made
1757+
* paths for it.)
17561758
*/
17571759
void
17581760
set_dummy_rel_pathlist(RelOptInfo*rel)
@@ -1765,13 +1767,14 @@ set_dummy_rel_pathlist(RelOptInfo *rel)
17651767
rel->pathlist=NIL;
17661768
rel->partial_pathlist=NIL;
17671769

1770+
/* Set up the dummy path */
17681771
add_path(rel, (Path*)create_append_path(rel,NIL,NULL,0,NIL));
17691772

17701773
/*
1771-
* We set the cheapestpath immediately,to ensure that IS_DUMMY_REL()
1772-
*will recognize the relation as dummy if anyone asks. This is redundant
1773-
*when we're calledfrom set_rel_size(), but not when called from
1774-
*elsewhere, and doing ittwice is harmless anyway.
1774+
* We set the cheapest-pathfieldsimmediately,just in case they were
1775+
*pointing at some discarded path. This is redundant when we're called
1776+
* from set_rel_size(), but not when called from elsewhere, and doing it
1777+
* twice is harmless anyway.
17751778
*/
17761779
set_cheapest(rel);
17771780
}

‎src/backend/optimizer/path/joinrels.c

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ static void make_rels_by_clauseless_joins(PlannerInfo *root,
2828
ListCell*other_rels);
2929
staticboolhas_join_restriction(PlannerInfo*root,RelOptInfo*rel);
3030
staticboolhas_legal_joinclause(PlannerInfo*root,RelOptInfo*rel);
31-
staticboolis_dummy_rel(RelOptInfo*rel);
3231
staticvoidmark_dummy_rel(RelOptInfo*rel);
3332
staticboolrestriction_is_constant_false(List*restrictlist,
3433
RelOptInfo*joinrel,
@@ -1177,10 +1176,38 @@ have_dangerous_phv(PlannerInfo *root,
11771176
/*
11781177
* is_dummy_rel --- has relation been proven empty?
11791178
*/
1180-
staticbool
1179+
bool
11811180
is_dummy_rel(RelOptInfo*rel)
11821181
{
1183-
returnIS_DUMMY_REL(rel);
1182+
Path*path;
1183+
1184+
/*
1185+
* A rel that is known dummy will have just one path that is a childless
1186+
* Append. (Even if somehow it has more paths, a childless Append will
1187+
* have cost zero and hence should be at the front of the pathlist.)
1188+
*/
1189+
if (rel->pathlist==NIL)
1190+
return false;
1191+
path= (Path*)linitial(rel->pathlist);
1192+
1193+
/*
1194+
* Initially, a dummy path will just be a childless Append. But in later
1195+
* planning stages we might stick a ProjectSetPath and/or ProjectionPath
1196+
* on top, since Append can't project. Rather than make assumptions about
1197+
* which combinations can occur, just descend through whatever we find.
1198+
*/
1199+
for (;;)
1200+
{
1201+
if (IsA(path,ProjectionPath))
1202+
path= ((ProjectionPath*)path)->subpath;
1203+
elseif (IsA(path,ProjectSetPath))
1204+
path= ((ProjectSetPath*)path)->subpath;
1205+
else
1206+
break;
1207+
}
1208+
if (IS_DUMMY_APPEND(path))
1209+
return true;
1210+
return false;
11841211
}
11851212

11861213
/*

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

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1013,7 +1013,7 @@ create_append_plan(PlannerInfo *root, AppendPath *best_path)
10131013
*
10141014
* Note that an AppendPath with no members is also generated in certain
10151015
* cases where there was no appending construct at all, but we know the
1016-
* relation is empty (see set_dummy_rel_pathlist).
1016+
* relation is empty (see set_dummy_rel_pathlist and mark_dummy_rel).
10171017
*/
10181018
if (best_path->subpaths==NIL)
10191019
{
@@ -6428,12 +6428,11 @@ is_projection_capable_path(Path *path)
64286428
caseT_Append:
64296429

64306430
/*
6431-
* Append can't project, but if it's being used to represent a
6432-
* dummy path, claim that it can project. This prevents us from
6433-
* converting a rel from dummy to non-dummy status by applying a
6434-
* projection to its dummy path.
6431+
* Append can't project, but if an AppendPath is being used to
6432+
* represent a dummy path, what will actually be generated is a
6433+
* Result which can project.
64356434
*/
6436-
returnIS_DUMMY_PATH(path);
6435+
returnIS_DUMMY_APPEND(path);
64376436
caseT_ProjectSet:
64386437

64396438
/*

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1326,7 +1326,7 @@ inheritance_planner(PlannerInfo *root)
13261326
* If this child rel was excluded by constraint exclusion, exclude it
13271327
* from the result plan.
13281328
*/
1329-
if (IS_DUMMY_PATH(subpath))
1329+
if (IS_DUMMY_REL(sub_final_rel))
13301330
continue;
13311331

13321332
/*

‎src/include/nodes/relation.h

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1171,6 +1171,9 @@ typedef struct CustomPath
11711171
* elements. These cases are optimized during create_append_plan.
11721172
* In particular, an AppendPath with no subpaths is a "dummy" path that
11731173
* is created to represent the case that a relation is provably empty.
1174+
* (This is a convenient representation because it means that when we build
1175+
* an appendrel and find that all its children have been excluded, no extra
1176+
* action is needed to recognize the relation as dummy.)
11741177
*/
11751178
typedefstructAppendPath
11761179
{
@@ -1180,13 +1183,16 @@ typedef struct AppendPath
11801183
List*subpaths;/* list of component Paths */
11811184
}AppendPath;
11821185

1183-
#defineIS_DUMMY_PATH(p) \
1186+
#defineIS_DUMMY_APPEND(p) \
11841187
(IsA((p), AppendPath) && ((AppendPath *) (p))->subpaths == NIL)
11851188

1186-
/* A relation that's been proven empty will have one path that is dummy */
1187-
#defineIS_DUMMY_REL(r) \
1188-
((r)->cheapest_total_path != NULL && \
1189-
IS_DUMMY_PATH((r)->cheapest_total_path))
1189+
/*
1190+
* A relation that's been proven empty will have one path that is dummy
1191+
* (but might have projection paths on top). For historical reasons,
1192+
* this is provided as a macro that wraps is_dummy_rel().
1193+
*/
1194+
#defineIS_DUMMY_REL(r) is_dummy_rel(r)
1195+
externboolis_dummy_rel(RelOptInfo*rel);
11901196

11911197
/*
11921198
* MergeAppendPath represents a MergeAppend plan, ie, the merging of sorted

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,39 @@ SELECT generate_series(1, generate_series(1, 3)), generate_series(2, 4);
8383

8484
CREATE TABLE few(id int, dataa text, datab text);
8585
INSERT INTO few VALUES(1, 'a', 'foo'),(2, 'a', 'bar'),(3, 'b', 'bar');
86+
-- SRF with a provably-dummy relation
87+
explain (verbose, costs off)
88+
SELECT unnest(ARRAY[1, 2]) FROM few WHERE false;
89+
QUERY PLAN
90+
--------------------------------------
91+
ProjectSet
92+
Output: unnest('{1,2}'::integer[])
93+
-> Result
94+
One-Time Filter: false
95+
(4 rows)
96+
97+
SELECT unnest(ARRAY[1, 2]) FROM few WHERE false;
98+
unnest
99+
--------
100+
(0 rows)
101+
102+
-- SRF shouldn't prevent upper query from recognizing lower as dummy
103+
explain (verbose, costs off)
104+
SELECT * FROM few f1,
105+
(SELECT unnest(ARRAY[1,2]) FROM few f2 WHERE false OFFSET 0) ss;
106+
QUERY PLAN
107+
------------------------------------------------
108+
Result
109+
Output: f1.id, f1.dataa, f1.datab, ss.unnest
110+
One-Time Filter: false
111+
(3 rows)
112+
113+
SELECT * FROM few f1,
114+
(SELECT unnest(ARRAY[1,2]) FROM few f2 WHERE false OFFSET 0) ss;
115+
id | dataa | datab | unnest
116+
----+-------+-------+--------
117+
(0 rows)
118+
86119
-- SRF output order of sorting is maintained, if SRF is not referenced
87120
SELECT few.id, generate_series(1,3) g FROM few ORDER BY id DESC;
88121
id | g

‎src/test/regress/sql/tsrf.sql

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,18 @@ SELECT generate_series(1, generate_series(1, 3)), generate_series(2, 4);
2828
CREATETABLEfew(idint, dataatext, databtext);
2929
INSERT INTO fewVALUES(1,'a','foo'),(2,'a','bar'),(3,'b','bar');
3030

31+
-- SRF with a provably-dummy relation
32+
explain (verbose, costs off)
33+
SELECT unnest(ARRAY[1,2])FROM fewWHERE false;
34+
SELECT unnest(ARRAY[1,2])FROM fewWHERE false;
35+
36+
-- SRF shouldn't prevent upper query from recognizing lower as dummy
37+
explain (verbose, costs off)
38+
SELECT*FROM few f1,
39+
(SELECT unnest(ARRAY[1,2])FROM few f2WHERE false OFFSET0) ss;
40+
SELECT*FROM few f1,
41+
(SELECT unnest(ARRAY[1,2])FROM few f2WHERE false OFFSET0) ss;
42+
3143
-- SRF output order of sorting is maintained, if SRF is not referenced
3244
SELECTfew.id, generate_series(1,3) gFROM fewORDER BY idDESC;
3345

0 commit comments

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