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

Commit77ba232

Browse files
committed
Fix nested PlaceHolderVar expressions that appear only in targetlists.
A PlaceHolderVar's expression might contain another, lower-levelPlaceHolderVar. If the outer PlaceHolderVar is used, the inner onecertainly will be also, and so we have to make sure that both of them getinto the placeholder_list with correct ph_may_need values during theinitial pre-scan of the query (before deconstruct_jointree starts).We did this correctly for PlaceHolderVars appearing in the query quals,but overlooked the issue for those appearing in the top-level targetlist;with the result that nested placeholders referenced only in the targetlistdid not work correctly, as illustrated in bug #6154.While at it, add some error checking to find_placeholder_info to ensurethat we don't try to create new placeholders after it's too late to do so;they have to all be created before deconstruct_jointree starts.Back-patch to 8.4 where the PlaceHolderVar mechanism was introduced.
1 parentd82a9d2 commit77ba232

File tree

8 files changed

+163
-40
lines changed

8 files changed

+163
-40
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3491,7 +3491,7 @@ set_rel_width(PlannerInfo *root, RelOptInfo *rel)
34913491
elseif (IsA(node,PlaceHolderVar))
34923492
{
34933493
PlaceHolderVar*phv= (PlaceHolderVar*)node;
3494-
PlaceHolderInfo*phinfo=find_placeholder_info(root,phv);
3494+
PlaceHolderInfo*phinfo=find_placeholder_info(root,phv, false);
34953495

34963496
tuple_width+=phinfo->ph_width;
34973497
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -850,7 +850,7 @@ generate_base_implied_equalities_no_const(PlannerInfo *root,
850850
PVC_RECURSE_AGGREGATES,
851851
PVC_INCLUDE_PLACEHOLDERS);
852852

853-
add_vars_to_targetlist(root,vars,ec->ec_relids);
853+
add_vars_to_targetlist(root,vars,ec->ec_relids, false);
854854
list_free(vars);
855855
}
856856
}

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

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ build_base_rel_tlists(PlannerInfo *root, List *final_tlist)
137137

138138
if (tlist_vars!=NIL)
139139
{
140-
add_vars_to_targetlist(root,tlist_vars,bms_make_singleton(0));
140+
add_vars_to_targetlist(root,tlist_vars,bms_make_singleton(0), true);
141141
list_free(tlist_vars);
142142
}
143143
}
@@ -151,10 +151,15 @@ build_base_rel_tlists(PlannerInfo *root, List *final_tlist)
151151
*
152152
* The list may also contain PlaceHolderVars. These don't necessarily
153153
* have a single owning relation; we keep their attr_needed info in
154-
* root->placeholder_list instead.
154+
* root->placeholder_list instead. If create_new_ph is true, it's OK
155+
* to create new PlaceHolderInfos, and we also have to update ph_may_need;
156+
* otherwise, the PlaceHolderInfos must already exist, and we should only
157+
* update their ph_needed. (It should be true before deconstruct_jointree
158+
* begins, and false after that.)
155159
*/
156160
void
157-
add_vars_to_targetlist(PlannerInfo*root,List*vars,Relidswhere_needed)
161+
add_vars_to_targetlist(PlannerInfo*root,List*vars,
162+
Relidswhere_needed,boolcreate_new_ph)
158163
{
159164
ListCell*temp;
160165

@@ -185,18 +190,20 @@ add_vars_to_targetlist(PlannerInfo *root, List *vars, Relids where_needed)
185190
elseif (IsA(node,PlaceHolderVar))
186191
{
187192
PlaceHolderVar*phv= (PlaceHolderVar*)node;
188-
PlaceHolderInfo*phinfo=find_placeholder_info(root,phv);
193+
PlaceHolderInfo*phinfo=find_placeholder_info(root,phv,
194+
create_new_ph);
189195

196+
/* Always adjust ph_needed */
190197
phinfo->ph_needed=bms_add_members(phinfo->ph_needed,
191198
where_needed);
192199

193200
/*
194-
*Update ph_may_need too.This is currently only necessary when
195-
*being called from build_base_rel_tlists, but we may as well do
196-
*it always.
201+
*If we are creating PlaceHolderInfos, mark them with the
202+
*correct maybe-needed locations. Otherwise, it's too late to
203+
*change that.
197204
*/
198-
phinfo->ph_may_need=bms_add_members(phinfo->ph_may_need,
199-
where_needed);
205+
if (create_new_ph)
206+
mark_placeholder_maybe_needed(root,phinfo,where_needed);
200207
}
201208
else
202209
elog(ERROR,"unrecognized node type: %d", (int)nodeTag(node));
@@ -1035,7 +1042,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
10351042
PVC_RECURSE_AGGREGATES,
10361043
PVC_INCLUDE_PLACEHOLDERS);
10371044

