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

Commit9e7e29c

Browse files
committed
Fix planner problems with LATERAL references in PlaceHolderVars.
The planner largely failed to consider the possibility that aPlaceHolderVar's expression might contain a lateral reference to a Varcoming from somewhere outside the PHV's syntactic scope. We had a previousreport of a problem in this area, which I tried to fix in a quick-hack wayin commit4da6439, but Antonin Houskapointed out that there were still some problems, and investigation turnedup other issues. This patch largely reverts that commit in favor of a morethoroughly thought-through solution. The new theory is that a PHV'sph_eval_at level cannot be higher than its original syntactic level. If itcontains lateral references, those don't change the ph_eval_at level, butrather they create a lateral-reference requirement for the ph_eval_at joinrelation. The code in joinpath.c needs to handle that.Another issue is that createplan.c wasn't handling nested PlaceHolderVarsproperly.In passing, push knowledge of lateral-reference checks for join clausesinto join_clause_is_movable_to. This is mainly so that FDWs don't needto deal with it.This patch doesn't fix the original join-qual-placement problem reported byJeremy Evans (and indeed, one of the new regression test cases shows thewrong answer because of that). But the PlaceHolderVar problems need to befixed before that issue can be addressed, so committing this separatelyseems reasonable.
1 parent175ec8d commit9e7e29c

File tree

25 files changed

+838
-309
lines changed

25 files changed

+838
-309
lines changed

‎contrib/postgres_fdw/postgres_fdw.c

