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

Commit925f46f

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 parentdadf981 commit925f46f

File tree

7 files changed

+180
-103
lines changed

7 files changed

+180
-103
lines changed

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

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1607,7 +1607,7 @@ add_paths_to_append_rel(PlannerInfo *root, RelOptInfo *rel,
16071607
* Consider an append of unordered, unparameterized partial paths. Make
16081608
* it parallel-aware if possible.
16091609
*/
1610-
if (partial_subpaths_valid)
1610+
if (partial_subpaths_valid&&partial_subpaths!=NIL)
16111611
{
16121612
AppendPath*appendpath;
16131613
ListCell*lc;
@@ -2004,9 +2004,11 @@ accumulate_append_subpath(Path *path, List **subpaths, List **special_subpaths)
20042004
* Build a dummy path for a relation that's been excluded by constraints
20052005
*
20062006
* Rather than inventing a special "dummy" path type, we represent this as an
2007-
* AppendPath with no members (see alsoIS_DUMMY_PATH/IS_DUMMY_REL macros).
2007+
* AppendPath with no members (see alsoIS_DUMMY_APPEND/IS_DUMMY_REL macros).
20082008
*
2009-
* This is exported because inheritance_planner() has need for it.
2009+
* (See also mark_dummy_rel, which does basically the same thing, but is
2010+
* typically used to change a rel into dummy state after we already made
2011+
* paths for it.)
20102012
*/
20112013
void
20122014
set_dummy_rel_pathlist(RelOptInfo*rel)
@@ -2019,14 +2021,15 @@ set_dummy_rel_pathlist(RelOptInfo *rel)
20192021
rel->pathlist=NIL;
20202022
rel->partial_pathlist=NIL;
20212023

2024+
/* Set up the dummy path */
20222025
add_path(rel, (Path*)create_append_path(NULL,rel,NIL,NIL,NULL,
20232026
0, false,NIL,-1));
20242027

20252028
/*
2026-
* We set the cheapestpath immediately,to ensure that IS_DUMMY_REL()
2027-
*will recognize the relation as dummy if anyone asks. This is redundant
2028-
*when we're calledfrom set_rel_size(), but not when called from
2029-
*elsewhere, and doing ittwice is harmless anyway.
2029+
* We set the cheapest-pathfieldsimmediately,just in case they were
2030+
*pointing at some discarded path. This is redundant when we're called
2031+
* from set_rel_size(), but not when called from elsewhere, and doing it
2032+
* twice is harmless anyway.
20302033
*/
20312034
set_cheapest(rel);
20322035
}
@@ -3552,12 +3555,12 @@ generate_partitionwise_join_paths(PlannerInfo *root, RelOptInfo *rel)
35523555
/* Add partitionwise join paths for partitioned child-joins. */
35533556
generate_partitionwise_join_paths(root,child_rel);
35543557

3558+
set_cheapest(child_rel);
3559+
35553560
/* Dummy children will not be scanned, so ignore those. */
35563561
if (IS_DUMMY_REL(child_rel))
35573562
continue;
35583563

3559-
set_cheapest(child_rel);
3560-
35613564
#ifdefOPTIMIZER_DEBUG
35623565
debug_print_rel(root,child_rel);
35633566
#endif

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

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ static void make_rels_by_clauseless_joins(PlannerInfo *root,
3333
ListCell*other_rels);
3434
staticboolhas_join_restriction(PlannerInfo*root,RelOptInfo*rel);
3535
staticboolhas_legal_joinclause(PlannerInfo*root,RelOptInfo*rel);
36-
staticboolis_dummy_rel(RelOptInfo*rel);
3736
staticboolrestriction_is_constant_false(List*restrictlist,
3837
RelOptInfo*joinrel,
3938
boolonly_pushed_down);
@@ -1192,10 +1191,38 @@ have_dangerous_phv(PlannerInfo *root,
11921191
/*
11931192
* is_dummy_rel --- has relation been proven empty?
11941193
*/
1195-
staticbool
1194+
bool
11961195
is_dummy_rel(RelOptInfo*rel)
11971196
{
1198-
returnIS_DUMMY_REL(rel);
1197+
Path*path;
1198+
1199+
/*
1200+
* A rel that is known dummy will have just one path that is a childless
1201+
* Append. (Even if somehow it has more paths, a childless Append will
1202+
* have cost zero and hence should be at the front of the pathlist.)
1203+
*/
1204+
if (rel->pathlist==NIL)
1205+
return false;
1206+
path= (Path*)linitial(rel->pathlist);
1207+
1208+
/*
1209+
* Initially, a dummy path will just be a childless Append. But in later
1210+
* planning stages we might stick a ProjectSetPath and/or ProjectionPath
1211+
* on top, since Append can't project. Rather than make assumptions about
1212+
* which combinations can occur, just descend through whatever we find.
1213+
*/
1214+
for (;;)
1215+
{
1216+
if (IsA(path,ProjectionPath))
1217+
path= ((ProjectionPath*)path)->subpath;
1218+
elseif (IsA(path,ProjectSetPath))
1219+
path= ((ProjectSetPath*)path)->subpath;
1220+
else
1221+
break;
1222+
}
1223+
if (IS_DUMMY_APPEND(path))
1224+
return true;
1225+
return false;
11991226
}
12001227

12011228
/*

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

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1038,7 +1038,7 @@ create_append_plan(PlannerInfo *root, AppendPath *best_path)
10381038
*
10391039
* Note that an AppendPath with no members is also generated in certain
10401040
* cases where there was no appending construct at all, but we know the
1041-
* relation is empty (see set_dummy_rel_pathlist).
1041+
* relation is empty (see set_dummy_rel_pathlist and mark_dummy_rel).
10421042
*/
10431043
if (best_path->subpaths==NIL)
10441044
{
@@ -6506,12 +6506,11 @@ is_projection_capable_path(Path *path)
65066506
caseT_Append:
65076507

65086508
/*
6509-
* Append can't project, but if it's being used to represent a
6510-
* dummy path, claim that it can project. This prevents us from
6511-
* converting a rel from dummy to non-dummy status by applying a
6512-
* projection to its dummy path.
6509+
* Append can't project, but if an AppendPath is being used to
6510+
* represent a dummy path, what will actually be generated is a
6511+
* Result which can project.
65136512
*/
6514-
returnIS_DUMMY_PATH(path);
6513+
returnIS_DUMMY_APPEND(path);
65156514
caseT_ProjectSet:
65166515

65176516
/*

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

Lines changed: 74 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -1495,7 +1495,7 @@ inheritance_planner(PlannerInfo *root)
14951495
* If this child rel was excluded by constraint exclusion, exclude it
14961496
* from the result plan.
14971497
*/
1498-
if (IS_DUMMY_PATH(subpath))
1498+
if (IS_DUMMY_REL(sub_final_rel))
14991499
continue;
15001500

15011501
/*
@@ -3987,12 +3987,10 @@ create_ordinary_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel,
39873987
* If this is the topmost grouping relation or if the parent relation is
39883988
* doing some form of partitionwise aggregation, then we may be able to do
39893989
* it at this level also. However, if the input relation is not
3990-
* partitioned, partitionwise aggregate is impossible, and if it is dummy,
3991-
* partitionwise aggregate is pointless.
3990+
* partitioned, partitionwise aggregate is impossible.
39923991
*/
39933992
if (extra->patype!=PARTITIONWISE_AGGREGATE_NONE&&
3994-
input_rel->part_scheme&&input_rel->part_rels&&
3995-
!IS_DUMMY_REL(input_rel))
3993+
IS_PARTITIONED_REL(input_rel))
39963994
{
39973995
/*
39983996
* If this is the topmost relation or if the parent relation is doing
@@ -6817,27 +6815,48 @@ apply_scanjoin_target_to_paths(PlannerInfo *root,
68176815
boolscanjoin_target_parallel_safe,
68186816
booltlist_same_exprs)
68196817
{
6820-
ListCell*lc;
6818+
boolrel_is_partitioned=IS_PARTITIONED_REL(rel);
68216819
PathTarget*scanjoin_target;
6822-
boolis_dummy_rel=IS_DUMMY_REL(rel);
6820+
ListCell*lc;
68236821

6822+
/* This recurses, so be paranoid. */
68246823
check_stack_depth();
68256824

6825+
/*
6826+
* If the rel is partitioned, we want to drop its existing paths and
6827+
* generate new ones. This function would still be correct if we kept the
6828+
* existing paths: we'd modify them to generate the correct target above
6829+
* the partitioning Append, and then they'd compete on cost with paths
6830+
* generating the target below the Append. However, in our current cost
6831+
* model the latter way is always the same or cheaper cost, so modifying
6832+
* the existing paths would just be useless work. Moreover, when the cost
6833+
* is the same, varying roundoff errors might sometimes allow an existing
6834+
* path to be picked, resulting in undesirable cross-platform plan
6835+
* variations. So we drop old paths and thereby force the work to be done
6836+
* below the Append, except in the case of a non-parallel-safe target.
6837+
*
6838+
* Some care is needed, because we have to allow generate_gather_paths to
6839+
* see the old partial paths in the next stanza. Hence, zap the main
6840+
* pathlist here, then allow generate_gather_paths to add path(s) to the
6841+
* main list, and finally zap the partial pathlist.
6842+
*/
6843+
if (rel_is_partitioned)
6844+
rel->pathlist=NIL;
6845+
68266846
/*
68276847
* If the scan/join target is not parallel-safe, partial paths cannot
68286848
* generate it.
68296849
*/
68306850
if (!scanjoin_target_parallel_safe)
68316851
{
68326852
/*
6833-
* Since we can't generate the final scan/join target, this is our
6834-
* last opportunity to use any partial paths that exist. We don't do
6835-
* this if the case where the target is parallel-safe, since we will
6836-
* be able to generate superior paths by doing it after the final
6837-
* scan/join target has been applied.
6838-
*
6839-
* Note that this may invalidate rel->cheapest_total_path, so we must
6840-
* not rely on it after this point without first calling set_cheapest.
6853+
* Since we can't generate the final scan/join target in parallel
6854+
* workers, this is our last opportunity to use any partial paths that
6855+
* exist; so build Gather path(s) that use them and emit whatever the
6856+
* current reltarget is. We don't do this in the case where the
6857+
* target is parallel-safe, since we will be able to generate superior
6858+
* paths by doing it after the final scan/join target has been
6859+
* applied.
68416860
*/
68426861
generate_gather_paths(root,rel, false);
68436862

@@ -6846,79 +6865,46 @@ apply_scanjoin_target_to_paths(PlannerInfo *root,
68466865
rel->consider_parallel= false;
68476866
}
68486867

6849-
/*
6850-
* Update the reltarget. This may not be strictly necessary in all cases,
6851-
* but it is at least necessary when create_append_path() gets called
6852-
* below directly or indirectly, since that function uses the reltarget as
6853-
* the pathtarget for the resulting path. It seems like a good idea to do
6854-
* it unconditionally.
6855-
*/
6856-
rel->reltarget=llast_node(PathTarget,scanjoin_targets);
6857-
6858-
/* Special case: handle dummy relations separately. */
6859-
if (is_dummy_rel)
6860-
{
6861-
/*
6862-
* Since this is a dummy rel, it's got a single Append path with no
6863-
* child paths. Replace it with a new path having the final scan/join
6864-
* target. (Note that since Append is not projection-capable, it
6865-
* would be bad to handle this using the general purpose code below;
6866-
* we'd end up putting a ProjectionPath on top of the existing Append
6867-
* node, which would cause this relation to stop appearing to be a
6868-
* dummy rel.)
6869-
*/
6870-
rel->pathlist=list_make1(create_append_path(root,rel,NIL,NIL,
6871-
NULL,0, false,NIL,
6872-
-1));
6868+
/* Finish dropping old paths for a partitioned rel, per comment above */
6869+
if (rel_is_partitioned)
68736870
rel->partial_pathlist=NIL;
6874-
set_cheapest(rel);
6875-
Assert(IS_DUMMY_REL(rel));
6876-
6877-
/*
6878-
* Forget about any child relations. There's no point in adjusting
6879-
* them and no point in using them for later planning stages (in
6880-
* particular, partitionwise aggregate).
6881-
*/
6882-
rel->nparts=0;
6883-
rel->part_rels=NULL;
6884-
rel->boundinfo=NULL;
6885-
6886-
return;
6887-
}
68886871

68896872
/* Extract SRF-free scan/join target. */
68906873
scanjoin_target=linitial_node(PathTarget,scanjoin_targets);
68916874

68926875
/*
6893-
* Adjust each input path. If the tlist exprs are the same, we can just
6894-
* inject the sortgroupref information into the existing pathtarget.
6895-
* Otherwise, replace each path with a projection path that generates the
6896-
* SRF-free scan/join target. This can't change the ordering of paths
6897-
* within rel->pathlist, so we just modify the list in place.
6876+
* Apply the SRF-free scan/join target to each existing path.
6877+
*
6878+
* If the tlist exprs are the same, we can just inject the sortgroupref
6879+
* information into the existing pathtargets. Otherwise, replace each
6880+
* path with a projection path that generates the SRF-free scan/join
6881+
* target. This can't change the ordering of paths within rel->pathlist,
6882+
* so we just modify the list in place.
68986883
*/
68996884
foreach(lc,rel->pathlist)
69006885
{
69016886
Path*subpath= (Path*)lfirst(lc);
6902-
Path*newpath;
69036887

6888+
/* Shouldn't have any parameterized paths anymore */
69046889
Assert(subpath->param_info==NULL);
69056890

69066891
if (tlist_same_exprs)
69076892
subpath->pathtarget->sortgrouprefs=
69086893
scanjoin_target->sortgrouprefs;
69096894
else
69106895
{
6896+
Path*newpath;
6897+
69116898
newpath= (Path*)create_projection_path(root,rel,subpath,
69126899
scanjoin_target);
69136900
lfirst(lc)=newpath;
69146901
}
69156902
}
69166903

6917-
/*Samefor partial paths. */
6904+
/*Likewise adjust the targetsfor any partial paths. */
69186905
foreach(lc,rel->partial_pathlist)
69196906
{
69206907
Path*subpath= (Path*)lfirst(lc);
6921-
Path*newpath;
69226908

69236909
/* Shouldn't have any parameterized paths anymore */
69246910
Assert(subpath->param_info==NULL);
@@ -6928,39 +6914,54 @@ apply_scanjoin_target_to_paths(PlannerInfo *root,
69286914
scanjoin_target->sortgrouprefs;
69296915
else
69306916
{
6931-
newpath= (Path*)create_projection_path(root,
6932-
rel,
6933-
subpath,
6917+
Path*newpath;
6918+
6919+
newpath= (Path*)create_projection_path(root,rel,subpath,
69346920
scanjoin_target);
69356921
lfirst(lc)=newpath;
69366922
}
69376923
}
69386924

6939-
/* Now fix things up if scan/join target contains SRFs */
6925+
/*
6926+
* Now, if final scan/join target contains SRFs, insert ProjectSetPath(s)
6927+
* atop each existing path. (Note that this function doesn't look at the
6928+
* cheapest-path fields, which is a good thing because they're bogus right
6929+
* now.)
6930+
*/
69406931
if (root->parse->hasTargetSRFs)
69416932
adjust_paths_for_srfs(root,rel,
69426933
scanjoin_targets,
69436934
scanjoin_targets_contain_srfs);
69446935

69456936
/*
6946-
* If the relation is partitioned, recursively apply the same changes to
6947-
* all partitions and generate new Append paths. Since Append is not
6948-
* projection-capable, that might save a separate Result node, and it also
6949-
* is important for partitionwise aggregate.
6937+
* Update the rel's target to be the final (with SRFs) scan/join target.
6938+
* This now matches the actual output of all the paths, and we might get
6939+
* confused in createplan.c if they don't agree. We must do this now so
6940+
* that any append paths made in the next part will use the correct
6941+
* pathtarget (cf. create_append_path).
69506942
*/
6951-
if (rel->part_scheme&&rel->part_rels)
6943+
rel->reltarget=llast_node(PathTarget,scanjoin_targets);
6944+
6945+
/*
6946+
* If the relation is partitioned, recursively apply the scan/join target
6947+
* to all partitions, and generate brand-new Append paths in which the
6948+
* scan/join target is computed below the Append rather than above it.
6949+
* Since Append is not projection-capable, that might save a separate
6950+
* Result node, and it also is important for partitionwise aggregate.
6951+
*/
6952+
if (rel_is_partitioned)
69526953
{
6953-
intpartition_idx;
69546954
List*live_children=NIL;
6955+
intpartition_idx;
69556956

69566957
/* Adjust each partition. */
69576958
for (partition_idx=0;partition_idx<rel->nparts;partition_idx++)
69586959
{
69596960
RelOptInfo*child_rel=rel->part_rels[partition_idx];
6960-
ListCell*lc;
69616961
AppendRelInfo**appinfos;
69626962
intnappinfos;
69636963
List*child_scanjoin_targets=NIL;
6964+
ListCell*lc;
69646965

69656966
/* Translate scan/join targets for this child. */
69666967
appinfos=find_appinfos_by_relids(root,child_rel->relids,
@@ -6992,8 +6993,7 @@ apply_scanjoin_target_to_paths(PlannerInfo *root,
69926993
}
69936994

69946995
/* Build new paths for this relation by appending child paths. */
6995-
if (live_children!=NIL)
6996-
add_paths_to_append_rel(root,rel,live_children);
6996+
add_paths_to_append_rel(root,rel,live_children);
69976997
}
69986998

69996999
/*

0 commit comments

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