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

Commit6ea364e

Browse files
committed
Prevent overly-aggressive collapsing of joins to RTE_RESULT relations.
The RTE_RESULT simplification logic added by commit4be058f had aflaw: it would collapse out a RTE_RESULT that is due to compute aPlaceHolderVar, and reassign the PHV to the parent join level, even ifanother input relation of the join contained a lateral reference tothe PHV. That can't work because the PHV would be computed too late.In practice it led to failures of internal sanity checks later inplanning (either assertion failures or errors such as "failed toconstruct the join relation").To fix, add code to check for the presence of such PHVs in relevantportions of the query tree. Notably, this required refactoringrange_table_walker so that a caller could ask to walk individual RTEsnot the whole list. (It might be a good idea to refactorrange_table_mutator in the same way, if only to keep those functionslooking similar; but I didn't do so here as it wasn't necessary forthe bug fix.)This exercise also taught me that find_dependent_phvs(), as it stood,could only safely be used on the entire Query, not on subtrees.Adjust its API to reflect that; which in passing allows it to havea fast path for the common case of no PHVs anywhere.Per report from Will Leinweber. Back-patch to v12 where the bugwas introduced.Discussion:https://postgr.es/m/CALLb-4xJMd4GZt2YCecMC95H-PafuWNKcmps4HLRx2NHNBfB4g@mail.gmail.com
1 parente0e569e commit6ea364e

File tree

5 files changed

+193
-66
lines changed

5 files changed

+193
-66
lines changed

‎src/backend/nodes/nodeFuncs.c

Lines changed: 61 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -2378,59 +2378,74 @@ range_table_walker(List *rtable,
23782378

23792379
foreach(rt,rtable)
23802380
{
2381-
RangeTblEntry*rte= (RangeTblEntry*)lfirst(rt);
2381+
RangeTblEntry*rte=lfirst_node(RangeTblEntry,rt);
23822382

2383-
/*
2384-
* Walkers might need to examine the RTE node itself either before or
2385-
* after visiting its contents (or, conceivably, both). Note that if
2386-
* you specify neither flag, the walker won't visit the RTE at all.
2387-
*/
2388-
if (flags&QTW_EXAMINE_RTES_BEFORE)
2389-
if (walker(rte,context))
2390-
return true;
2383+
if (range_table_entry_walker(rte,walker,context,flags))
2384+
return true;
2385+
}
2386+
return false;
2387+
}
23912388

2392-
switch (rte->rtekind)
2393-
{
2394-
caseRTE_RELATION:
2395-
if (walker(rte->tablesample,context))
2396-
return true;
2397-
break;
2398-
caseRTE_SUBQUERY:
2399-
if (!(flags&QTW_IGNORE_RT_SUBQUERIES))
2400-
if (walker(rte->subquery,context))
2401-
return true;
2402-
break;
2403-
caseRTE_JOIN:
2404-
if (!(flags&QTW_IGNORE_JOINALIASES))
2405-
if (walker(rte->joinaliasvars,context))
2406-
return true;
2407-
break;
2408-
caseRTE_FUNCTION:
2409-
if (walker(rte->functions,context))
2410-
return true;
2411-
break;
2412-
caseRTE_TABLEFUNC:
2413-
if (walker(rte->tablefunc,context))
2389+
/*
2390+
* Some callers even want to scan the expressions in individual RTEs.
2391+
*/
2392+
bool
2393+
range_table_entry_walker(RangeTblEntry*rte,
2394+
bool (*walker) (),
2395+
void*context,
2396+
intflags)
2397+
{
2398+
/*
2399+
* Walkers might need to examine the RTE node itself either before or
2400+
* after visiting its contents (or, conceivably, both). Note that if you
2401+
* specify neither flag, the walker won't be called on the RTE at all.
2402+
*/
2403+
if (flags&QTW_EXAMINE_RTES_BEFORE)
2404+
if (walker(rte,context))
2405+
return true;
2406+
2407+
switch (rte->rtekind)
2408+
{
2409+
caseRTE_RELATION:
2410+
if (walker(rte->tablesample,context))
2411+
return true;
2412+
break;
2413+
caseRTE_SUBQUERY:
2414+
if (!(flags&QTW_IGNORE_RT_SUBQUERIES))
2415+
if (walker(rte->subquery,context))
24142416
return true;
2415-
break;
2416-
caseRTE_VALUES:
2417-
if (walker(rte->values_lists,context))
2417+
break;
2418+
caseRTE_JOIN:
2419+
if (!(flags&QTW_IGNORE_JOINALIASES))
2420+
if (walker(rte->joinaliasvars,context))
24182421
return true;
2419-
break;
2420-
caseRTE_CTE:
2421-
caseRTE_NAMEDTUPLESTORE:
2422-
caseRTE_RESULT:
2423-
/* nothing to do */
2424-
break;
2425-
}
2422+
break;
2423+
caseRTE_FUNCTION:
2424+
if (walker(rte->functions,context))
2425+
return true;
2426+
break;
2427+
caseRTE_TABLEFUNC:
2428+
if (walker(rte->tablefunc,context))
2429+
return true;
2430+
break;
2431+
caseRTE_VALUES:
2432+
if (walker(rte->values_lists,context))
2433+
return true;
2434+
break;
2435+
caseRTE_CTE:
2436+
caseRTE_NAMEDTUPLESTORE:
2437+
caseRTE_RESULT:
2438+
/* nothing to do */
2439+
break;
2440+
}
24262441

2427-
if (walker(rte->securityQuals,context))
2442+
if (walker(rte->securityQuals,context))
2443+
return true;
2444+
2445+
if (flags&QTW_EXAMINE_RTES_AFTER)
2446+
if (walker(rte,context))
24282447
return true;
24292448

2430-
if (flags&QTW_EXAMINE_RTES_AFTER)
2431-
if (walker(rte,context))
2432-
return true;
2433-
}
24342449
return false;
24352450
}
24362451

