Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 8279eb4

Browse files
committed
Fix planner's handling of outer PlaceHolderVars within subqueries.
For some reason, in the original coding of the PlaceHolderVar mechanism I had supposed that PlaceHolderVars couldn't propagate into subqueries. That is of course entirely possible. When it happens, we need to treat an outer-level PlaceHolderVar much like an outer Var or Aggref, that is SS_replace_correlation_vars() needs to replace the PlaceHolderVar with a Param, and then when building the finished SubPlan we have to provide the PlaceHolderVar expression as an actual parameter for the SubPlan. The handling of the contained expression is a bit delicate but it can be treated exactly like an Aggref's expression. In addition to the missing logic in subselect.c, prepjointree.c was failing to search subqueries for PlaceHolderVars that need their relids adjusted during subquery pullup. It looks like everyplace else that touches PlaceHolderVars got it right, though. Per report from Mark Murawski. In 9.1 and HEAD, queries affected by this oversight would fail with "ERROR: Upper-level PlaceHolderVar found where not expected". But in 9.0 and 8.4, you'd silently get possibly-wrong answers, since the value transmitted into the subquery wouldn't go to null when it should.
1 parent ed61127 commit 8279eb4

File tree

5 files changed

+195
-49
lines changed

5 files changed

+195
-49
lines changed

src/backend/optimizer/plan/subselect.c

Lines changed: 107 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -191,25 +191,22 @@ assign_nestloop_param_var(PlannerInfo *root, Var *var)
191191
}
192192

