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

Commit1b1d3d9

Browse files
committed
Remove ph_may_need from PlaceHolderInfo, with attendant simplifications.
The planner logic that attempted to make a preliminary estimate of theph_needed levels for PlaceHolderVars seems to be completely broken bylateral references. Fortunately, the potential join order optimizationthat this code supported seems to be of relatively little value inpractice; so let's just get rid of it rather than trying to fix it.Getting rid of this allows fairly substantial simplifications inplaceholder.c, too, so planning in such cases should be a bit faster.Issue noted while pursuing bugs reported by Jeremy Evans and AntoninHouska, though this doesn't in itself fix either of their reported cases.What this does do is prevent an Assert crash in the kind of queryillustrated by the added regression test. (I'm not sure that the plan forthat query is stable enough across platforms to be usable as a regressiontest output ... but we'll soon find out from the buildfarm.)Back-patch to 9.3. The problem case can't arise without LATERAL, sono need to touch older branches.
1 parent5368a23 commit1b1d3d9

File tree

10 files changed

+103
-135
lines changed

10 files changed

+103
-135
lines changed

‎src/backend/nodes/copyfuncs.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1957,7 +1957,6 @@ _copyPlaceHolderInfo(const PlaceHolderInfo *from)
19571957
COPY_NODE_FIELD(ph_var);
19581958
COPY_BITMAPSET_FIELD(ph_eval_at);
19591959
COPY_BITMAPSET_FIELD(ph_needed);
1960-
COPY_BITMAPSET_FIELD(ph_may_need);
19611960
COPY_SCALAR_FIELD(ph_width);
19621961

19631962
returnnewnode;

‎src/backend/nodes/equalfuncs.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -822,7 +822,6 @@ _equalPlaceHolderInfo(const PlaceHolderInfo *a, const PlaceHolderInfo *b)
822822
COMPARE_NODE_FIELD(ph_var);
823823
COMPARE_BITMAPSET_FIELD(ph_eval_at);
824824
COMPARE_BITMAPSET_FIELD(ph_needed);
825-
COMPARE_BITMAPSET_FIELD(ph_may_need);
826825
COMPARE_SCALAR_FIELD(ph_width);
827826

828827
return true;

‎src/backend/nodes/outfuncs.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1939,7 +1939,6 @@ _outPlaceHolderInfo(StringInfo str, const PlaceHolderInfo *node)
19391939
WRITE_NODE_FIELD(ph_var);
19401940
WRITE_BITMAPSET_FIELD(ph_eval_at);
19411941
WRITE_BITMAPSET_FIELD(ph_needed);
1942-
WRITE_BITMAPSET_FIELD(ph_may_need);
19431942
WRITE_INT_FIELD(ph_width);
19441943
}
19451944

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -388,8 +388,6 @@ remove_rel_from_query(PlannerInfo *root, int relid, Relids joinrelids)
388388
phinfo->ph_eval_at=bms_add_member(phinfo->ph_eval_at,relid);
389389

390390
phinfo->ph_needed=bms_del_member(phinfo->ph_needed,relid);
391-
/* ph_may_need probably isn't used after this, but fix it anyway */
392-
phinfo->ph_may_need=bms_del_member(phinfo->ph_may_need,relid);
393391
}
394392