1038-
add_vars_to_targetlist(root,vars,relids);
1045+
add_vars_to_targetlist(root,vars,relids, false);
10391046
list_free(vars);
10401047
}
10411048

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

Lines changed: 58 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424

2525
/* Local functions */
2626
staticRelidsfind_placeholders_recurse(PlannerInfo*root,Node*jtnode);
27-
staticvoidfind_placeholders_in_qual(PlannerInfo*root,Node*qual,
27+
staticvoidmark_placeholders_in_expr(PlannerInfo*root,Node*expr,
2828
Relidsrelids);
2929

3030

@@ -50,18 +50,24 @@ make_placeholder_expr(PlannerInfo *root, Expr *expr, Relids phrels)
5050

5151
/*
5252
* find_placeholder_info
53-
*Fetch the PlaceHolderInfo for the given PHV; create it if not found
53+
*Fetch the PlaceHolderInfo for the given PHV
54+
*
55+
* If the PlaceHolderInfo doesn't exist yet, create it if create_new_ph is
56+
* true, else throw an error.
5457
*
5558
* This is separate from make_placeholder_expr because subquery pullup has
5659
* to make PlaceHolderVars for expressions that might not be used at all in
5760
* the upper query, or might not remain after const-expression simplification.
5861
* We build PlaceHolderInfos only for PHVs that are still present in the
5962
* simplified query passed to query_planner().
6063
*
61-
* Note: this should only be called after query_planner() has started.
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.
6267
*/
6368
PlaceHolderInfo*
64-
find_placeholder_info(PlannerInfo*root,PlaceHolderVar*phv)
69+
find_placeholder_info(PlannerInfo*root,PlaceHolderVar*phv,
70+
boolcreate_new_ph)
6571
{
6672
PlaceHolderInfo*phinfo;
6773
ListCell*lc;
@@ -77,6 +83,9 @@ find_placeholder_info(PlannerInfo *root, PlaceHolderVar *phv)
7783
}
7884

7985
/* Not found, so create it */
86+
if (!create_new_ph)
87+
elog(ERROR,"too late to create a new PlaceHolderInfo");
88+
8089
phinfo=makeNode(PlaceHolderInfo);
8190

8291
phinfo->phid=phv->phid;
@@ -157,7 +166,7 @@ find_placeholders_recurse(PlannerInfo *root, Node *jtnode)
157166
/*
158167
* Now process the top-level quals.
159168
*/
160-
find_placeholders_in_qual(root,f->quals,jtrelids);
169+
mark_placeholders_in_expr(root,f->quals,jtrelids);
161170
}
162171
elseif (IsA(jtnode,JoinExpr))
163172
{
@@ -173,7 +182,7 @@ find_placeholders_recurse(PlannerInfo *root, Node *jtnode)
173182
jtrelids=bms_join(leftids,rightids);
174183

175184
/* Process the qual clauses */
176-
find_placeholders_in_qual(root,j->quals,jtrelids);
185+
mark_placeholders_in_expr(root,j->quals,jtrelids);
177186
}
178187
else
179188
{
@@ -185,14 +194,15 @@ find_placeholders_recurse(PlannerInfo *root, Node *jtnode)
185194
}
186195

