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

Commitd04e255

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 parentfd005e1 commitd04e255

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
@@ -2379,59 +2379,74 @@ range_table_walker(List *rtable,
23792379

23802380
foreach(rt,rtable)
23812381
{
2382-
RangeTblEntry*rte= (RangeTblEntry*)lfirst(rt);
2382+
RangeTblEntry*rte=lfirst_node(RangeTblEntry,rt);
23832383

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

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

2428-
if (walker(rte->securityQuals,context))
2443+
if (walker(rte->securityQuals,context))
2444+
return true;
2445+
2446+
if (flags&QTW_EXAMINE_RTES_AFTER)
2447+
if (walker(rte,context))
24292448
return true;
24302449

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

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

Lines changed: 93 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,9 @@ static void reduce_outer_joins_pass2(Node *jtnode,
111111
staticNode*remove_useless_results_recurse(PlannerInfo*root,Node*jtnode);
112112
staticintget_result_relid(PlannerInfo*root,Node*jtnode);
113113
staticvoidremove_result_refs(PlannerInfo*root,intvarno,Node*newjtloc);
114-
staticboolfind_dependent_phvs(Node*node,intvarno);
114+
staticboolfind_dependent_phvs(PlannerInfo*root,intvarno);
115+
staticboolfind_dependent_phvs_in_jointree(PlannerInfo*root,
116+
Node*node,intvarno);
115117
staticvoidsubstitute_phv_relids(Node*node,
116118
intvarno,Relidssubrelids);
117119
staticvoidfix_append_rel_relids(List*append_rel_list,intvarno,
@@ -2779,9 +2781,12 @@ reduce_outer_joins_pass2(Node *jtnode,
27792781
* and not remove the RTE_RESULT: there is noplace else to evaluate the
27802782
* PlaceHolderVar. (That is, in such cases the RTE_RESULT *does* have output
27812783
* columns.) But if the RTE_RESULT is an immediate child of an inner join,
2782-
* we can change the PlaceHolderVar's phrels so as to evaluate it at the
2783-
* inner join instead. This is OK because we really only care that PHVs are
2784-
* evaluated above or below the correct outer joins.
2784+
* we can usually change the PlaceHolderVar's phrels so as to evaluate it at
2785+
* the inner join instead. This is OK because we really only care that PHVs
2786+
* are evaluated above or below the correct outer joins. We can't, however,
2787+
* postpone the evaluation of a PHV to above where it is used; so there are
2788+
* some checks below on whether output PHVs are laterally referenced in the
2789+
* other join input rel(s).
27852790
*
27862791
* We used to try to do this work as part of pull_up_subqueries() where the
27872792
* potentially-optimizable cases get introduced; but it's way simpler, and
@@ -2851,8 +2856,11 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode)
28512856
/*
28522857
* We can drop RTE_RESULT rels from the fromlist so long as at least
28532858
* one child remains, since joining to a one-row table changes
2854-
* nothing. The easiest way to mechanize this rule is to modify the
2855-
* list in-place, using list_delete_cell.
2859+
* nothing. (But we can't drop a RTE_RESULT that computes PHV(s) that
2860+
* are needed by some sibling. The cleanup transformation below would
2861+
* reassign the PHVs to be computed at the join, which is too late for
2862+
* the sibling's use.) The easiest way to mechanize this rule is to
2863+
* modify the list in-place, using list_delete_cell.
28562864
*/
28572865
prev=NULL;
28582866
for (cell=list_head(f->fromlist);cell;cell=next)
@@ -2867,12 +2875,14 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode)
28672875
next=lnext(cell);
28682876

28692877
/*
2870-
* If it's an RTE_RESULT with at least one sibling, we can drop
2871-
* it. We don't yet know what the inner join's final relid set
2872-
* will be, so postpone cleanup of PHVs etc till after this loop.
2878+
* If it's an RTE_RESULT with at least one sibling, and no sibling
2879+
* references dependent PHVs, we can drop it. We don't yet know
2880+
* what the inner join's final relid set will be, so postpone
2881+
* cleanup of PHVs etc till after this loop.
28732882
*/
28742883
if (list_length(f->fromlist)>1&&
2875-
(varno=get_result_relid(root,child))!=0)
2884+
(varno=get_result_relid(root,child))!=0&&
2885+
!find_dependent_phvs_in_jointree(root, (Node*)f,varno))
28762886
{
28772887
f->fromlist=list_delete_cell(f->fromlist,cell,prev);
28782888
result_relids=bms_add_member(result_relids,varno);
@@ -2925,8 +2935,18 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode)
29252935
* the join with a FromExpr with just the other side; and if
29262936
* the qual is empty (JOIN ON TRUE) then we can omit the
29272937
* FromExpr as well.
2938+
*
2939+
* Just as in the FromExpr case, we can't simplify if the
2940+
* other input rel references any PHVs that are marked as to
2941+
* be evaluated at the RTE_RESULT rel, because we can't
2942+
* postpone their evaluation in that case. But we only have
2943+
* to check this in cases where it's syntactically legal for
2944+
* the other input to have a LATERAL reference to the
2945+
* RTE_RESULT rel. Only RHSes of inner and left joins are
2946+
* allowed to have such refs.
29282947
*/
2929-
if ((varno=get_result_relid(root,j->larg))!=0)
2948+
if ((varno=get_result_relid(root,j->larg))!=0&&
2949+
!find_dependent_phvs_in_jointree(root,j->rarg,varno))
29302950
{
29312951
remove_result_refs(root,varno,j->rarg);
29322952
if (j->quals)
@@ -2955,6 +2975,8 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode)
29552975
* strength-reduced to a plain inner join, since each LHS row
29562976
* necessarily has exactly one join partner. So we can always
29572977
* discard the RHS, much as in the JOIN_INNER case above.
2978+
* (Again, the LHS could not contain a lateral reference to
2979+
* the RHS.)
29582980
*
29592981
* Otherwise, it's still true that each LHS row should be
29602982
* returned exactly once, and since the RHS returns no columns
@@ -2965,7 +2987,7 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode)
29652987
*/
29662988
if ((varno=get_result_relid(root,j->rarg))!=0&&
29672989
(j->quals==NULL||
2968-
!find_dependent_phvs((Node*)root->parse,varno)))
2990+
!find_dependent_phvs(root,varno)))
29692991
{
29702992
remove_result_refs(root,varno,j->larg);
29712993
jtnode=j->larg;
@@ -2975,7 +2997,7 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode)
29752997
/* Mirror-image of the JOIN_LEFT case */
29762998
if ((varno=get_result_relid(root,j->larg))!=0&&
29772999
(j->quals==NULL||
2978-
!find_dependent_phvs((Node*)root->parse,varno)))
3000+
!find_dependent_phvs(root,varno)))
29793001
{
29803002
remove_result_refs(root,varno,j->rarg);
29813003
jtnode=j->rarg;
@@ -2996,7 +3018,7 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode)
29963018
*/
29973019
if ((varno=get_result_relid(root,j->rarg))!=0)
29983020
{
2999-
Assert(!find_dependent_phvs((Node*)root->parse,varno));
3021+
Assert(!find_dependent_phvs(root,varno));
30003022
remove_result_refs(root,varno,j->larg);
30013023
if (j->quals)
30023024
jtnode= (Node*)
@@ -3080,6 +3102,12 @@ remove_result_refs(PlannerInfo *root, int varno, Node *newjtloc)
30803102
/*
30813103
* find_dependent_phvs - are there any PlaceHolderVars whose relids are
30823104
* exactly the given varno?
3105+
*
3106+
* find_dependent_phvs should be used when we want to see if there are
3107+
* any such PHVs anywhere in the Query. Another use-case is to see if
3108+
* a subtree of the join tree contains such PHVs; but for that, we have
3109+
* to look not only at the join tree nodes themselves but at the
3110+
* referenced RTEs. For that, use find_dependent_phvs_in_jointree.
30833111
*/
30843112

30853113
typedefstruct
@@ -3126,20 +3154,65 @@ find_dependent_phvs_walker(Node *node,
31263154
}
31273155

31283156
staticbool
3129-
find_dependent_phvs(Node*node,intvarno)
3157+
find_dependent_phvs(PlannerInfo*root,intvarno)
3158+
{
3159+
find_dependent_phvs_contextcontext;
3160+
3161+
/* If there are no PHVs anywhere, we needn't work hard */
3162+
if (root->glob->lastPHId==0)
3163+
return false;
3164+
3165+
context.relids=bms_make_singleton(varno);
3166+
context.sublevels_up=0;
3167+
3168+
returnquery_tree_walker(root->parse,
3169+
find_dependent_phvs_walker,
3170+
(void*)&context,
3171+
0);
3172+
}
3173+
3174+
staticbool
3175+
find_dependent_phvs_in_jointree(PlannerInfo*root,Node*node,intvarno)
31303176
{
31313177
find_dependent_phvs_contextcontext;
3178+
Relidssubrelids;
3179+
intrelid;
3180+
3181+
/* If there are no PHVs anywhere, we needn't work hard */
3182+
if (root->glob->lastPHId==0)
3183+
return false;
31323184

31333185
context.relids=bms_make_singleton(varno);
31343186
context.sublevels_up=0;
31353187

31363188
/*
3137-
* Must be prepared to start with a Query or a bare expression tree.
3189+
* See if the jointree fragment itself contains references (in join quals)
3190+
*/
3191+
if (find_dependent_phvs_walker(node,&context))
3192+
return true;
3193+
3194+
/*
3195+
* Otherwise, identify the set of referenced RTEs (we can ignore joins,
3196+
* since they should be flattened already, so their join alias lists no
3197+
* longer matter), and tediously check each RTE. We can ignore RTEs that
3198+
* are not marked LATERAL, though, since they couldn't possibly contain
3199+
* any cross-references to other RTEs.
31383200
*/
3139-
returnquery_or_expression_tree_walker(node,
3140-
find_dependent_phvs_walker,
3141-
(void*)&context,
3142-
0);
3201+
subrelids=get_relids_in_jointree(node, false);
3202+
relid=-1;
3203+
while ((relid=bms_next_member(subrelids,relid)) >=0)
3204+
{
3205+
RangeTblEntry*rte=rt_fetch(relid,root->parse->rtable);
3206+
3207+
if (rte->lateral&&
3208+
range_table_entry_walker(rte,
3209+
find_dependent_phvs_walker,
3210+
(void*)&context,
3211+
0))
3212+
return true;
3213+
}
3214+
3215+
return false;
31433216
}
31443217

31453218
/*

‎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
@@ -3240,6 +3240,32 @@ where t1.unique2 < 42 and t1.stringu1 > t2.stringu2;
32403240
11 | WFAAAA | 3 | LKIAAA
32413241
(1 row)
32423242

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

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

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

1018+
-- Here's a variant that we can't fold too aggressively, though,
1019+
-- or we end up with noplace to evaluate the lateral PHV
1020+
explain (verbose, costs off)
1021+
select*from
1022+
(select1as x) ss1left join (select2as y) ss2on (true),
1023+
lateral (selectss2.yas zlimit1) ss3;
1024+
select*from
1025+
(select1as x) ss1left join (select2as y) ss2on (true),
1026+
lateral (selectss2.yas zlimit1) ss3;
1027+
10181028
--
10191029
-- test extraction of restriction OR clauses from join OR clause
10201030
-- (we used to only do this for indexable clauses)

0 commit comments

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