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

Commit8650245

Browse files
tglsfdcpull[bot]
authored andcommitted
Use an explicit state flag to control PlaceHolderInfo creation.
Up to now, callers of find_placeholder_info() were required to passa flag indicating if it's OK to make a new PlaceHolderInfo. That'dbe fine if the callers had free choice, but they do not. Once webegin deconstruct_jointree() it's no longer OK to make more PHIs;while callers before that always want to create a PHI if it's notthere already. So there's no freedom of action, only the opportunityto cause bugs by creating PHIs too late. Let's get rid of that infavor of adding a state flag PlannerInfo.placeholdersFrozen, whichwe can set at the point where it's no longer OK to make more PHIs.This patch also simplifies a couple of call sites that were usingcomplicated logic to avoid calling find_placeholder_info() as muchas possible. Now that that lookup is O(1) thanks to the previouscommit, the extra bitmap manipulations are probably a net negative.Discussion:https://postgr.es/m/1405792.1660677844@sss.pgh.pa.us
1 parent8a8a672 commit8650245

File tree

12 files changed

+42
-49
lines changed

12 files changed

+42
-49
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6245,7 +6245,7 @@ set_rel_width(PlannerInfo *root, RelOptInfo *rel)
62456245
* scanning this rel, so be sure to include it in reltarget->cost.
62466246
*/
62476247
PlaceHolderVar*phv= (PlaceHolderVar*)node;
6248-
PlaceHolderInfo*phinfo=find_placeholder_info(root,phv, false);
6248+
PlaceHolderInfo*phinfo=find_placeholder_info(root,phv);
62496249
QualCostcost;
62506250

62516251
tuple_width+=phinfo->ph_width;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1323,7 +1323,7 @@ generate_base_implied_equalities_no_const(PlannerInfo *root,
13231323
PVC_RECURSE_WINDOWFUNCS |
13241324
PVC_INCLUDE_PLACEHOLDERS);
13251325

1326-
add_vars_to_targetlist(root,vars,ec->ec_relids, false);
1326+
add_vars_to_targetlist(root,vars,ec->ec_relids);
13271327
list_free(vars);
13281328
}
13291329
}

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

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4915,13 +4915,8 @@ replace_nestloop_params_mutator(Node *node, PlannerInfo *root)
49154915
/* Upper-level PlaceHolderVars should be long gone at this point */
49164916
Assert(phv->phlevelsup==0);
49174917