187196
/*
188-
* find_placeholders_in_qual
189-
*Process a qual clause for find_placeholders_in_jointree.
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.
190200
*
191201
* relids is the syntactic join level to mark as the "maybe needed" level
192-
* for each PlaceHolderVar found in thequal clause.
202+
* for each PlaceHolderVar found in theexpression.
193203
*/
194204
staticvoid
195-
find_placeholders_in_qual(PlannerInfo*root,Node*qual,Relidsrelids)
205+
mark_placeholders_in_expr(PlannerInfo*root,Node*expr,Relidsrelids)
196206
{
197207
List*vars;
198208
ListCell*vl;
@@ -201,7 +211,7 @@ find_placeholders_in_qual(PlannerInfo *root, Node *qual, Relids relids)
201211
* pull_var_clause does more than we need here, but it'll do and it's
202212
* convenient to use.
203213
*/
204-
vars=pull_var_clause(qual,
214+
vars=pull_var_clause(expr,
205215
PVC_RECURSE_AGGREGATES,
206216
PVC_INCLUDE_PLACEHOLDERS);
207217
foreach(vl,vars)
@@ -214,25 +224,47 @@ find_placeholders_in_qual(PlannerInfo *root, Node *qual, Relids relids)
214224
continue;
215225

216226
/* Create a PlaceHolderInfo entry if there's not one already */
217-
phinfo=find_placeholder_info(root,phv);
218-
219-
/* Mark the PHV as possibly needed at the qual's syntactic level */
220-
phinfo->ph_may_need=bms_add_members(phinfo->ph_may_need,relids);
227+
phinfo=find_placeholder_info(root,phv, true);
221228

222-
/*
223-
* This is a bit tricky: the PHV's contained expression may contain
224-
* other, lower-level PHVs. We need to get those into the
225-
* PlaceHolderInfo list, but they aren't going to be needed where the
226-
* outer PHV is referenced. Rather, they'll be needed where the outer
227-
* PHV is evaluated. We can estimate that (conservatively) as the
228-
* syntactic location of the PHV's expression. Recurse to take care
229-
* of any such PHVs.
230-
*/
231-
find_placeholders_in_qual(root, (Node*)phv->phexpr,phv->phrels);
229+
/* Mark it, and recursively process any contained placeholders */
230+
mark_placeholder_maybe_needed(root,phinfo,relids);
232231
}
233232
list_free(vars);
234233
}
235234

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+
/* Mark the PHV as possibly needed at the given syntactic level */
254+
phinfo->ph_may_need=bms_add_members(phinfo->ph_may_need,relids);
255+
256+
/*
257+
* This is a bit tricky: the PHV's contained expression may contain other,
258+
* lower-level PHVs. We need to get those into the PlaceHolderInfo list,
259+
* but they aren't going to be needed where the outer PHV is referenced.
260+
* Rather, they'll be needed where the outer PHV is evaluated. We can
261+
* estimate that (conservatively) as the syntactic location of the PHV's
262+
* expression. Recurse to take care of any such PHVs.
263+
*/
264+
mark_placeholders_in_expr(root, (Node*)phinfo->ph_var->phexpr,
265+
phinfo->ph_var->phrels);
266+
}
267+
236268
/*
237269
* update_placeholder_eval_levels
238270
*Adjust the target evaluation levels for placeholders
@@ -353,7 +385,7 @@ fix_placeholder_input_needed_levels(PlannerInfo *root)
353385
PVC_RECURSE_AGGREGATES,
354386
PVC_INCLUDE_PLACEHOLDERS);
355387

356-
add_vars_to_targetlist(root,vars,eval_at);
388+
add_vars_to_targetlist(root,vars,eval_at, false);
357389
list_free(vars);
358390
}
359391
}

‎src/include/optimizer/placeholder.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,10 @@
2020
externPlaceHolderVar*make_placeholder_expr(PlannerInfo*root,Expr*expr,
2121
Relidsphrels);
2222
externPlaceHolderInfo*find_placeholder_info(PlannerInfo*root,
23-
PlaceHolderVar*phv);
23+
PlaceHolderVar*phv,boolcreate_new_ph);
2424
externvoidfind_placeholders_in_jointree(PlannerInfo*root);
25+
externvoidmark_placeholder_maybe_needed(PlannerInfo*root,
26+
PlaceHolderInfo*phinfo,Relidsrelids);
2527
externvoidupdate_placeholder_eval_levels(PlannerInfo*root,
2628
SpecialJoinInfo*new_sjinfo);
2729
externvoidfix_placeholder_input_needed_levels(PlannerInfo*root);

‎src/include/optimizer/planmain.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ extern intjoin_collapse_limit;
9292
externvoidadd_base_rels_to_query(PlannerInfo*root,Node*jtnode);
9393
externvoidbuild_base_rel_tlists(PlannerInfo*root,List*final_tlist);
9494
externvoidadd_vars_to_targetlist(PlannerInfo*root,List*vars,
95-
Relidswhere_needed);
95+
Relidswhere_needed,boolcreate_new_ph);
9696
externList*deconstruct_jointree(PlannerInfo*root);
9797
externvoiddistribute_restrictinfo_to_rels(PlannerInfo*root,
9898
RestrictInfo*restrictinfo);

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2488,6 +2488,51 @@ order by c.name;
24882488
(3 rows)
24892489

24902490
rollback;
2491+
--
2492+
-- test incorrect handling of placeholders that only appear in targetlists,
2493+
-- per bug #6154
2494+
--
2495+
SELECT * FROM
2496+
( SELECT 1 as key1 ) sub1
2497+
LEFT JOIN
2498+
( SELECT sub3.key3, sub4.value2, COALESCE(sub4.value2, 66) as value3 FROM
2499+
( SELECT 1 as key3 ) sub3
2500+
LEFT JOIN
2501+
( SELECT sub5.key5, COALESCE(sub6.value1, 1) as value2 FROM
2502+
( SELECT 1 as key5 ) sub5
2503+
LEFT JOIN
2504+
( SELECT 2 as key6, 42 as value1 ) sub6
2505+
ON sub5.key5 = sub6.key6
2506+
) sub4
2507+
ON sub4.key5 = sub3.key3
2508+
) sub2
2509+
ON sub1.key1 = sub2.key3;
2510+
key1 | key3 | value2 | value3
2511+
------+------+--------+--------
2512+
1 | 1 | 1 | 1
2513+
(1 row)
2514+
2515+
-- test the path using join aliases, too
2516+
SELECT * FROM
2517+
( SELECT 1 as key1 ) sub1
2518+
LEFT JOIN
2519+
( SELECT sub3.key3, value2, COALESCE(value2, 66) as value3 FROM
2520+
( SELECT 1 as key3 ) sub3
2521+
LEFT JOIN
2522+
( SELECT sub5.key5, COALESCE(sub6.value1, 1) as value2 FROM
2523+
( SELECT 1 as key5 ) sub5
2524+
LEFT JOIN
2525+
( SELECT 2 as key6, 42 as value1 ) sub6
2526+
ON sub5.key5 = sub6.key6
2527+
) sub4
2528+
ON sub4.key5 = sub3.key3
2529+
) sub2
2530+
ON sub1.key1 = sub2.key3;
2531+
key1 | key3 | value2 | value3
2532+
------+------+--------+--------
2533+
1 | 1 | 1 | 1
2534+
(1 row)
2535+
24912536
--
24922537
-- test the corner cases FULL JOIN ON TRUE and FULL JOIN ON FALSE
24932538
--

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -602,6 +602,43 @@ order by c.name;
602602

603603
rollback;
604604

605+
--
606+
-- test incorrect handling of placeholders that only appear in targetlists,
607+
-- per bug #6154
608+
--
609+
SELECT*FROM
610+
(SELECT1as key1 ) sub1
611+
LEFT JOIN
612+
(SELECTsub3.key3,sub4.value2, COALESCE(sub4.value2,66)as value3FROM
613+
(SELECT1as key3 ) sub3
614+
LEFT JOIN
615+
(SELECTsub5.key5, COALESCE(sub6.value1,1)as value2FROM
616+
(SELECT1as key5 ) sub5
617+
LEFT JOIN
618+
(SELECT2as key6,42as value1 ) sub6
619+
ONsub5.key5=sub6.key6
620+
) sub4
621+
ONsub4.key5=sub3.key3
622+
) sub2
623+
ONsub1.key1=sub2.key3;
624+
625+
-- test the path using join aliases, too
626+
SELECT*FROM
627+
(SELECT1as key1 ) sub1
628+
LEFT JOIN
629+
(SELECTsub3.key3, value2, COALESCE(value2,66)as value3FROM
630+
(SELECT1as key3 ) sub3
631+
LEFT JOIN
632+
(SELECTsub5.key5, COALESCE(sub6.value1,1)as value2FROM
633+
(SELECT1as key5 ) sub5
634+
LEFT JOIN
635+
(SELECT2as key6,42as value1 ) sub6
636+
ONsub5.key5=sub6.key6
637+
) sub4
638+
ONsub4.key5=sub3.key3
639+
) sub2
640+
ONsub1.key1=sub2.key3;
641+
605642
--
606643
-- test the corner cases FULL JOIN ON TRUE and FULL JOIN ON FALSE
607644
--

0 commit comments

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