193193
/*
194-
* Generate a Param node to replace the given PlaceHolderVar, which will be
195-
* supplied from an upper NestLoop join node.
194+
* Select a PARAM_EXEC number to identify the given PlaceHolderVar.
195+
* If the PlaceHolderVar already has a param slot, return that one.
196196
*
197-
* This is just like assign_nestloop_param_var, except for PlaceHolderVars.
197+
* This is just like assign_param_for_var, except for PlaceHolderVars.
198198
*/
199-
Param *
200-
assign_nestloop_param_placeholdervar(PlannerInfo *root, PlaceHolderVar *phv)
199+
static int
200+
assign_param_for_placeholdervar(PlannerInfo *root, PlaceHolderVar *phv)
201201
{
202-
Param *retval;
203202
ListCell *ppl;
204203
PlannerParamItem *pitem;
205204
Index abslevel;
206205
int i;
207206

208-
Assert(phv->phlevelsup == 0);
209-
abslevel = root->query_level;
207+
abslevel = root->query_level - phv->phlevelsup;
210208

211209
/* If there's already a paramlist entry for this same PHV, just use it */
212-
/* We assume comparing the PHIDs is sufficient */
213210
i = 0;
214211
foreach(ppl, root->glob->paramlist)
215212
{
@@ -218,25 +215,77 @@ assign_nestloop_param_placeholdervar(PlannerInfo *root, PlaceHolderVar *phv)
218215
{
219216
PlaceHolderVar *pphv = (PlaceHolderVar *) pitem->item;
220217

218+
/* We assume comparing the PHIDs is sufficient */
221219
if (pphv->phid == phv->phid)
222-
break;
220+
return i;
223221
}
224222
i++;
225223
}
226224

227-
if (ppl == NULL)
225+
/* Nope, so make a new one */
226+
phv = (PlaceHolderVar *) copyObject(phv);
227+
if (phv->phlevelsup != 0)
228228
{
229-
/* Nope, so make a new one */
230-
phv = (PlaceHolderVar *) copyObject(phv);
229+
IncrementVarSublevelsUp((Node *) phv, -((int) phv->phlevelsup), 0);
230+
Assert(phv->phlevelsup == 0);
231+
}
231232

232-
pitem = makeNode(PlannerParamItem);
233-
pitem->item = (Node *) phv;
234-
pitem->abslevel = abslevel;
233+
pitem = makeNode(PlannerParamItem);
234+
pitem->item = (Node *) phv;
235+
pitem->abslevel = abslevel;
235236

236-
root->glob->paramlist = lappend(root->glob->paramlist, pitem);
237+
root->glob->paramlist = lappend(root->glob->paramlist, pitem);
237238

238-
/* i is already the correct list index for the new item */
239-
}
239+
/* i is already the correct list index for the new item */
240+
return i;
241+
}
242+
243+
/*
244+
* Generate a Param node to replace the given PlaceHolderVar,
245+
* which is expected to have phlevelsup > 0 (ie, it is not local).
246+
*
247+
* This is just like replace_outer_var, except for PlaceHolderVars.
248+
*/
249+
static Param *
250+
replace_outer_placeholdervar(PlannerInfo *root, PlaceHolderVar *phv)
251+
{
252+
Param *retval;
253+
int i;
254+
255+
Assert(phv->phlevelsup > 0 && phv->phlevelsup < root->query_level);
256+
257+
/*
258+
* Find the PlaceHolderVar in root->glob->paramlist, or add it if not
259+
* present.
260+
*/
261+
i = assign_param_for_placeholdervar(root, phv);
262+
263+
retval = makeNode(Param);
264+
retval->paramkind = PARAM_EXEC;
265+
retval->paramid = i;
266+
retval->paramtype = exprType((Node *) phv->phexpr);
267+
retval->paramtypmod = exprTypmod((Node *) phv->phexpr);
268+
retval->paramcollid = exprCollation((Node *) phv->phexpr);
269+
retval->location = -1;
270+
271+
return retval;
272+
}
273+
274+
/*
275+
* Generate a Param node to replace the given PlaceHolderVar, which will be
276+
* supplied from an upper NestLoop join node.
277+
*
278+
* This is just like assign_nestloop_param_var, except for PlaceHolderVars.
279+
*/
280+
Param *
281+
assign_nestloop_param_placeholdervar(PlannerInfo *root, PlaceHolderVar *phv)
282+
{
283+
Param *retval;
284+
int i;
285+
286+
Assert(phv->phlevelsup == 0);
287+
288+
i = assign_param_for_placeholdervar(root, phv);
240289

241290
retval = makeNode(Param);
242291
retval->paramkind = PARAM_EXEC;
@@ -555,17 +604,19 @@ build_subplan(PlannerInfo *root, Plan *plan, PlannerInfo *subroot,
555604
Node *arg;
556605

557606
/*
558-
* The Var or Aggref has already been adjusted to have the correct
559-
* varlevelsup or agglevelsup. We probably don't even need to
560-
* copy it again, but be safe.
607+
* The Var, PlaceHolderVar, or Aggref has already been adjusted to
608+
* have the correct varlevelsup, phlevelsup, or agglevelsup. We
609+
* probably don't even need to copy it again, but be safe.
561610
*/
562611
arg = copyObject(pitem->item);
563612

564613
/*
565-
* If it's an Aggref, its arguments might contain SubLinks, which
566-
* have not yet been processed. Do that now.
614+
* If it's a PlaceHolderVar or Aggref, its arguments might contain
615+
* SubLinks, which have not yet been processed (see the comments
616+
* for SS_replace_correlation_vars). Do that now.
567617
*/
568-
if (IsA(arg, Aggref))
618+
if (IsA(arg, PlaceHolderVar) ||
619+
IsA(arg, Aggref))
569620
arg = SS_process_sublinks(root, arg, false);
570621

571622
splan->parParam = lappend_int(splan->parParam, paramid);
@@ -1668,24 +1719,25 @@ convert_EXISTS_to_ANY(PlannerInfo *root, Query *subselect,
16681719
/*
16691720
* Replace correlation vars (uplevel vars) with Params.
16701721
*
1671-
* Uplevel aggregates are replaced, too.
1722+
* Uplevel PlaceHolderVars and aggregates are replaced, too.
16721723
*
16731724
* Note: it is critical that this runs immediately after SS_process_sublinks.
1674-
* Since we do not recurse into the arguments of uplevel aggregates, they will
1675-
* get copied to the appropriate subplan args list in the parent query with
1676-
* uplevel vars not replaced by Params, but only adjusted in level (see
1677-
* replace_outer_agg). That's exactly what we want for the vars of the parent
1678-
* level --- but if an aggregate's argument contains any further-up variables,
1679-
* they have to be replaced with Params in their turn. That will happen when
1680-
* the parent level runs SS_replace_correlation_vars. Therefore it must do
1681-
* so after expanding its sublinks to subplans. And we don't want any steps
1682-
* in between, else those steps would never get applied to the aggregate
1683-
* argument expressions, either in the parent or the child level.
1725+
* Since we do not recurse into the arguments of uplevel PHVs and aggregates,
1726+
* they will get copied to the appropriate subplan args list in the parent
1727+
* query with uplevel vars not replaced by Params, but only adjusted in level
1728+
* (see replace_outer_placeholdervar and replace_outer_agg). That's exactly
1729+
* what we want for the vars of the parent level --- but if a PHV's or
1730+
* aggregate's argument contains any further-up variables, they have to be
1731+
* replaced with Params in their turn. That will happen when the parent level
1732+
* runs SS_replace_correlation_vars. Therefore it must do so after expanding
1733+
* its sublinks to subplans. And we don't want any steps in between, else
1734+
* those steps would never get applied to the argument expressions, either in
1735+
* the parent or the child level.
16841736
*
16851737
* Another fairly tricky thing going on here is the handling of SubLinks in
1686-
* the arguments of uplevel aggregates. Those are not touched inside the
1687-
* intermediate query level, either. Instead, SS_process_sublinks recurses
1688-
* on them after copying the Aggref expression into the parent plan level
1738+
* the arguments of uplevel PHVs/aggregates. Those are not touched inside the
1739+
* intermediate query level, either. Instead, SS_process_sublinks recurses on
1740+
* them after copying the PHV or Aggref expression into the parent plan level
16891741
* (this is actually taken care of in build_subplan).
16901742
*/
16911743
Node *
@@ -1705,6 +1757,12 @@ replace_correlation_vars_mutator(Node *node, PlannerInfo *root)
17051757
if (((Var *) node)->varlevelsup > 0)
17061758
return (Node *) replace_outer_var(root, (Var *) node);
17071759
}
1760+
if (IsA(node, PlaceHolderVar))
1761+
{
1762+
if (((PlaceHolderVar *) node)->phlevelsup > 0)
1763+
return (Node *) replace_outer_placeholdervar(root,
1764+
(PlaceHolderVar *) node);
1765+
}
17081766
if (IsA(node, Aggref))
17091767
{
17101768
if (((Aggref *) node)->agglevelsup > 0)
@@ -1764,12 +1822,17 @@ process_sublinks_mutator(Node *node, process_sublinks_context *context)
17641822
}
17651823

17661824
/*
1767-
* Don't recurse into the arguments of an outer aggregate here. Any
1768-
* SubLinks in the arguments have to be dealt with at the outer query
1769-
* level; they'll be handled when build_subplan collects the Aggref into
1770-
* the arguments to be passed down to the current subplan.
1825+
* Don't recurse into the arguments of an outer PHV or aggregate here.
1826+
* Any SubLinks in the arguments have to be dealt with at the outer query
1827+
* level; they'll be handled when build_subplan collects the PHV or Aggref
1828+
* into the arguments to be passed down to the current subplan.
17711829
*/
1772-
if (IsA(node, Aggref))
1830+
if (IsA(node, PlaceHolderVar))
1831+
{
1832+
if (((PlaceHolderVar *) node)->phlevelsup > 0)
1833+
return node;
1834+
}
1835+
else if (IsA(node, Aggref))
17731836
{
17741837
if (((Aggref *) node)->agglevelsup > 0)
17751838
return node;

src/backend/optimizer/prep/prepjointree.c

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2069,8 +2069,6 @@ reduce_outer_joins_pass2(Node *jtnode,
20692069
*
20702070
* Find any PlaceHolderVar nodes in the given tree that reference the
20712071
* pulled-up relid, and change them to reference the replacement relid(s).
2072-
* We do not need to recurse into subqueries, since no subquery of the current
2073-
* top query could (yet) contain such a reference.
20742072
*
20752073
* NOTE: although this has the form of a walker, we cheat and modify the
20762074
* nodes in-place. This should be OK since the tree was copied by
@@ -2081,6 +2079,7 @@ reduce_outer_joins_pass2(Node *jtnode,
20812079
typedef struct
20822080
{
20832081
int varno;
2082+
int sublevels_up;
20842083
Relids subrelids;
20852084
} substitute_multiple_relids_context;
20862085

@@ -2094,7 +2093,8 @@ substitute_multiple_relids_walker(Node *node,
20942093
{
20952094
PlaceHolderVar *phv = (PlaceHolderVar *) node;
20962095

2097-
if (bms_is_member(context->varno, phv->phrels))
2096+
if (phv->phlevelsup == context->sublevels_up &&
2097+
bms_is_member(context->varno, phv->phrels))
20982098
{
20992099
phv->phrels = bms_union(phv->phrels,
21002100
context->subrelids);
@@ -2103,6 +2103,18 @@ substitute_multiple_relids_walker(Node *node,
21032103
}
21042104
/* fall through to examine children */
21052105
}
2106+
if (IsA(node, Query))
2107+
{
2108+
/* Recurse into subselects */
2109+
bool result;
2110+
2111+
context->sublevels_up++;
2112+
result = query_tree_walker((Query *) node,
2113+
substitute_multiple_relids_walker,
2114+
(void *) context, 0);
2115+
context->sublevels_up--;
2116+
return result;
2117+
}
21062118
/* Shouldn't need to handle planner auxiliary nodes here */
21072119
Assert(!IsA(node, SpecialJoinInfo));
21082120
Assert(!IsA(node, AppendRelInfo));
@@ -2119,6 +2131,7 @@ substitute_multiple_relids(Node *node, int varno, Relids subrelids)
21192131
substitute_multiple_relids_context context;
21202132

21212133
context.varno = varno;
2134+
context.sublevels_up = 0;
21222135
context.subrelids = subrelids;
21232136

21242137
/*

src/include/nodes/relation.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1445,8 +1445,10 @@ typedef struct MinMaxAggInfo
14451445
* from a NestLoop node of that level to its inner scan. The varlevelsup
14461446
* value in the Var will always be zero.
14471447
*
1448-
* A PlaceHolderVar: this works much like the Var case. It is currently
1449-
* only needed for NestLoop parameters, not outer references.
1448+
* A PlaceHolderVar: this works much like the Var case, except that the
1449+
* entry is a PlaceHolderVar node with a contained expression. The PHV
1450+
* will have phlevelsup = 0, and the contained expression is adjusted
1451+
* to match in level.
14501452
*
14511453
* An Aggref (with an expression tree representing its argument): the slot
14521454
* represents an aggregate expression that is an outer reference for some

src/test/regress/expected/join.out

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2571,6 +2571,53 @@ SELECT qq, unique1
25712571
456 | 7318
25722572
(3 rows)
25732573

2574+
--
2575+
-- test case where a PlaceHolderVar is propagated into a subquery
2576+
--
2577+
explain (costs off)
2578+
select * from
2579+
int8_tbl t1 left join
2580+
(select q1 as x, 42 as y from int8_tbl t2) ss
2581+
on t1.q2 = ss.x
2582+
where
2583+
1 = (select 1 from int8_tbl t3 where ss.y is not null limit 1)
2584+
order by 1,2;
2585+
QUERY PLAN
2586+
-----------------------------------------------------------
2587+
Sort
2588+
Sort Key: t1.q1, t1.q2
2589+
-> Hash Left Join
2590+
Hash Cond: (t1.q2 = t2.q1)
2591+
Filter: (1 = (SubPlan 1))
2592+
-> Seq Scan on int8_tbl t1
2593+
-> Hash
2594+
-> Seq Scan on int8_tbl t2
2595+
SubPlan 1
2596+
-> Limit
2597+
-> Result
2598+
One-Time Filter: ((42) IS NOT NULL)
2599+
-> Seq Scan on int8_tbl t3
2600+
(13 rows)
2601+
2602+
select * from
2603+
int8_tbl t1 left join
2604+
(select q1 as x, 42 as y from int8_tbl t2) ss
2605+
on t1.q2 = ss.x
2606+
where
2607+
1 = (select 1 from int8_tbl t3 where ss.y is not null limit 1)
2608+
order by 1,2;
2609+
q1 | q2 | x | y
2610+
------------------+------------------+------------------+----
2611+
123 | 4567890123456789 | 4567890123456789 | 42
2612+
123 | 4567890123456789 | 4567890123456789 | 42
2613+
123 | 4567890123456789 | 4567890123456789 | 42
2614+
4567890123456789 | 123 | 123 | 42
2615+
4567890123456789 | 123 | 123 | 42
2616+
4567890123456789 | 4567890123456789 | 4567890123456789 | 42
2617+
4567890123456789 | 4567890123456789 | 4567890123456789 | 42
2618+
4567890123456789 | 4567890123456789 | 4567890123456789 | 42
2619+
(8 rows)
2620+
25742621
--
25752622
-- test the corner cases FULL JOIN ON TRUE and FULL JOIN ON FALSE
25762623
--

src/test/regress/sql/join.sql

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -662,6 +662,27 @@ SELECT qq, unique1
662662
USING (qq)
663663
INNER JOIN tenk1 c ON qq = unique2;
664664

665+
--
666+
-- test case where a PlaceHolderVar is propagated into a subquery
667+
--
668+
669+
explain (costs off)
670+
select * from
671+
int8_tbl t1 left join
672+
(select q1 as x, 42 as y from int8_tbl t2) ss
673+
on t1.q2 = ss.x
674+
where
675+
1 = (select 1 from int8_tbl t3 where ss.y is not null limit 1)
676+
order by 1,2;
677+
678+
select * from
679+
int8_tbl t1 left join
680+
(select q1 as x, 42 as y from int8_tbl t2) ss
681+
on t1.q2 = ss.x
682+
where
683+
1 = (select 1 from int8_tbl t3 where ss.y is not null limit 1)
684+
order by 1,2;
685+
665686
--
666687
-- test the corner cases FULL JOIN ON TRUE and FULL JOIN ON FALSE
667688
--

0 commit comments

Comments
 (0)