4918-
/*
4919-
* Check whether we need to replace the PHV. We use bms_overlap as a
4920-
* cheap/quick test to see if the PHV might be evaluated in the outer
4921-
* rels, and then grab its PlaceHolderInfo to tell for sure.
4922-
*/
4923-
if (!bms_overlap(phv->phrels,root->curOuterRels)||
4924-
!bms_is_subset(find_placeholder_info(root,phv, false)->ph_eval_at,
4918+
/* Check whether we need to replace the PHV */
4919+
if (!bms_is_subset(find_placeholder_info(root,phv)->ph_eval_at,
49254920
root->curOuterRels))
49264921
{
49274922
/*

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

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ build_base_rel_tlists(PlannerInfo *root, List *final_tlist)
189189

190190
if (tlist_vars!=NIL)
191191
{
192-
add_vars_to_targetlist(root,tlist_vars,bms_make_singleton(0), true);
192+
add_vars_to_targetlist(root,tlist_vars,bms_make_singleton(0));
193193
list_free(tlist_vars);
194194
}
195195

@@ -206,7 +206,7 @@ build_base_rel_tlists(PlannerInfo *root, List *final_tlist)
206206
if (having_vars!=NIL)
207207
{
208208
add_vars_to_targetlist(root,having_vars,
209-
bms_make_singleton(0), true);
209+
bms_make_singleton(0));
210210
list_free(having_vars);
211211
}
212212
}
@@ -221,14 +221,12 @@ build_base_rel_tlists(PlannerInfo *root, List *final_tlist)
221221
*
222222
* The list may also contain PlaceHolderVars. These don't necessarily
223223
* have a single owning relation; we keep their attr_needed info in
224-
* root->placeholder_list instead. If create_new_ph is true, it's OK
225-
* to create new PlaceHolderInfos; otherwise, the PlaceHolderInfos must
226-
* already exist, and we should only update their ph_needed. (This should
227-
* be true before deconstruct_jointree begins, and false after that.)
224+
* root->placeholder_list instead. Find or create the associated
225+
* PlaceHolderInfo entry, and update its ph_needed.
228226
*/
229227
void
230228
add_vars_to_targetlist(PlannerInfo*root,List*vars,
231-
Relidswhere_needed,boolcreate_new_ph)
229+
Relidswhere_needed)
232230
{
233231
ListCell*temp;
234232

@@ -262,8 +260,7 @@ add_vars_to_targetlist(PlannerInfo *root, List *vars,
262260
elseif (IsA(node,PlaceHolderVar))
263261
{
264262
PlaceHolderVar*phv= (PlaceHolderVar*)node;
265-
PlaceHolderInfo*phinfo=find_placeholder_info(root,phv,
266-
create_new_ph);
263+
PlaceHolderInfo*phinfo=find_placeholder_info(root,phv);
267264

268265
phinfo->ph_needed=bms_add_members(phinfo->ph_needed,
269266
where_needed);
@@ -432,7 +429,7 @@ extract_lateral_references(PlannerInfo *root, RelOptInfo *brel, Index rtindex)
432429
* Push Vars into their source relations' targetlists, and PHVs into
433430
* root->placeholder_list.
434431
*/
435-
add_vars_to_targetlist(root,newvars,where_needed, true);
432+
add_vars_to_targetlist(root,newvars,where_needed);
436433

437434
/* Remember the lateral references for create_lateral_join_info */
438435
brel->lateral_vars=newvars;
@@ -493,8 +490,7 @@ create_lateral_join_info(PlannerInfo *root)
493490
elseif (IsA(node,PlaceHolderVar))
494491
{
495492
PlaceHolderVar*phv= (PlaceHolderVar*)node;
496-
PlaceHolderInfo*phinfo=find_placeholder_info(root,phv,
497-
false);
493+
PlaceHolderInfo*phinfo=find_placeholder_info(root,phv);
498494

499495
found_laterals= true;
500496
lateral_relids=bms_add_members(lateral_relids,
@@ -691,6 +687,14 @@ deconstruct_jointree(PlannerInfo *root)
691687
Relidsinner_join_rels;
692688
List*postponed_qual_list=NIL;
693689

690+
/*
691+
* After this point, no more PlaceHolderInfos may be made, because
692+
* make_outerjoininfo and update_placeholder_eval_levels require all
693+
* active placeholders to be present in root->placeholder_list while we
694+
* crawl up the join tree.
695+
*/
696+
root->placeholdersFrozen= true;
697+
694698
/* Start recursion at top of jointree */
695699
Assert(root->parse->jointree!=NULL&&
696700
IsA(root->parse->jointree,FromExpr));
@@ -1866,7 +1870,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
18661870
PVC_RECURSE_WINDOWFUNCS |
18671871
PVC_INCLUDE_PLACEHOLDERS);
18681872

1869-
add_vars_to_targetlist(root,vars,relids, false);
1873+
add_vars_to_targetlist(root,vars,relids);
18701874
list_free(vars);
18711875
}
18721876

@@ -2380,7 +2384,7 @@ process_implied_equality(PlannerInfo *root,
23802384
PVC_RECURSE_WINDOWFUNCS |
23812385
PVC_INCLUDE_PLACEHOLDERS);
23822386

2383-
add_vars_to_targetlist(root,vars,relids, false);
2387+
add_vars_to_targetlist(root,vars,relids);
23842388
list_free(vars);
23852389
}
23862390

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -635,6 +635,7 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
635635
root->qual_security_level=0;
636636
root->hasPseudoConstantQuals= false;
637637
root->hasAlternativeSubPlans= false;
638+
root->placeholdersFrozen= false;
638639
root->hasRecursion=hasRecursion;
639640
if (hasRecursion)
640641
root->wt_param_id=assign_special_exec_param(root);

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1011,6 +1011,7 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte,
10111011
subroot->grouping_map=NULL;
10121012
subroot->minmax_aggs=NIL;
10131013
subroot->qual_security_level=0;
1014+
subroot->placeholdersFrozen= false;
10141015
subroot->hasRecursion= false;
10151016
subroot->wt_param_id=-1;
10161017
subroot->non_recursive_path=NULL;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ expand_inherited_rtentry(PlannerInfo *root, RelOptInfo *rel,
291291
* Add the newly added Vars to parent's reltarget. We needn't worry
292292
* about the children's reltargets, they'll be made later.
293293
*/
294-
add_vars_to_targetlist(root,newvars,bms_make_singleton(0), false);
294+
add_vars_to_targetlist(root,newvars,bms_make_singleton(0));
295295
}
296296

297297
table_close(oldrelation,NoLock);

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

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,7 @@ process_subquery_nestloop_params(PlannerInfo *root, List *subplan_params)
470470
ListCell*lc;
471471

472472
/* If not from a nestloop outer rel, complain */
473-
if (!bms_is_subset(find_placeholder_info(root,phv, false)->ph_eval_at,
473+
if (!bms_is_subset(find_placeholder_info(root,phv)->ph_eval_at,
474474
root->curOuterRels))
475475
elog(ERROR,"non-LATERAL parameter required by subquery");
476476

@@ -517,8 +517,7 @@ identify_current_nestloop_params(PlannerInfo *root, Relids leftrelids)
517517

518518
/*
519519
* We are looking for Vars and PHVs that can be supplied by the
520-
* lefthand rels. The "bms_overlap" test is just an optimization to
521-
* allow skipping find_placeholder_info() if the PHV couldn't match.
520+
* lefthand rels.
522521
*/
523522
if (IsA(nlp->paramval,Var)&&
524523
bms_is_member(nlp->paramval->varno,leftrelids))
@@ -528,11 +527,8 @@ identify_current_nestloop_params(PlannerInfo *root, Relids leftrelids)
528527
result=lappend(result,nlp);
529528
}
530529
elseif (IsA(nlp->paramval,PlaceHolderVar)&&
531-
bms_overlap(((PlaceHolderVar*)nlp->paramval)->phrels,
532-
leftrelids)&&
533530
bms_is_subset(find_placeholder_info(root,
534-
(PlaceHolderVar*)nlp->paramval,
535-
false)->ph_eval_at,
531+
(PlaceHolderVar*)nlp->paramval)->ph_eval_at,
536532
leftrelids))
537533
{
538534
root->curOuterParams=foreach_delete_current(root->curOuterParams,

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

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -52,22 +52,19 @@ make_placeholder_expr(PlannerInfo *root, Expr *expr, Relids phrels)
5252
* find_placeholder_info
5353
*Fetch the PlaceHolderInfo for the given PHV
5454
*
55-
* If the PlaceHolderInfo doesn't exist yet, create it ifcreate_new_ph is
56-
*true, else throw an error.
55+
* If the PlaceHolderInfo doesn't exist yet, create it ifwe haven't yet
56+
*frozen the set of PlaceHolderInfos for the query; else throw an error.
5757
*
5858
* This is separate from make_placeholder_expr because subquery pullup has
5959
* to make PlaceHolderVars for expressions that might not be used at all in
6060
* the upper query, or might not remain after const-expression simplification.
6161
* We build PlaceHolderInfos only for PHVs that are still present in the
6262
* simplified query passed to query_planner().
6363
*
64-
* Note: this should only be called after query_planner() has started. Also,
65-
* create_new_ph must not be true after deconstruct_jointree begins, because
66-
* make_outerjoininfo assumes that we already know about all placeholders.
64+
* Note: this should only be called after query_planner() has started.
6765
*/
6866
PlaceHolderInfo*
69-
find_placeholder_info(PlannerInfo*root,PlaceHolderVar*phv,
70-
boolcreate_new_ph)
67+
find_placeholder_info(PlannerInfo*root,PlaceHolderVar*phv)
7168
{
7269
PlaceHolderInfo*phinfo;
7370
Relidsrels_used;
@@ -87,7 +84,7 @@ find_placeholder_info(PlannerInfo *root, PlaceHolderVar *phv,
8784
}
8885

8986
/* Not found, so create it */
90-
if (!create_new_ph)
87+
if (root->placeholdersFrozen)
9188
elog(ERROR,"too late to create a new PlaceHolderInfo");
9289

9390
phinfo=makeNode(PlaceHolderInfo);
@@ -166,16 +163,13 @@ find_placeholder_info(PlannerInfo *root, PlaceHolderVar *phv,
166163
*
167164
* We don't need to look at the targetlist because build_base_rel_tlists()
168165
* will already have made entries for any PHVs in the tlist.
169-
*
170-
* This is called before we begin deconstruct_jointree. Once we begin
171-
* deconstruct_jointree, all active placeholders must be present in
172-
* root->placeholder_list, because make_outerjoininfo and
173-
* update_placeholder_eval_levels require this info to be available
174-
* while we crawl up the join tree.
175166
*/
176167
void
177168
find_placeholders_in_jointree(PlannerInfo*root)
178169
{
170+
/* This must be done before freezing the set of PHIs */
171+
Assert(!root->placeholdersFrozen);
172+
179173
/* We need do nothing if the query contains no PlaceHolderVars */
180174
if (root->glob->lastPHId!=0)
181175
{
@@ -265,7 +259,7 @@ find_placeholders_in_expr(PlannerInfo *root, Node *expr)
265259
continue;
266260

267261
/* Create a PlaceHolderInfo entry if there's not one already */
268-
(void)find_placeholder_info(root,phv, true);
262+
(void)find_placeholder_info(root,phv);
269263
}
270264
list_free(vars);
271265
}
@@ -392,7 +386,7 @@ fix_placeholder_input_needed_levels(PlannerInfo *root)
392386
PVC_RECURSE_WINDOWFUNCS |
393387
PVC_INCLUDE_PLACEHOLDERS);
394388

395-
add_vars_to_targetlist(root,vars,phinfo->ph_eval_at, false);
389+
add_vars_to_targetlist(root,vars,phinfo->ph_eval_at);
396390
list_free(vars);
397391
}
398392
}

‎src/include/nodes/pathnodes.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,8 @@ struct PlannerInfo
454454
boolhasPseudoConstantQuals;
455455
/* true if we've made any of those */
456456
boolhasAlternativeSubPlans;
457+
/* true once we're no longer allowed to add PlaceHolderInfos */
458+
boolplaceholdersFrozen;
457459
/* true if planning a recursive WITH item */
458460
boolhasRecursion;
459461

0 commit comments

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