Lines changed: 3 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -540,7 +540,6 @@ postgresGetForeignPaths(PlannerInfo *root,
540540
{
541541
PgFdwRelationInfo*fpinfo= (PgFdwRelationInfo*)baserel->fdw_private;
542542
ForeignPath*path;
543-
Relidslateral_referencers;
544543
List*join_quals;
545544
Relidsrequired_outer;
546545
doublerows;
@@ -579,34 +578,13 @@ postgresGetForeignPaths(PlannerInfo *root,
579578
* consider combinations of clauses, probably.
580579
*/
581580

582-
/*
583-
* If there are any rels that have LATERAL references to this one, we
584-
* cannot use join quals referencing them as remote quals for this one,
585-
* since such rels would have to be on the inside not the outside of a
586-
* nestloop join relative to this one.Create a Relids set listing all
587-
* such rels, for use in checks of potential join clauses.
588-
*/
589-
lateral_referencers=NULL;
590-
foreach(lc,root->lateral_info_list)
591-
{
592-
LateralJoinInfo*ljinfo= (LateralJoinInfo*)lfirst(lc);
593-
594-
if (bms_is_member(baserel->relid,ljinfo->lateral_lhs))
595-
lateral_referencers=bms_add_member(lateral_referencers,
596-
ljinfo->lateral_rhs);
597-
}
598-
599581
/* Scan the rel's join clauses */
600582
foreach(lc,baserel->joininfo)
601583
{
602584
RestrictInfo*rinfo= (RestrictInfo*)lfirst(lc);
603585

604586
/* Check if clause can be moved to this rel */
605-
if (!join_clause_is_movable_to(rinfo,baserel->relid))
606-
continue;
607-
608-
/* Not useful if it conflicts with any LATERAL references */
609-
if (bms_overlap(rinfo->clause_relids,lateral_referencers))
587+
if (!join_clause_is_movable_to(rinfo,baserel))
610588
continue;
611589

612590
/* See if it is safe to send to remote */
@@ -667,7 +645,7 @@ postgresGetForeignPaths(PlannerInfo *root,
667645
baserel,
668646
ec_member_matches_foreign,
669647
(void*)&arg,
670-
lateral_referencers);
648+
baserel->lateral_referencers);
671649

672650
/* Done if there are no more expressions in the foreign rel */
673651
if (arg.current==NULL)
@@ -682,12 +660,9 @@ postgresGetForeignPaths(PlannerInfo *root,
682660
RestrictInfo*rinfo= (RestrictInfo*)lfirst(lc);
683661

684662
/* Check if clause can be moved to this rel */
685-
if (!join_clause_is_movable_to(rinfo,baserel->relid))
663+
if (!join_clause_is_movable_to(rinfo,baserel))
686664
continue;
687665

688-
/* Shouldn't conflict with any LATERAL references */
689-
Assert(!bms_overlap(rinfo->clause_relids,lateral_referencers));
690-
691666
/* See if it is safe to send to remote */
692667
if (!is_foreign_expr(root,baserel,rinfo->clause))
693668
continue;

‎src/backend/nodes/copyfuncs.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1921,8 +1921,8 @@ _copyLateralJoinInfo(const LateralJoinInfo *from)
19211921
{
19221922
LateralJoinInfo*newnode=makeNode(LateralJoinInfo);
19231923

1924-
COPY_SCALAR_FIELD(lateral_rhs);
19251924
COPY_BITMAPSET_FIELD(lateral_lhs);
1925+
COPY_BITMAPSET_FIELD(lateral_rhs);
19261926

19271927
returnnewnode;
19281928
}
@@ -1956,6 +1956,7 @@ _copyPlaceHolderInfo(const PlaceHolderInfo *from)
19561956
COPY_SCALAR_FIELD(phid);
19571957
COPY_NODE_FIELD(ph_var);
19581958
COPY_BITMAPSET_FIELD(ph_eval_at);
1959+
COPY_BITMAPSET_FIELD(ph_lateral);
19591960
COPY_BITMAPSET_FIELD(ph_needed);
19601961
COPY_SCALAR_FIELD(ph_width);
19611962

‎src/backend/nodes/equalfuncs.c

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -763,15 +763,19 @@ _equalPlaceHolderVar(const PlaceHolderVar *a, const PlaceHolderVar *b)
763763
/*
764764
* We intentionally do not compare phexpr.Two PlaceHolderVars with the
765765
* same ID and levelsup should be considered equal even if the contained
766-
* expressions have managed to mutate to different states.One way in
767-
* which that can happen is that initplan sublinks would get replaced by
768-
* differently-numbered Params when sublink folding is done. (The end
769-
* result of such a situation would be some unreferenced initplans, which
770-
* is annoying but not really a problem.)
766+
* expressions have managed to mutate to different states.This will
767+
* happen during final plan construction when there are nested PHVs, since
768+
* the inner PHV will get replaced by a Param in some copies of the outer
769+
* PHV. Another way in which it can happen is that initplan sublinks
770+
* could get replaced by differently-numbered Params when sublink folding
771+
* is done. (The end result of such a situation would be some
772+
* unreferenced initplans, which is annoying but not really a problem.) On
773+
* the same reasoning, there is no need to examine phrels.
771774
*
772775
* COMPARE_NODE_FIELD(phexpr);
776+
*
777+
* COMPARE_BITMAPSET_FIELD(phrels);
773778
*/
774-
COMPARE_BITMAPSET_FIELD(phrels);
775779
COMPARE_SCALAR_FIELD(phid);
776780
COMPARE_SCALAR_FIELD(phlevelsup);
777781

@@ -796,8 +800,8 @@ _equalSpecialJoinInfo(const SpecialJoinInfo *a, const SpecialJoinInfo *b)
796800
staticbool
797801
_equalLateralJoinInfo(constLateralJoinInfo*a,constLateralJoinInfo*b)
798802
{
799-
COMPARE_SCALAR_FIELD(lateral_rhs);
800803
COMPARE_BITMAPSET_FIELD(lateral_lhs);
804+
COMPARE_BITMAPSET_FIELD(lateral_rhs);
801805

802806
return true;
803807
}
@@ -819,8 +823,9 @@ static bool
819823
_equalPlaceHolderInfo(constPlaceHolderInfo*a,constPlaceHolderInfo*b)
820824
{
821825
COMPARE_SCALAR_FIELD(phid);
822-
COMPARE_NODE_FIELD(ph_var);
826+
COMPARE_NODE_FIELD(ph_var);/* should be redundant */
823827
COMPARE_BITMAPSET_FIELD(ph_eval_at);
828+
COMPARE_BITMAPSET_FIELD(ph_lateral);
824829
COMPARE_BITMAPSET_FIELD(ph_needed);
825830
COMPARE_SCALAR_FIELD(ph_width);
826831

‎src/backend/nodes/outfuncs.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1756,6 +1756,7 @@ _outRelOptInfo(StringInfo str, const RelOptInfo *node)
17561756
WRITE_INT_FIELD(max_attr);
17571757
WRITE_NODE_FIELD(lateral_vars);
17581758
WRITE_BITMAPSET_FIELD(lateral_relids);
1759+
WRITE_BITMAPSET_FIELD(lateral_referencers);
17591760
WRITE_NODE_FIELD(indexlist);
17601761
WRITE_UINT_FIELD(pages);
17611762
WRITE_FLOAT_FIELD(tuples,"%.0f");
@@ -1913,8 +1914,8 @@ _outLateralJoinInfo(StringInfo str, const LateralJoinInfo *node)
19131914
{
19141915
WRITE_NODE_TYPE("LATERALJOININFO");
19151916

1916-
WRITE_UINT_FIELD(lateral_rhs);
19171917
WRITE_BITMAPSET_FIELD(lateral_lhs);
1918+
WRITE_BITMAPSET_FIELD(lateral_rhs);
19181919
}
19191920

19201921
staticvoid
@@ -1938,6 +1939,7 @@ _outPlaceHolderInfo(StringInfo str, const PlaceHolderInfo *node)
19381939
WRITE_UINT_FIELD(phid);
19391940
WRITE_NODE_FIELD(ph_var);
19401941
WRITE_BITMAPSET_FIELD(ph_eval_at);
1942+
WRITE_BITMAPSET_FIELD(ph_lateral);
19411943
WRITE_BITMAPSET_FIELD(ph_needed);
19421944
WRITE_INT_FIELD(ph_width);
19431945
}

‎src/backend/optimizer/README

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -802,5 +802,19 @@ parameterized paths still apply, though; in particular, each such path is
802802
still expected to enforce any join clauses that can be pushed down to it,
803803
so that all paths of the same parameterization have the same rowcount.
804804

805+
We also allow LATERAL subqueries to be flattened (pulled up into the parent
806+
query) by the optimizer, but only when they don't contain any lateral
807+
references to relations outside the lowest outer join that can null the
808+
LATERAL subquery. This restriction prevents lateral references from being
809+
introduced into outer-join qualifications, which would create semantic
810+
confusion. Note that even with this restriction, pullup of a LATERAL
811+
subquery can result in creating PlaceHolderVars that contain lateral
812+
references to relations outside their syntactic scope. We still evaluate
813+
such PHVs at their syntactic location or lower, but the presence of such a
814+
PHV in the quals or targetlist of a plan node requires that node to appear
815+
on the inside of a nestloop join relative to the rel(s) supplying the
816+
lateral reference. (Perhaps now that that stuff works, we could relax the
817+
pullup restriction?)
818+
805819

806820
-- bjm & tgl

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

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -386,8 +386,7 @@ set_plain_rel_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
386386
/*
387387
* We don't support pushing join clauses into the quals of a seqscan, but
388388
* it could still have required parameterization due to LATERAL refs in
389-
* its tlist. (That can only happen if the seqscan is on a relation
390-
* pulled up out of a UNION ALL appendrel.)
389+
* its tlist.
391390
*/
392391
required_outer=rel->lateral_relids;
393392

@@ -550,8 +549,8 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
550549
* Note: the resulting childrel->reltargetlist may contain arbitrary
551550
* expressions, which otherwise would not occur in a reltargetlist.
552551
* Code that might be looking at an appendrel child must cope with
553-
* such.Note in particular that "arbitrary expression" can include
554-
*"Var belonging to another relation", due to LATERAL references.
552+
* such.(Normally, a reltargetlist would only include Vars and
553+
*PlaceHolderVars.)
555554
*/
556555
childrel->joininfo= (List*)
557556
adjust_appendrel_attrs(root,
@@ -1355,8 +1354,7 @@ set_cte_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
13551354
/*
13561355
* We don't support pushing join clauses into the quals of a CTE scan, but
13571356
* it could still have required parameterization due to LATERAL refs in
1358-
* its tlist. (That can only happen if the CTE scan is on a relation
1359-
* pulled up out of a UNION ALL appendrel.)
1357+
* its tlist.
13601358
*/
13611359
required_outer=rel->lateral_relids;
13621360

@@ -1408,10 +1406,8 @@ set_worktable_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
14081406
/*
14091407
* We don't support pushing join clauses into the quals of a worktable
14101408
* scan, but it could still have required parameterization due to LATERAL
1411-
* refs in its tlist. (That can only happen if the worktable scan is on a
1412-
* relation pulled up out of a UNION ALL appendrel. I'm not sure this is
1413-
* actually possible given the restrictions on recursive references, but
1414-
* it's easy enough to support.)
1409+
* refs in its tlist. (I'm not sure this is actually possible given the
1410+
* restrictions on recursive references, but it's easy enough to support.)
14151411
*/
14161412
required_outer=rel->lateral_relids;
14171413

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3943,10 +3943,9 @@ set_rel_width(PlannerInfo *root, RelOptInfo *rel)
39433943

39443944
/*
39453945
* Ordinarily, a Var in a rel's reltargetlist must belong to that rel;
3946-
* but there are corner cases involving LATERAL references in
3947-
* appendrel members where that isn't so (see set_append_rel_size()).
3948-
* If the Var has the wrong varno, fall through to the generic case
3949-
* (it doesn't seem worth the trouble to be any smarter).
3946+
* but there are corner cases involving LATERAL references where that
3947+
* isn't so. If the Var has the wrong varno, fall through to the
3948+
* generic case (it doesn't seem worth the trouble to be any smarter).
39503949
*/
39513950
if (IsA(node,Var)&&
39523951
((Var*)node)->varno==rel->relid)

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

Lines changed: 13 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -141,12 +141,10 @@ static void match_restriction_clauses_to_index(RelOptInfo *rel,
141141
IndexClauseSet*clauseset);
142142
staticvoidmatch_join_clauses_to_index(PlannerInfo*root,
143143
RelOptInfo*rel,IndexOptInfo*index,
144-
Relidslateral_referencers,
145144
IndexClauseSet*clauseset,
146145
List**joinorclauses);
147146
staticvoidmatch_eclass_clauses_to_index(PlannerInfo*root,
148147
IndexOptInfo*index,
149-
Relidslateral_referencers,
150148
IndexClauseSet*clauseset);
151149
staticvoidmatch_clauses_to_index(IndexOptInfo*index,
152150
List*clauses,
@@ -220,14 +218,14 @@ static Const *string_to_const(const char *str, Oid datatype);
220218
*
221219
* Note: check_partial_indexes() must have been run previously for this rel.
222220
*
223-
* Note: incornercases involving LATERALappendrel children, it's possible
224-
* that rel->lateral_relids is nonempty. Currently, we include lateral_relids
225-
* into the parameterization reported for each path, but don't take it into
226-
* account otherwise.The fact that any such rels *must* be available as
227-
* parameter sources perhaps should influence our choices of index quals ...
228-
* but for now, it doesn't seem worth troubling over. In particular, comments
229-
* below about "unparameterized" paths should be read as meaning
230-
* "unparameterized so far as the indexquals are concerned".
221+
* Note: in cases involving LATERALreferences in the relation's tlist, it's
222+
*possiblethat rel->lateral_relids is nonempty. Currently, we include
223+
*lateral_relidsinto the parameterization reported for each path, but don't
224+
*take it intoaccount otherwise.The fact that any such rels *must* be
225+
*available asparameter sources perhaps should influence our choices of
226+
*index quals ...but for now, it doesn't seem worth troubling over.
227+
*In particular, commentsbelow about "unparameterized" paths should be read
228+
*as meaning"unparameterized so far as the indexquals are concerned".
231229
*/
232230
void
233231
create_index_paths(PlannerInfo*root,RelOptInfo*rel)
@@ -236,7 +234,6 @@ create_index_paths(PlannerInfo *root, RelOptInfo *rel)
236234
List*bitindexpaths;
237235
List*bitjoinpaths;
238236
List*joinorclauses;
239-
Relidslateral_referencers;
240237
IndexClauseSetrclauseset;
241238
IndexClauseSetjclauseset;
242239
IndexClauseSeteclauseset;
@@ -246,23 +243,6 @@ create_index_paths(PlannerInfo *root, RelOptInfo *rel)
246243
if (rel->indexlist==NIL)
247244
return;
248245

249-
/*
250-
* If there are any rels that have LATERAL references to this one, we
251-
* cannot use join quals referencing them as index quals for this one,
252-
* since such rels would have to be on the inside not the outside of a
253-
* nestloop join relative to this one.Create a Relids set listing all
254-
* such rels, for use in checks of potential join clauses.
255-
*/
256-
lateral_referencers=NULL;
257-
foreach(lc,root->lateral_info_list)
258-
{
259-
LateralJoinInfo*ljinfo= (LateralJoinInfo*)lfirst(lc);
260-
261-
if (bms_is_member(rel->relid,ljinfo->lateral_lhs))
262-
lateral_referencers=bms_add_member(lateral_referencers,
263-
ljinfo->lateral_rhs);
264-
}
265-
266246
/* Bitmap paths are collected and then dealt with at the end */
267247
bitindexpaths=bitjoinpaths=joinorclauses=NIL;
268248

@@ -303,15 +283,15 @@ create_index_paths(PlannerInfo *root, RelOptInfo *rel)
303283
* EquivalenceClasses.Also, collect join OR clauses for later.
304284
*/
305285
MemSet(&jclauseset,0,sizeof(jclauseset));
306-
match_join_clauses_to_index(root,rel,index,lateral_referencers,
286+
match_join_clauses_to_index(root,rel,index,
307287
&jclauseset,&joinorclauses);
308288

309289
/*
310290
* Look for EquivalenceClasses that can generate joinclauses matching
311291
* the index.
312292
*/
313293
MemSet(&eclauseset,0,sizeof(eclauseset));
314-
match_eclass_clauses_to_index(root,index,lateral_referencers,
294+
match_eclass_clauses_to_index(root,index,
315295
&eclauseset);
316296

317297
/*
@@ -1957,7 +1937,6 @@ match_restriction_clauses_to_index(RelOptInfo *rel, IndexOptInfo *index,
19571937
staticvoid
19581938
match_join_clauses_to_index(PlannerInfo*root,
19591939
RelOptInfo*rel,IndexOptInfo*index,
1960-
Relidslateral_referencers,
19611940
IndexClauseSet*clauseset,
19621941
List**joinorclauses)
19631942
{
@@ -1969,11 +1948,7 @@ match_join_clauses_to_index(PlannerInfo *root,
19691948
RestrictInfo*rinfo= (RestrictInfo*)lfirst(lc);
19701949

19711950
/* Check if clause can be moved to this rel */
1972-
if (!join_clause_is_movable_to(rinfo,rel->relid))
1973-
continue;
1974-
1975-
/* Not useful if it conflicts with any LATERAL references */
1976-
if (bms_overlap(rinfo->clause_relids,lateral_referencers))
1951+
if (!join_clause_is_movable_to(rinfo,rel))
19771952
continue;
19781953

19791954
/* Potentially usable, so see if it matches the index or is an OR */
@@ -1991,7 +1966,6 @@ match_join_clauses_to_index(PlannerInfo *root,
19911966
*/
19921967
staticvoid
19931968
match_eclass_clauses_to_index(PlannerInfo*root,IndexOptInfo*index,
1994-
Relidslateral_referencers,
19951969
IndexClauseSet*clauseset)
19961970
{
19971971
intindexcol;
@@ -2012,7 +1986,7 @@ match_eclass_clauses_to_index(PlannerInfo *root, IndexOptInfo *index,
20121986
index->rel,
20131987
ec_member_matches_indexcol,
20141988
(void*)&arg,
2015-
lateral_referencers);
1989+
index->rel->lateral_referencers);
20161990

20171991
/*
20181992
* We have to check whether the results actually do match the index,
@@ -2644,7 +2618,7 @@ check_partial_indexes(PlannerInfo *root, RelOptInfo *rel)
26442618
RestrictInfo*rinfo= (RestrictInfo*)lfirst(lc);
26452619

26462620
/* Check if clause can be moved to this rel */
2647-
if (!join_clause_is_movable_to(rinfo,rel->relid))
2621+
if (!join_clause_is_movable_to(rinfo,rel))
26482622
continue;
26492623

26502624
clauselist=lappend(clauselist,rinfo);

0 commit comments

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