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

Commit 811a2cb

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 eca0c38 commit 811a2cb

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
@@ -193,25 +193,22 @@ assign_nestloop_param_var(PlannerInfo *root, Var *var)
193193
}
194194

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

210-
Assert(phv->phlevelsup == 0);
211-
abslevel = root->query_level;
209+
abslevel = root->query_level - phv->phlevelsup;
212210

213211
/* If there's already a paramlist entry for this same PHV, just use it */
214-
/* We assume comparing the PHIDs is sufficient */
215212
i = 0;
216213
foreach(ppl, root->glob->paramlist)
217214
{
@@ -220,25 +217,77 @@ assign_nestloop_param_placeholdervar(PlannerInfo *root, PlaceHolderVar *phv)
220217
{
221218
PlaceHolderVar *pphv = (PlaceHolderVar *) pitem->item;
222219

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

229-
if (ppl == NULL)
227+
/* Nope, so make a new one */
228+
phv = (PlaceHolderVar *) copyObject(phv);
229+
if (phv->phlevelsup != 0)
230230
{
231-
/* Nope, so make a new one */
232-
phv = (PlaceHolderVar *) copyObject(phv);
231+
IncrementVarSublevelsUp((Node *) phv, -((int) phv->phlevelsup), 0);
232+
Assert(phv->phlevelsup == 0);
233+
}
233234

234-
pitem = makeNode(PlannerParamItem);
235-
pitem->item = (Node *) phv;
236-
pitem->abslevel = abslevel;
235+
pitem = makeNode(PlannerParamItem);
236+
pitem->item = (Node *) phv;
237+
pitem->abslevel = abslevel;
237238

238-
root->glob->paramlist = lappend(root->glob->paramlist, pitem);
239+
root->glob->paramlist = lappend(root->glob->paramlist, pitem);
239240

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

243292
retval = makeNode(Param);
244293
retval->paramkind = PARAM_EXEC;
@@ -560,17 +609,19 @@ build_subplan(PlannerInfo *root, Plan *plan, List *rtable, List *rowmarks,
560609
Node *arg;
561610

562611
/*
563-
* The Var or Aggref has already been adjusted to have the correct
564-
* varlevelsup or agglevelsup. We probably don't even need to
565-
* copy it again, but be safe.
612+
* The Var, PlaceHolderVar, or Aggref has already been adjusted to
613+
* have the correct varlevelsup, phlevelsup, or agglevelsup. We
614+
* probably don't even need to copy it again, but be safe.
566615
*/
567616
arg = copyObject(pitem->item);
568617

569618
/*
570-
* If it's an Aggref, its arguments might contain SubLinks, which
571-
* have not yet been processed. Do that now.
619+
* If it's a PlaceHolderVar or Aggref, its arguments might contain
620+
* SubLinks, which have not yet been processed (see the comments
621+
* for SS_replace_correlation_vars). Do that now.
572622
*/
573-
if (IsA(arg, Aggref))
623+
if (IsA(arg, PlaceHolderVar) ||
624+
IsA(arg, Aggref))
574625
arg = SS_process_sublinks(root, arg, false);
575626

576627
splan->parParam = lappend_int(splan->parParam, paramid);
@@ -1678,24 +1729,25 @@ convert_EXISTS_to_ANY(PlannerInfo *root, Query *subselect,
16781729
/*
16791730
* Replace correlation vars (uplevel vars) with Params.
16801731
*
1681-
* Uplevel aggregates are replaced, too.
1732+
* Uplevel PlaceHolderVars and aggregates are replaced, too.
16821733
*
16831734
* Note: it is critical that this runs immediately after SS_process_sublinks.
1684-
* Since we do not recurse into the arguments of uplevel aggregates, they will
1685-
* get copied to the appropriate subplan args list in the parent query with
1686-
* uplevel vars not replaced by Params, but only adjusted in level (see
1687-
* replace_outer_agg). That's exactly what we want for the vars of the parent
1688-
* level --- but if an aggregate's argument contains any further-up variables,
1689-
* they have to be replaced with Params in their turn. That will happen when
1690-
* the parent level runs SS_replace_correlation_vars. Therefore it must do
1691-
* so after expanding its sublinks to subplans. And we don't want any steps
1692-
* in between, else those steps would never get applied to the aggregate
1693-
* argument expressions, either in the parent or the child level.
1735+
* Since we do not recurse into the arguments of uplevel PHVs and aggregates,
1736+
* they will get copied to the appropriate subplan args list in the parent
1737+
* query with uplevel vars not replaced by Params, but only adjusted in level
1738+
* (see replace_outer_placeholdervar and replace_outer_agg). That's exactly
1739+
* what we want for the vars of the parent level --- but if a PHV's or
1740+
* aggregate's argument contains any further-up variables, they have to be
1741+
* replaced with Params in their turn. That will happen when the parent level
1742+
* runs SS_replace_correlation_vars. Therefore it must do so after expanding
1743+
* its sublinks to subplans. And we don't want any steps in between, else
1744+
* those steps would never get applied to the argument expressions, either in
1745+
* the parent or the child level.
16941746
*
16951747
* Another fairly tricky thing going on here is the handling of SubLinks in
1696-
* the arguments of uplevel aggregates. Those are not touched inside the
1697-
* intermediate query level, either. Instead, SS_process_sublinks recurses
1698-
* on them after copying the Aggref expression into the parent plan level
1748+
* the arguments of uplevel PHVs/aggregates. Those are not touched inside the
1749+
* intermediate query level, either. Instead, SS_process_sublinks recurses on
1750+
* them after copying the PHV or Aggref expression into the parent plan level
16991751
* (this is actually taken care of in build_subplan).
17001752
*/
17011753
Node *
@@ -1715,6 +1767,12 @@ replace_correlation_vars_mutator(Node *node, PlannerInfo *root)
17151767
if (((Var *) node)->varlevelsup > 0)
17161768
return (Node *) replace_outer_var(root, (Var *) node);
17171769
}
1770+
if (IsA(node, PlaceHolderVar))
1771+
{
1772+
if (((PlaceHolderVar *) node)->phlevelsup > 0)
1773+
return (Node *) replace_outer_placeholdervar(root,
1774+
(PlaceHolderVar *) node);
1775+
}
17181776
if (IsA(node, Aggref))
17191777
{
17201778
if (((Aggref *) node)->agglevelsup > 0)
@@ -1774,12 +1832,17 @@ process_sublinks_mutator(Node *node, process_sublinks_context *context)
17741832
}
17751833

17761834
/*
1777-
* Don't recurse into the arguments of an outer aggregate here. Any
1778-
* SubLinks in the arguments have to be dealt with at the outer query
1779-
* level; they'll be handled when build_subplan collects the Aggref into
1780-
* the arguments to be passed down to the current subplan.
1835+
* Don't recurse into the arguments of an outer PHV or aggregate here.
1836+
* Any SubLinks in the arguments have to be dealt with at the outer query
1837+
* level; they'll be handled when build_subplan collects the PHV or Aggref
1838+
* into the arguments to be passed down to the current subplan.
17811839
*/
1782-
if (IsA(node, Aggref))
1840+
if (IsA(node, PlaceHolderVar))
1841+
{
1842+
if (((PlaceHolderVar *) node)->phlevelsup > 0)
1843+
return node;
1844+
}
1845+
else if (IsA(node, Aggref))
17831846
{
17841847
if (((Aggref *) node)->agglevelsup > 0)
17851848
return node;

src/backend/optimizer/prep/prepjointree.c

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1984,8 +1984,6 @@ reduce_outer_joins_pass2(Node *jtnode,
19841984
*
19851985
* Find any PlaceHolderVar nodes in the given tree that reference the
19861986
* pulled-up relid, and change them to reference the replacement relid(s).
1987-
* We do not need to recurse into subqueries, since no subquery of the current
1988-
* top query could (yet) contain such a reference.
19891987
*
19901988
* NOTE: although this has the form of a walker, we cheat and modify the
19911989
* nodes in-place. This should be OK since the tree was copied by
@@ -1996,6 +1994,7 @@ reduce_outer_joins_pass2(Node *jtnode,
19961994
typedef struct
19971995
{
19981996
int varno;
1997+
int sublevels_up;
19991998
Relids subrelids;
20001999
} substitute_multiple_relids_context;
20012000

@@ -2009,7 +2008,8 @@ substitute_multiple_relids_walker(Node *node,
20092008
{
20102009
PlaceHolderVar *phv = (PlaceHolderVar *) node;
20112010

2012-
if (bms_is_member(context->varno, phv->phrels))
2011+
if (phv->phlevelsup == context->sublevels_up &&
2012+
bms_is_member(context->varno, phv->phrels))
20132013
{
20142014
phv->phrels = bms_union(phv->phrels,
20152015
context->subrelids);
@@ -2018,6 +2018,18 @@ substitute_multiple_relids_walker(Node *node,
20182018
}
20192019
/* fall through to examine children */
20202020
}
2021+
if (IsA(node, Query))
2022+
{
2023+
/* Recurse into subselects */
2024+
bool result;
2025+
2026+
context->sublevels_up++;
2027+
result = query_tree_walker((Query *) node,
2028+
substitute_multiple_relids_walker,
2029+
(void *) context, 0);
2030+
context->sublevels_up--;
2031+
return result;
2032+
}
20212033
/* Shouldn't need to handle planner auxiliary nodes here */
20222034
Assert(!IsA(node, SpecialJoinInfo));
20232035
Assert(!IsA(node, AppendRelInfo));
@@ -2034,6 +2046,7 @@ substitute_multiple_relids(Node *node, int varno, Relids subrelids)
20342046
substitute_multiple_relids_context context;
20352047

20362048
context.varno = varno;
2049+
context.sublevels_up = 0;
20372050
context.subrelids = subrelids;
20382051

20392052
/*

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)