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

Commit8a14bdb

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 parentf600782 commit8a14bdb

File tree

8 files changed

+164
-41
lines changed

8 files changed

+164
-41
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3230,7 +3230,7 @@ set_rel_width(PlannerInfo *root, RelOptInfo *rel)
32303230
elseif (IsA(node,PlaceHolderVar))
32313231
{
32323232
PlaceHolderVar*phv= (PlaceHolderVar*)node;
3233-
PlaceHolderInfo*phinfo=find_placeholder_info(root,phv);
3233+
PlaceHolderInfo*phinfo=find_placeholder_info(root,phv, false);
32343234

32353235
tuple_width+=phinfo->ph_width;
32363236
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -709,7 +709,7 @@ generate_base_implied_equalities_no_const(PlannerInfo *root,
709709
List*vars=pull_var_clause((Node*)cur_em->em_expr,
710710
PVC_INCLUDE_PLACEHOLDERS);
711711

712-
add_vars_to_targetlist(root,vars,ec->ec_relids);
712+
add_vars_to_targetlist(root,vars,ec->ec_relids, false);
713713
list_free(vars);
714714
}
715715
}

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

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

136136
if (tlist_vars!=NIL)
137137
{
138-
add_vars_to_targetlist(root,tlist_vars,bms_make_singleton(0));
138+
add_vars_to_targetlist(root,tlist_vars,bms_make_singleton(0), true);
139139
list_free(tlist_vars);
140140
}
141141
}
@@ -149,10 +149,15 @@ build_base_rel_tlists(PlannerInfo *root, List *final_tlist)
149149
*
150150
* The list may also contain PlaceHolderVars. These don't necessarily
151151
* have a single owning relation; we keep their attr_needed info in
152-
* root->placeholder_list instead.
152+
* root->placeholder_list instead. If create_new_ph is true, it's OK
153+
* to create new PlaceHolderInfos, and we also have to update ph_may_need;
154+
* otherwise, the PlaceHolderInfos must already exist, and we should only
155+
* update their ph_needed. (It should be true before deconstruct_jointree
156+
* begins, and false after that.)
153157
*/
154158
void
155-
add_vars_to_targetlist(PlannerInfo*root,List*vars,Relidswhere_needed)
159+
add_vars_to_targetlist(PlannerInfo*root,List*vars,
160+
Relidswhere_needed,boolcreate_new_ph)
156161
{
157162
ListCell*temp;
158163

@@ -183,17 +188,19 @@ add_vars_to_targetlist(PlannerInfo *root, List *vars, Relids where_needed)
183188
elseif (IsA(node,PlaceHolderVar))
184189
{
185190
PlaceHolderVar*phv= (PlaceHolderVar*)node;
186-
PlaceHolderInfo*phinfo=find_placeholder_info(root,phv);
191+
PlaceHolderInfo*phinfo=find_placeholder_info(root,phv,
192+
create_new_ph);
187193

194+
/* Always adjust ph_needed */
188195
phinfo->ph_needed=bms_add_members(phinfo->ph_needed,
189196
where_needed);
190197
/*
191-
*Update ph_may_need too. This is currently only necessary
192-
*when being called from build_base_rel_tlists, but we may as
193-
*well do it always.
198+
*If we are creating PlaceHolderInfos, mark them with the
199+
*correct maybe-needed locations. Otherwise, it's too late to
200+
*change that.
194201
*/
195-
phinfo->ph_may_need=bms_add_members(phinfo->ph_may_need,
196-
where_needed);
202+
if (create_new_ph)
203+
mark_placeholder_maybe_needed(root,phinfo,where_needed);
197204
}
198205
else
199206
elog(ERROR,"unrecognized node type: %d", (int)nodeTag(node));
@@ -1030,7 +1037,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
10301037
{
10311038
List*vars=pull_var_clause(clause,PVC_INCLUDE_PLACEHOLDERS);
10321039

1033-
add_vars_to_targetlist(root,vars,relids);
1040+
add_vars_to_targetlist(root,vars,relids, false);
10341041
list_free(vars);
10351042
}
10361043

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

Lines changed: 59 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@
2424

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

3030

3131
/*
@@ -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,PVC_INCLUDE_PLACEHOLDERS);
214+
vars=pull_var_clause(expr,PVC_INCLUDE_PLACEHOLDERS);
205215
foreach(vl,vars)
206216
{
207217
PlaceHolderVar*phv= (PlaceHolderVar*)lfirst(vl);
@@ -212,25 +222,47 @@ find_placeholders_in_qual(PlannerInfo *root, Node *qual, Relids relids)
212222
continue;
213223

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

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

233+
/*
234+
* mark_placeholder_maybe_needed
235+
*Mark a placeholder as possibly needed at the specified join level.
236+
*
237+
* relids is the syntactic join level to mark as the "maybe needed" level
238+
* for the placeholder.
239+
*
240+
* This is called during an initial scan of the query's targetlist and quals
241+
* before we begin deconstruct_jointree. Once we begin deconstruct_jointree,
242+
* all active placeholders must be present in root->placeholder_list with
243+
* their correct ph_may_need values, because make_outerjoininfo and
244+
* update_placeholder_eval_levels require this info to be available while
245+
* we crawl up the join tree.
246+
*/
247+
void
248+
mark_placeholder_maybe_needed(PlannerInfo*root,PlaceHolderInfo*phinfo,
249+
Relidsrelids)
250+
{
251+
/* Mark the PHV as possibly needed at the given syntactic level */
252+
phinfo->ph_may_need=bms_add_members(phinfo->ph_may_need,relids);
253+
254+
/*
255+
* This is a bit tricky: the PHV's contained expression may contain other,
256+
* lower-level PHVs. We need to get those into the PlaceHolderInfo list,
257+
* but they aren't going to be needed where the outer PHV is referenced.
258+
* Rather, they'll be needed where the outer PHV is evaluated. We can
259+
* estimate that (conservatively) as the syntactic location of the PHV's
260+
* expression. Recurse to take care of any such PHVs.
261+
*/
262+
mark_placeholders_in_expr(root, (Node*)phinfo->ph_var->phexpr,
263+
phinfo->ph_var->phrels);
264+
}
265+
234266
/*
235267
* update_placeholder_eval_levels
236268
*Adjust the target evaluation levels for placeholders
@@ -350,7 +382,7 @@ fix_placeholder_input_needed_levels(PlannerInfo *root)
350382
List*vars=pull_var_clause((Node*)phinfo->ph_var->phexpr,
351383
PVC_INCLUDE_PLACEHOLDERS);
352384

353-
add_vars_to_targetlist(root,vars,eval_at);
385+
add_vars_to_targetlist(root,vars,eval_at, false);
354386
list_free(vars);
355387
}
356388
}

‎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