395393
/*

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

Lines changed: 9 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -155,10 +155,9 @@ build_base_rel_tlists(PlannerInfo *root, List *final_tlist)
155155
* The list may also contain PlaceHolderVars. These don't necessarily
156156
* have a single owning relation; we keep their attr_needed info in
157157
* root->placeholder_list instead. If create_new_ph is true, it's OK
158-
* to create new PlaceHolderInfos, and we also have to update ph_may_need;
159-
* otherwise, the PlaceHolderInfos must already exist, and we should only
160-
* update their ph_needed. (It should be true before deconstruct_jointree
161-
* begins, and false after that.)
158+
* to create new PlaceHolderInfos; otherwise, the PlaceHolderInfos must
159+
* already exist, and we should only update their ph_needed. (This should
160+
* be true before deconstruct_jointree begins, and false after that.)
162161
*/
163162
void
164163
add_vars_to_targetlist(PlannerInfo*root,List*vars,
@@ -196,20 +195,8 @@ add_vars_to_targetlist(PlannerInfo *root, List *vars,
196195
PlaceHolderInfo*phinfo=find_placeholder_info(root,phv,
197196
create_new_ph);
198197

199-
/* Always adjust ph_needed */
200198
phinfo->ph_needed=bms_add_members(phinfo->ph_needed,
201199
where_needed);
202-
203-
/*
204-
* If we are creating PlaceHolderInfos, mark them with the correct
205-
* maybe-needed locations.Otherwise, it's too late to change
206-
* that, so we'd better not have set ph_needed to more than
207-
* ph_may_need.
208-
*/
209-
if (create_new_ph)
210-
mark_placeholder_maybe_needed(root,phinfo,where_needed);
211-
else
212-
Assert(bms_is_subset(phinfo->ph_needed,phinfo->ph_may_need));
213200
}
214201
else
215202
elog(ERROR,"unrecognized node type: %d", (int)nodeTag(node));
@@ -235,7 +222,7 @@ add_vars_to_targetlist(PlannerInfo *root, List *vars,
235222
* means setting suitable where_needed values for them.
236223
*
237224
* This has to run before deconstruct_jointree, since it might result in
238-
* creation of PlaceHolderInfos or extension of their ph_may_need sets.
225+
* creation of PlaceHolderInfos.
239226
*/
240227
void
241228
find_lateral_references(PlannerInfo*root)
@@ -1005,11 +992,11 @@ make_outerjoininfo(PlannerInfo *root,
1005992

1006993
/*
1007994
* Examine PlaceHolderVars. If a PHV is supposed to be evaluated within
1008-
* this join's nullable side,and it may get used above this join, then
1009-
*ensure that min_righthand contains thefull eval_at set ofthe PHV.
1010-
*This ensures thatthePHV actually can be evaluated within the RHS.
1011-
*Note that this works only because we should already have determined the
1012-
*final eval_at level for any PHVsyntactically within this join.
995+
* this join's nullable side,then ensure that min_righthand contains the
996+
*full eval_at set of thePHV. This ensures thatthe PHV actually can be
997+
*evaluated withintheRHS. Note that this works only because we should
998+
*already have determined the final eval_at level for any PHV
999+
* syntactically within this join.
10131000
*/
10141001
foreach(l,root->placeholder_list)
10151002
{
@@ -1020,11 +1007,6 @@ make_outerjoininfo(PlannerInfo *root,
10201007
if (!bms_is_subset(ph_syn_level,right_rels))
10211008
continue;
10221009

1023-
/* We can also ignore it if it's certainly not used above this join */
1024-
/* XXX this test is probably overly conservative */
1025-
if (bms_is_subset(phinfo->ph_may_need,min_righthand))
1026-
continue;
1027-
10281010
/* Else, prevent join from being formed before we eval the PHV */
10291011
min_righthand=bms_add_members(min_righthand,phinfo->ph_eval_at);
10301012
}

‎src/backend/optimizer/util/placeholder.c

Lines changed: 26 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,8 @@
2323
#include"utils/lsyscache.h"
2424

2525
/* Local functions */
26-
staticRelidsfind_placeholders_recurse(PlannerInfo*root,Node*jtnode);
27-
staticvoidmark_placeholders_in_expr(PlannerInfo*root,Node*expr,
28-
Relidsrelids);
26+
staticvoidfind_placeholders_recurse(PlannerInfo*root,Node*jtnode);
27+
staticvoidfind_placeholders_in_expr(PlannerInfo*root,Node*expr);
2928

3029

3130
/*
@@ -90,16 +89,23 @@ find_placeholder_info(PlannerInfo *root, PlaceHolderVar *phv,
9089

9190
phinfo->phid=phv->phid;
9291
phinfo->ph_var=copyObject(phv);
92+
/* initialize ph_eval_at as the set of contained relids */
9393
phinfo->ph_eval_at=pull_varnos((Node*)phv);
9494
/* ph_eval_at may change later, see update_placeholder_eval_levels */
9595
phinfo->ph_needed=NULL;/* initially it's unused */
96-
phinfo->ph_may_need=NULL;
9796
/* for the moment, estimate width using just the datatype info */
9897
phinfo->ph_width=get_typavgwidth(exprType((Node*)phv->phexpr),
9998
exprTypmod((Node*)phv->phexpr));
10099

101100
root->placeholder_list=lappend(root->placeholder_list,phinfo);
102101

102+
/*
103+
* The PHV's contained expression may contain other, lower-level PHVs. We
104+
* now know we need to get those into the PlaceHolderInfo list, too, so we
105+
* may as well do that immediately.
106+
*/
107+
find_placeholders_in_expr(root, (Node*)phinfo->ph_var->phexpr);
108+
103109
returnphinfo;
104110
}
105111

@@ -119,7 +125,7 @@ find_placeholders_in_jointree(PlannerInfo *root)
119125
/* Start recursion at top of jointree */
120126
Assert(root->parse->jointree!=NULL&&
121127
IsA(root->parse->jointree,FromExpr));
122-
(void)find_placeholders_recurse(root, (Node*)root->parse->jointree);
128+
find_placeholders_recurse(root, (Node*)root->parse->jointree);
123129
}
124130
}
125131

@@ -128,81 +134,59 @@ find_placeholders_in_jointree(PlannerInfo *root)
128134
* One recursion level of find_placeholders_in_jointree.
129135
*
130136
* jtnode is the current jointree node to examine.
131-
*
132-
* The result is the set of base Relids contained in or below jtnode.
133-
* This is just an internal convenience, it's not used at the top level.
134137
*/
135-
staticRelids
138+
staticvoid
136139
find_placeholders_recurse(PlannerInfo*root,Node*jtnode)
137140
{
138-
Relidsjtrelids;
139-
140141
if (jtnode==NULL)
141-
returnNULL;
142+
return;
142143
if (IsA(jtnode,RangeTblRef))
143144
{
144-
intvarno= ((RangeTblRef*)jtnode)->rtindex;
145-
146-
/* No quals to deal with, just return correct result */
147-
jtrelids=bms_make_singleton(varno);
145+
/* No quals to deal with here */
148146
}
149147
elseif (IsA(jtnode,FromExpr))
150148
{
151149
FromExpr*f= (FromExpr*)jtnode;
152150
ListCell*l;
153151

154152
/*
155-
* First, recurse to handle child joins, and form their relid set.
153+
* First, recurse to handle child joins.
156154
*/
157-
jtrelids=NULL;
158155
foreach(l,f->fromlist)
159156
{
160-
Relidssub_relids;
161-
162-
sub_relids=find_placeholders_recurse(root,lfirst(l));
163-
jtrelids=bms_join(jtrelids,sub_relids);
157+
find_placeholders_recurse(root,lfirst(l));
164158
}
165159

166160
/*
167161
* Now process the top-level quals.
168162
*/
169-
mark_placeholders_in_expr(root,f->quals,jtrelids);
163+
find_placeholders_in_expr(root,f->quals);
170164
}
171165
elseif (IsA(jtnode,JoinExpr))
172166
{
173167
JoinExpr*j= (JoinExpr*)jtnode;
174-
Relidsleftids,
175-
rightids;
176168

177169
/*
178-
* First, recurse to handle child joins, and form their relid set.
170+
* First, recurse to handle child joins.
179171
*/
180-
leftids=find_placeholders_recurse(root,j->larg);
181-
rightids=find_placeholders_recurse(root,j->rarg);
182-
jtrelids=bms_join(leftids,rightids);
172+
find_placeholders_recurse(root,j->larg);
173+
find_placeholders_recurse(root,j->rarg);
183174

184175
/* Process the qual clauses */
185-
mark_placeholders_in_expr(root,j->quals,jtrelids);
176+
find_placeholders_in_expr(root,j->quals);
186177
}
187178
else
188-
{
189179
elog(ERROR,"unrecognized node type: %d",
190180
(int)nodeTag(jtnode));
191-
jtrelids=NULL;/* keep compiler quiet */
192-
}
193-
returnjtrelids;
194181
}
195182

196183
/*
197-
* mark_placeholders_in_expr
198-
*Find all PlaceHolderVars in the given expression, and mark them
199-
*as possibly needed at the specified join level.
200-
*
201-
* relids is the syntactic join level to mark as the "maybe needed" level
202-
* for each PlaceHolderVar found in the expression.
184+
* find_placeholders_in_expr
185+
*Find all PlaceHolderVars in the given expression, and create
186+
*PlaceHolderInfo entries for them.
203187
*/
204188
staticvoid
205-
mark_placeholders_in_expr(PlannerInfo*root,Node*expr,Relidsrelids)
189+
find_placeholders_in_expr(PlannerInfo*root,Node*expr)
206190
{
207191
List*vars;
208192
ListCell*vl;
@@ -217,63 +201,17 @@ mark_placeholders_in_expr(PlannerInfo *root, Node *expr, Relids relids)
217201
foreach(vl,vars)
218202
{
219203
PlaceHolderVar*phv= (PlaceHolderVar*)lfirst(vl);
220-
PlaceHolderInfo*phinfo;
221204

222205
/* Ignore any plain Vars */
223206
if (!IsA(phv,PlaceHolderVar))
224207
continue;
225208

226209
/* Create a PlaceHolderInfo entry if there's not one already */
227-
phinfo=find_placeholder_info(root,phv, true);
228-
229-
/* Mark it, and recursively process any contained placeholders */
230-
mark_placeholder_maybe_needed(root,phinfo,relids);
210+
(void)find_placeholder_info(root,phv, true);
231211
}
232212
list_free(vars);
233213
}
234214

235-
/*
236-
* mark_placeholder_maybe_needed
237-
*Mark a placeholder as possibly needed at the specified join level.
238-
*
239-
* relids is the syntactic join level to mark as the "maybe needed" level
240-
* for the placeholder.
241-
*
242-
* This is called during an initial scan of the query's targetlist and quals
243-
* before we begin deconstruct_jointree. Once we begin deconstruct_jointree,
244-
* all active placeholders must be present in root->placeholder_list with
245-
* their correct ph_may_need values, because make_outerjoininfo and
246-
* update_placeholder_eval_levels require this info to be available while
247-
* we crawl up the join tree.
248-
*/
249-
void
250-
mark_placeholder_maybe_needed(PlannerInfo*root,PlaceHolderInfo*phinfo,
251-
Relidsrelids)
252-
{
253-
Relidsest_eval_level;
254-
255-
/* Mark the PHV as possibly needed at the given syntactic level */
256-
phinfo->ph_may_need=bms_add_members(phinfo->ph_may_need,relids);
257-
258-
/*
259-
* This is a bit tricky: the PHV's contained expression may contain other,
260-
* lower-level PHVs. We need to get those into the PlaceHolderInfo list,
261-
* but they aren't going to be needed where the outer PHV is referenced.
262-
* Rather, they'll be needed where the outer PHV is evaluated. We can
263-
* estimate that conservatively as the syntactic location of the PHV's
264-
* expression, but not less than the level of any Vars it contains.
265-
* (Normally the Vars would come from below the syntactic location anyway,
266-
* but this might not be true if the PHV contains any LATERAL references.)
267-
*/
268-
est_eval_level=bms_union(phinfo->ph_var->phrels,phinfo->ph_eval_at);
269-
270-
/* Now recurse to take care of any such PHVs */
271-
mark_placeholders_in_expr(root, (Node*)phinfo->ph_var->phexpr,
272-
est_eval_level);
273-
274-
bms_free(est_eval_level);
275-
}
276-
277215
/*
278216
* update_placeholder_eval_levels
279217
*Adjust the target evaluation levels for placeholders

‎src/include/nodes/relation.h

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1465,18 +1465,9 @@ typedef struct AppendRelInfo
14651465
* then allow it to bubble up like a Var until the ph_needed join level.
14661466
* ph_needed has the same definition as attr_needed for a regular Var.
14671467
*
1468-
* ph_may_need is an initial estimate of ph_needed, formed using the
1469-
* syntactic locations of references to the PHV. We need this in order to
1470-
* determine whether the PHV reference forces a join ordering constraint:
1471-
* if the PHV has to be evaluated below the nullable side of an outer join,
1472-
* and then used above that outer join, we must constrain join order to ensure
1473-
* there's a valid place to evaluate the PHV below the join. The final
1474-
* actual ph_needed level might be lower than ph_may_need, but we can't
1475-
* determine that until later on. Fortunately this doesn't matter for what
1476-
* we need ph_may_need for: if there's a PHV reference syntactically
1477-
* above the outer join, it's not going to be allowed to drop below the outer
1478-
* join, so we would come to the same conclusions about join order even if
1479-
* we had the final ph_needed value to compare to.
1468+
* Notice that when ph_eval_at is a join rather than a single baserel, the
1469+
* PlaceHolderInfo may create constraints on join order: the ph_eval_at join
1470+
* has to be formed below any outer joins that should null the PlaceHolderVar.
14801471
*
14811472
* We create a PlaceHolderInfo only after determining that the PlaceHolderVar
14821473
* is actually referenced in the plan tree, so that unreferenced placeholders
@@ -1491,7 +1482,6 @@ typedef struct PlaceHolderInfo
14911482
PlaceHolderVar*ph_var;/* copy of PlaceHolderVar tree */
14921483
Relidsph_eval_at;/* lowest level we can evaluate value at */
14931484
Relidsph_needed;/* highest level the value is needed at */
1494-
Relidsph_may_need;/* highest level it might be needed at */
14951485
int32ph_width;/* estimated attribute width */
14961486
}PlaceHolderInfo;
14971487

‎src/include/optimizer/placeholder.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@ extern PlaceHolderVar *make_placeholder_expr(PlannerInfo *root, Expr *expr,
2222
externPlaceHolderInfo*find_placeholder_info(PlannerInfo*root,
2323
PlaceHolderVar*phv,boolcreate_new_ph);
2424
externvoidfind_placeholders_in_jointree(PlannerInfo*root);
25-
externvoidmark_placeholder_maybe_needed(PlannerInfo*root,
26-
PlaceHolderInfo*phinfo,Relidsrelids);
2725
externvoidupdate_placeholder_eval_levels(PlannerInfo*root,
2826
SpecialJoinInfo*new_sjinfo);
2927
externvoidfix_placeholder_input_needed_levels(PlannerInfo*root);

0 commit comments

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