‎src/backend/optimizer/prep/prepjointree.c

Lines changed: 93 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,9 @@ static void reduce_outer_joins_pass2(Node *jtnode,
120120
staticNode*remove_useless_results_recurse(PlannerInfo*root,Node*jtnode);
121121
staticintget_result_relid(PlannerInfo*root,Node*jtnode);
122122
staticvoidremove_result_refs(PlannerInfo*root,intvarno,Node*newjtloc);
123-
staticboolfind_dependent_phvs(Node*node,intvarno);
123+
staticboolfind_dependent_phvs(PlannerInfo*root,intvarno);
124+
staticboolfind_dependent_phvs_in_jointree(PlannerInfo*root,
125+
Node*node,intvarno);
124126
staticvoidsubstitute_phv_relids(Node*node,
125127
intvarno,Relidssubrelids);
126128
staticvoidfix_append_rel_relids(List*append_rel_list,intvarno,
@@ -2957,9 +2959,12 @@ reduce_outer_joins_pass2(Node *jtnode,
29572959
* and not remove the RTE_RESULT: there is noplace else to evaluate the
29582960
* PlaceHolderVar. (That is, in such cases the RTE_RESULT *does* have output
29592961
* columns.) But if the RTE_RESULT is an immediate child of an inner join,
2960-
* we can change the PlaceHolderVar's phrels so as to evaluate it at the
2961-
* inner join instead. This is OK because we really only care that PHVs are
2962-
* evaluated above or below the correct outer joins.
2962+
* we can usually change the PlaceHolderVar's phrels so as to evaluate it at
2963+
* the inner join instead. This is OK because we really only care that PHVs
2964+
* are evaluated above or below the correct outer joins. We can't, however,
2965+
* postpone the evaluation of a PHV to above where it is used; so there are
2966+
* some checks below on whether output PHVs are laterally referenced in the
2967+
* other join input rel(s).
29632968
*
29642969
* We used to try to do this work as part of pull_up_subqueries() where the
29652970
* potentially-optimizable cases get introduced; but it's way simpler, and
@@ -3021,8 +3026,11 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode)
30213026
/*
30223027
* We can drop RTE_RESULT rels from the fromlist so long as at least
30233028
* one child remains, since joining to a one-row table changes
3024-
* nothing. The easiest way to mechanize this rule is to modify the
3025-
* list in-place.
3029+
* nothing. (But we can't drop a RTE_RESULT that computes PHV(s) that
3030+
* are needed by some sibling. The cleanup transformation below would
3031+
* reassign the PHVs to be computed at the join, which is too late for
3032+
* the sibling's use.) The easiest way to mechanize this rule is to
3033+
* modify the list in-place.
30263034
*/
30273035
foreach(cell,f->fromlist)
30283036
{
@@ -3035,12 +3043,14 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode)
30353043
lfirst(cell)=child;
30363044

30373045
/*
3038-
* If it's an RTE_RESULT with at least one sibling, we can drop
3039-
* it. We don't yet know what the inner join's final relid set
3040-
* will be, so postpone cleanup of PHVs etc till after this loop.
3046+
* If it's an RTE_RESULT with at least one sibling, and no sibling
3047+
* references dependent PHVs, we can drop it. We don't yet know
3048+
* what the inner join's final relid set will be, so postpone
3049+
* cleanup of PHVs etc till after this loop.
30413050
*/
30423051
if (list_length(f->fromlist)>1&&
3043-
(varno=get_result_relid(root,child))!=0)
3052+
(varno=get_result_relid(root,child))!=0&&
3053+
!find_dependent_phvs_in_jointree(root, (Node*)f,varno))
30443054
{
30453055
f->fromlist=foreach_delete_current(f->fromlist,cell);
30463056
result_relids=bms_add_member(result_relids,varno);
@@ -3091,8 +3101,18 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode)
30913101
* the join with a FromExpr with just the other side; and if
30923102
* the qual is empty (JOIN ON TRUE) then we can omit the
30933103
* FromExpr as well.
3104+
*
3105+
* Just as in the FromExpr case, we can't simplify if the
3106+
* other input rel references any PHVs that are marked as to
3107+
* be evaluated at the RTE_RESULT rel, because we can't
3108+
* postpone their evaluation in that case. But we only have
3109+
* to check this in cases where it's syntactically legal for
3110+
* the other input to have a LATERAL reference to the
3111+
* RTE_RESULT rel. Only RHSes of inner and left joins are
3112+
* allowed to have such refs.
30943113
*/
3095-
if ((varno=get_result_relid(root,j->larg))!=0)
3114+
if ((varno=get_result_relid(root,j->larg))!=0&&
3115+
!find_dependent_phvs_in_jointree(root,j->rarg,varno))
30963116
{
30973117
remove_result_refs(root,varno,j->rarg);
30983118
if (j->quals)
@@ -3121,6 +3141,8 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode)
31213141
* strength-reduced to a plain inner join, since each LHS row
31223142
* necessarily has exactly one join partner. So we can always
31233143
* discard the RHS, much as in the JOIN_INNER case above.
3144+
* (Again, the LHS could not contain a lateral reference to
3145+
* the RHS.)
31243146
*
31253147
* Otherwise, it's still true that each LHS row should be
31263148
* returned exactly once, and since the RHS returns no columns
@@ -3131,7 +3153,7 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode)
31313153
*/
31323154
if ((varno=get_result_relid(root,j->rarg))!=0&&
31333155
(j->quals==NULL||
3134-
!find_dependent_phvs((Node*)root->parse,varno)))
3156+
!find_dependent_phvs(root,varno)))
31353157
{
31363158
remove_result_refs(root,varno,j->larg);
31373159
jtnode=j->larg;
@@ -3141,7 +3163,7 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode)
31413163
/* Mirror-image of the JOIN_LEFT case */
31423164
if ((varno=get_result_relid(root,j->larg))!=0&&
31433165
(j->quals==NULL||
3144-
!find_dependent_phvs((Node*)root->parse,varno)))
3166+
!find_dependent_phvs(root,varno)))
31453167
{
31463168
remove_result_refs(root,varno,j->rarg);
31473169
jtnode=j->rarg;
@@ -3162,7 +3184,7 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode)
31623184
*/
31633185
if ((varno=get_result_relid(root,j->rarg))!=0)
31643186
{
3165-
Assert(!find_dependent_phvs((Node*)root->parse,varno));
3187+
Assert(!find_dependent_phvs(root,varno));
31663188
remove_result_refs(root,varno,j->larg);
31673189
if (j->quals)
31683190
jtnode= (Node*)
@@ -3246,6 +3268,12 @@ remove_result_refs(PlannerInfo *root, int varno, Node *newjtloc)
32463268
/*
32473269
* find_dependent_phvs - are there any PlaceHolderVars whose relids are
32483270
* exactly the given varno?
3271+
*
3272+
* find_dependent_phvs should be used when we want to see if there are
3273+
* any such PHVs anywhere in the Query. Another use-case is to see if
3274+
* a subtree of the join tree contains such PHVs; but for that, we have
3275+
* to look not only at the join tree nodes themselves but at the
3276+
* referenced RTEs. For that, use find_dependent_phvs_in_jointree.
32493277
*/
32503278

32513279
typedefstruct
@@ -3292,20 +3320,65 @@ find_dependent_phvs_walker(Node *node,
32923320
}
32933321

32943322
staticbool
3295-
find_dependent_phvs(Node*node,intvarno)
3323+
find_dependent_phvs(PlannerInfo*root,intvarno)
3324+
{
3325+
find_dependent_phvs_contextcontext;
3326+
3327+
/* If there are no PHVs anywhere, we needn't work hard */
3328+
if (root->glob->lastPHId==0)
3329+
return false;
3330+
3331+
context.relids=bms_make_singleton(varno);
3332+
context.sublevels_up=0;
3333+
3334+
returnquery_tree_walker(root->parse,
3335+
find_dependent_phvs_walker,
3336+
(void*)&context,
3337+
0);
3338+
}
3339+
3340+
staticbool
3341+
find_dependent_phvs_in_jointree(PlannerInfo*root,Node*node,intvarno)
32963342
{
32973343
find_dependent_phvs_contextcontext;
3344+
Relidssubrelids;
3345+
intrelid;
3346+
3347+
/* If there are no PHVs anywhere, we needn't work hard */
3348+
if (root->glob->lastPHId==0)
3349+
return false;
32983350

32993351
context.relids=bms_make_singleton(varno);
33003352
context.sublevels_up=0;
33013353

33023354
/*
3303-
* Must be prepared to start with a Query or a bare expression tree.
3355+
* See if the jointree fragment itself contains references (in join quals)
3356+
*/
3357+
if (find_dependent_phvs_walker(node,&context))
3358+
return true;
3359+
3360+
/*
3361+
* Otherwise, identify the set of referenced RTEs (we can ignore joins,
3362+
* since they should be flattened already, so their join alias lists no
3363+
* longer matter), and tediously check each RTE. We can ignore RTEs that
3364+
* are not marked LATERAL, though, since they couldn't possibly contain
3365+
* any cross-references to other RTEs.
33043366
*/
3305-
returnquery_or_expression_tree_walker(node,
3306-
find_dependent_phvs_walker,
3307-
(void*)&context,
3308-
0);
3367+
subrelids=get_relids_in_jointree(node, false);
3368+
relid=-1;
3369+
while ((relid=bms_next_member(subrelids,relid)) >=0)
3370+
{
3371+
RangeTblEntry*rte=rt_fetch(relid,root->parse->rtable);
3372+
3373+
if (rte->lateral&&
3374+
range_table_entry_walker(rte,
3375+
find_dependent_phvs_walker,
3376+
(void*)&context,
3377+
0))
3378+
return true;
3379+
}
3380+
3381+
return false;
33093382
}
33103383

33113384
/*

‎src/include/nodes/nodeFuncs.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,9 @@ extern bool range_table_walker(List *rtable, bool (*walker) (),
141141
externList*range_table_mutator(List*rtable,Node*(*mutator) (),
142142
void*context,intflags);
143143

144+
externboolrange_table_entry_walker(RangeTblEntry*rte,bool (*walker) (),
145+
void*context,intflags);
146+
144147
externboolquery_or_expression_tree_walker(Node*node,bool (*walker) (),
145148
void*context,intflags);
146149
externNode*query_or_expression_tree_mutator(Node*node,Node*(*mutator) (),

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3242,6 +3242,32 @@ where t1.unique2 < 42 and t1.stringu1 > t2.stringu2;
32423242
11 | WFAAAA | 3 | LKIAAA
32433243
(1 row)
32443244

3245+
-- Here's a variant that we can't fold too aggressively, though,
3246+
-- or we end up with noplace to evaluate the lateral PHV
3247+
explain (verbose, costs off)
3248+
select * from
3249+
(select 1 as x) ss1 left join (select 2 as y) ss2 on (true),
3250+
lateral (select ss2.y as z limit 1) ss3;
3251+
QUERY PLAN
3252+
---------------------------
3253+
Nested Loop
3254+
Output: 1, (2), ((2))
3255+
-> Result
3256+
Output: 2
3257+
-> Limit
3258+
Output: ((2))
3259+
-> Result
3260+
Output: (2)
3261+
(8 rows)
3262+
3263+
select * from
3264+
(select 1 as x) ss1 left join (select 2 as y) ss2 on (true),
3265+
lateral (select ss2.y as z limit 1) ss3;
3266+
x | y | z
3267+
---+---+---
3268+
1 | 2 | 2
3269+
(1 row)
3270+
32453271
--
32463272
-- test inlining of immutable functions
32473273
--

‎src/test/regress/sql/join.sql

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1018,6 +1018,16 @@ select t1.unique2, t1.stringu1, t2.unique1, t2.stringu2 from
10181018
on (subq1.y1=t2.unique1)
10191019
wheret1.unique2<42andt1.stringu1>t2.stringu2;
10201020

1021+
-- Here's a variant that we can't fold too aggressively, though,
1022+
-- or we end up with noplace to evaluate the lateral PHV
1023+
explain (verbose, costs off)
1024+
select*from
1025+
(select1as x) ss1left join (select2as y) ss2on (true),
1026+
lateral (selectss2.yas zlimit1) ss3;
1027+
select*from
1028+
(select1as x) ss1left join (select2as y) ss2on (true),
1029+
lateral (selectss2.yas zlimit1) ss3;
1030+
10211031
--
10221032
-- test inlining of immutable functions
10231033
--

0 commit comments

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