Skip to content

Commit 7e3bf99

Browse files
tglsfdctglsfdc
committed
Fix handling of PlaceHolderVars in nestloop parameter management.
If we use a PlaceHolderVar from the outer relation in an inner indexscan, we need to reference the PlaceHolderVar as such as the value to be passed in from the outer relation. The previous code effectively tried to reconstruct the PHV from its component expression, which doesn't work since (a) the Vars therein aren't necessarily bubbled up far enough, and (b) it would be the wrong semantics anyway because of the possibility that the PHV is supposed to have gone to null at some point before the current join. Point (a) led to "variable not found in subplan target list" planner errors, but point (b) would have led to silently wrong answers. Per report from Roger Niederland.
1 parent 1a77f8b commit 7e3bf99

File tree

9 files changed

+208
-19
lines changed

9 files changed

+208
-19
lines changed

‎src/backend/executor/nodeNestloop.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ ExecNestLoop(NestLoopState *node)
148148

149149
prm = &(econtext->ecxt_param_exec_vals[paramno]);
150150
/* Param value should be an OUTER_VAR var */
151+
Assert(IsA(nlp->paramval, Var));
151152
Assert(nlp->paramval->varno == OUTER_VAR);
152153
Assert(nlp->paramval->varattno > 0);
153154
prm->value = slot_getattr(outerTupleSlot,

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

Lines changed: 64 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include "optimizer/clauses.h"
2828
#include "optimizer/cost.h"
2929
#include "optimizer/paths.h"
30+
#include "optimizer/placeholder.h"
3031
#include "optimizer/plancat.h"
3132
#include "optimizer/planmain.h"
3233
#include "optimizer/predtest.h"
@@ -1926,7 +1927,20 @@ create_nestloop_plan(PlannerInfo *root,
19261927
NestLoopParam *nlp = (NestLoopParam *) lfirst(cell);
19271928

19281929
next = lnext(cell);
1929-
if (bms_is_member(nlp->paramval->varno, outerrelids))
1930+
if (IsA(nlp->paramval, Var) &&
1931+
bms_is_member(nlp->paramval->varno, outerrelids))
1932+
{
1933+
root->curOuterParams = list_delete_cell(root->curOuterParams,
1934+
cell, prev);
1935+
nestParams = lappend(nestParams, nlp);
1936+
}
1937+
else if (IsA(nlp->paramval, PlaceHolderVar) &&
1938+
bms_overlap(((PlaceHolderVar *) nlp->paramval)->phrels,
1939+
outerrelids) &&
1940+
bms_is_subset(find_placeholder_info(root,
1941+
(PlaceHolderVar *) nlp->paramval,
1942+
false)->ph_eval_at,
1943+
outerrelids))
19301944
{
19311945
root->curOuterParams = list_delete_cell(root->curOuterParams,
19321946
cell, prev);
@@ -2354,11 +2368,12 @@ create_hashjoin_plan(PlannerInfo *root,
23542368

23552369
/*
23562370
* replace_nestloop_params
2357-
* Replace outer-relation Vars in the given expression with nestloop Params
2371+
* Replace outer-relation Vars and PlaceHolderVars in the given expression
2372+
* with nestloop Params
23582373
*
2359-
* All Vars belonging to the relation(s) identified by root->curOuterRels
2360-
* are replaced by Params, and entries are added to root->curOuterParams if
2361-
* not already present.
2374+
* All Vars and PlaceHolderVars belonging to the relation(s) identified by
2375+
* root->curOuterRels are replaced by Params, and entries are added to
2376+
* root->curOuterParams if not already present.
23622377
*/
23632378
static Node *
23642379
replace_nestloop_params(PlannerInfo *root, Node *expr)
@@ -2385,7 +2400,7 @@ replace_nestloop_params_mutator(Node *node, PlannerInfo *root)
23852400
if (!bms_is_member(var->varno, root->curOuterRels))
23862401
return node;
23872402
/* Create a Param representing the Var */
2388-
param = assign_nestloop_param(root, var);
2403+
param = assign_nestloop_param_var(root, var);
23892404
/* Is this param already listed in root->curOuterParams? */
23902405
foreach(lc, root->curOuterParams)
23912406
{
@@ -2405,6 +2420,48 @@ replace_nestloop_params_mutator(Node *node, PlannerInfo *root)
24052420
/* And return the replacement Param */
24062421
return (Node *) param;
24072422
}
2423+
if (IsA(node, PlaceHolderVar))
2424+
{
2425+
PlaceHolderVar *phv = (PlaceHolderVar *) node;
2426+
Param *param;
2427+
NestLoopParam *nlp;
2428+
ListCell *lc;
2429+
2430+
/* Upper-level PlaceHolderVars should be long gone at this point */
2431+
Assert(phv->phlevelsup == 0);
2432+
2433+
/*
2434+
* If not to be replaced, just return the PlaceHolderVar unmodified.
2435+
* We use bms_overlap as a cheap/quick test to see if the PHV might
2436+
* be evaluated in the outer rels, and then grab its PlaceHolderInfo
2437+
* to tell for sure.
2438+
*/
2439+
if (!bms_overlap(phv->phrels, root->curOuterRels))
2440+
return node;
2441+
if (!bms_is_subset(find_placeholder_info(root, phv, false)->ph_eval_at,
2442+
root->curOuterRels))
2443+
return node;
2444+
/* Create a Param representing the PlaceHolderVar */
2445+
param = assign_nestloop_param_placeholdervar(root, phv);
2446+
/* Is this param already listed in root->curOuterParams? */
2447+
foreach(lc, root->curOuterParams)
2448+
{
2449+
nlp = (NestLoopParam *) lfirst(lc);
2450+
if (nlp->paramno == param->paramid)
2451+
{
2452+
Assert(equal(phv, nlp->paramval));
2453+
/* Present, so we can just return the Param */
2454+
return (Node *) param;
2455+
}
2456+
}
2457+
/* No, so add it */
2458+
nlp = makeNode(NestLoopParam);
2459+
nlp->paramno = param->paramid;
2460+
nlp->paramval = (Var *) phv;
2461+
root->curOuterParams = lappend(root->curOuterParams, nlp);
2462+
/* And return the replacement Param */
2463+
return (Node *) param;
2464+
}
24082465
return expression_tree_mutator(node,
24092466
replace_nestloop_params_mutator,
24102467
(void *) root);
@@ -2417,7 +2474,7 @@ replace_nestloop_params_mutator(Node *node, PlannerInfo *root)
24172474
*
24182475
* We have four tasks here:
24192476
* * Remove RestrictInfo nodes from the input clauses.
2420-
* * Replace any outer-relation Var nodes with nestloop Params.
2477+
* * Replace any outer-relation Var or PHV nodes with nestloop Params.
24212478
* (XXX eventually, that responsibility should go elsewhere?)
24222479
* * Index keys must be represented by Var nodes with varattno set to the
24232480
* index's attribute number, not the attribute number in the original rel.

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1082,6 +1082,10 @@ set_join_references(PlannerInfo *root, Join *join, int rtoffset)
10821082
outer_itlist,
10831083
OUTER_VAR,
10841084
rtoffset);
1085+
/* Check we replaced any PlaceHolderVar with simple Var */
1086+
if (!(IsA(nlp->paramval, Var) &&
1087+
nlp->paramval->varno == OUTER_VAR))
1088+
elog(ERROR, "NestLoopParam was not reduced to a simple Var");
10851089
}
10861090
}
10871091
else if (IsA(join, MergeJoin))

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

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ replace_outer_var(PlannerInfo *root, Var *var)
170170
* the Var to be local to the current query level.
171171
*/
172172
Param *
173-
assign_nestloop_param(PlannerInfo *root, Var *var)
173+
assign_nestloop_param_var(PlannerInfo *root, Var *var)
174174
{
175175
Param *retval;
176176
int i;
@@ -190,6 +190,65 @@ assign_nestloop_param(PlannerInfo *root, Var *var)
190190
return retval;
191191
}
192192

193+
/*
194+
* Generate a Param node to replace the given PlaceHolderVar, which will be
195+
* supplied from an upper NestLoop join node.
196+
*
197+
* This is just like assign_nestloop_param_var, except for PlaceHolderVars.
198+
*/
199+
Param *
200+
assign_nestloop_param_placeholdervar(PlannerInfo *root, PlaceHolderVar *phv)
201+
{
202+
Param *retval;
203+
ListCell *ppl;
204+
PlannerParamItem *pitem;
205+
Index abslevel;
206+
int i;
207+
208+
Assert(phv->phlevelsup == 0);
209+
abslevel = root->query_level;
210+
211+
/* If there's already a paramlist entry for this same PHV, just use it */
212+
/* We assume comparing the PHIDs is sufficient */
213+
i = 0;
214+
foreach(ppl, root->glob->paramlist)
215+
{
216+
pitem = (PlannerParamItem *) lfirst(ppl);
217+
if (pitem->abslevel == abslevel && IsA(pitem->item, PlaceHolderVar))
218+
{
219+
PlaceHolderVar *pphv = (PlaceHolderVar *) pitem->item;
220+
221+
if (pphv->phid == phv->phid)
222+
break;
223+
}
224+
i++;
225+
}
226+
227+
if (ppl == NULL)
228+
{
229+
/* Nope, so make a new one */
230+
phv = (PlaceHolderVar *) copyObject(phv);
231+
232+
pitem = makeNode(PlannerParamItem);
233+
pitem->item = (Node *) phv;
234+
pitem->abslevel = abslevel;
235+
236+
root->glob->paramlist = lappend(root->glob->paramlist, pitem);
237+
238+
/* i is already the correct list index for the new item */
239+
}
240+
241+
retval = makeNode(Param);
242+
retval->paramkind = PARAM_EXEC;
243+
retval->paramid = i;
244+
retval->paramtype = exprType((Node *) phv->phexpr);
245+
retval->paramtypmod = exprTypmod((Node *) phv->phexpr);
246+
retval->paramcollid = exprCollation((Node *) phv->phexpr);
247+
retval->location = -1;
248+
249+
return retval;
250+
}
251+
193252
/*
194253
* Generate a Param node to replace the given Aggref
195254
* which is expected to have agglevelsup > 0 (ie, it is not local).

‎src/include/nodes/plannodes.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -508,7 +508,9 @@ typedef struct Join
508508
* The nestParams list identifies any executor Params that must be passed
509509
* into execution of the inner subplan carrying values from the current row
510510
* of the outer subplan. Currently we restrict these values to be simple
511-
* Vars, but perhaps someday that'd be worth relaxing.
511+
* Vars, but perhaps someday that'd be worth relaxing. (Note: during plan
512+
* creation, the paramval can actually be a PlaceHolderVar expression; but it
513+
* must be a Var with varno OUTER_VAR by the time it gets to the executor.)
512514
* ----------------
513515
*/
514516
typedef struct NestLoop

‎src/include/nodes/relation.h

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1439,13 +1439,16 @@ typedef struct MinMaxAggInfo
14391439
*
14401440
* Each paramlist item shows the absolute query level it is associated with,
14411441
* where the outermost query is level 1 and nested subqueries have higher
1442-
* numbers. The item the parameter slot represents can be one of three kinds:
1442+
* numbers. The item the parameter slot represents can be one of four kinds:
14431443
*
14441444
* A Var: the slot represents a variable of that level that must be passed
14451445
* down because subqueries have outer references to it, or must be passed
14461446
* from a NestLoop node of that level to its inner scan. The varlevelsup
14471447
* value in the Var will always be zero.
14481448
*
1449+
* A PlaceHolderVar: this works much like the Var case. It is currently
1450+
* only needed for NestLoop parameters, not outer references.
1451+
*
14491452
* An Aggref (with an expression tree representing its argument): the slot
14501453
* represents an aggregate expression that is an outer reference for some
14511454
* subquery. The Aggref itself has agglevelsup = 0, and its argument tree
@@ -1455,20 +1458,20 @@ typedef struct MinMaxAggInfo
14551458
* for that subplan). The absolute level shown for such items corresponds
14561459
* to the parent query of the subplan.
14571460
*
1458-
* Note: we detect duplicate Var parameters and coalesce them into one slot,
1459-
* but we do not bother to do this for Aggrefs, and it would be incorrect
1460-
* to do so for Param slots. Duplicate detection is actually *necessary*
1461-
* in the case of NestLoop parameters since it serves to match up the usage
1462-
* of a Param (in the inner scan) with the assignment of the value (in the
1463-
* NestLoop node). This might result in the same PARAM_EXEC slot being used
1464-
* by multiple NestLoop nodes or SubPlan nodes, but no harm is done since
1461+
* Note: we detect duplicate Var and PlaceHolderVar parameters and coalesce
1462+
* them into one slot, but we do not bother to do this for Aggrefs, and it
1463+
* would be incorrect to do so for Param slots. Duplicate detection is
1464+
* actually *necessary* for NestLoop parameters since it serves to match up
1465+
* the usage of a Param (in the inner scan) with the assignment of the value
1466+
* (in the NestLoop node). This might result in the same PARAM_EXEC slot being
1467+
* used by multiple NestLoop nodes or SubPlan nodes, but no harm is done since
14651468
* the same value would be assigned anyway.
14661469
*/
14671470
typedef struct PlannerParamItem
14681471
{
14691472
NodeTag type;
14701473

1471-
Node *item; /* the Var, Aggref, or Param */
1474+
Node *item; /* the Var, PlaceHolderVar, Aggref, or Param */
14721475
Index abslevel; /* its absolute query level */
14731476
} PlannerParamItem;
14741477

‎src/include/optimizer/subselect.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,9 @@ extern void SS_finalize_plan(PlannerInfo *root, Plan *plan,
2929
bool attach_initplans);
3030
extern Param *SS_make_initplan_from_plan(PlannerInfo *root, Plan *plan,
3131
Oid resulttype, int32 resulttypmod, Oid resultcollation);
32-
extern Param *assign_nestloop_param(PlannerInfo *root, Var *var);
32+
extern Param *assign_nestloop_param_var(PlannerInfo *root, Var *var);
33+
extern Param *assign_nestloop_param_placeholdervar(PlannerInfo *root,
34+
PlaceHolderVar *phv);
3335
extern int SS_assign_special_param(PlannerInfo *root);
3436

3537
#endif /* SUBSELECT_H */

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2129,6 +2129,7 @@ on (x1 = xx1) where (xx2 is not null);
21292129
-- regression test: check for bug with propagation of implied equality
21302130
-- to outside an IN
21312131
--
2132+
analyze tenk1; -- ensure we get consistent plans here
21322133
select count(*) from tenk1 a where unique1 in
21332134
(select unique1 from tenk1 b join tenk1 c using (unique1)
21342135
where b.unique2 = 42);
@@ -2533,6 +2534,43 @@ ON sub1.key1 = sub2.key3;
25332534
1 | 1 | 1 | 1
25342535
(1 row)
25352536

2537+
--
2538+
-- test case where a PlaceHolderVar is used as a nestloop parameter
2539+
--
2540+
EXPLAIN (COSTS OFF)
2541+
SELECT qq, unique1
2542+
FROM
2543+
( SELECT COALESCE(q1, 0) AS qq FROM int8_tbl a ) AS ss1
2544+
FULL OUTER JOIN
2545+
( SELECT COALESCE(q2, -1) AS qq FROM int8_tbl b ) AS ss2
2546+
USING (qq)
2547+
INNER JOIN tenk1 c ON qq = unique2;
2548+
QUERY PLAN
2549+
-------------------------------------------------------------------------------------------------------
2550+
Nested Loop
2551+
-> Hash Full Join
2552+
Hash Cond: (COALESCE(a.q1, 0::bigint) = COALESCE(b.q2, (-1)::bigint))
2553+
-> Seq Scan on int8_tbl a
2554+
-> Hash
2555+
-> Seq Scan on int8_tbl b
2556+
-> Index Scan using tenk1_unique2 on tenk1 c
2557+
Index Cond: (unique2 = COALESCE((COALESCE(a.q1, 0::bigint)), (COALESCE(b.q2, (-1)::bigint))))
2558+
(8 rows)
2559+
2560+
SELECT qq, unique1
2561+
FROM
2562+
( SELECT COALESCE(q1, 0) AS qq FROM int8_tbl a ) AS ss1
2563+
FULL OUTER JOIN
2564+
( SELECT COALESCE(q2, -1) AS qq FROM int8_tbl b ) AS ss2
2565+
USING (qq)
2566+
INNER JOIN tenk1 c ON qq = unique2;
2567+
qq | unique1
2568+
-----+---------
2569+
123 | 4596
2570+
123 | 4596
2571+
456 | 7318
2572+
(3 rows)
2573+
25362574
--
25372575
-- test the corner cases FULL JOIN ON TRUE and FULL JOIN ON FALSE
25382576
--

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,8 @@ on (x1 = xx1) where (xx2 is not null);
330330
-- regression test: check for bug with propagation of implied equality
331331
-- to outside an IN
332332
--
333+
analyze tenk1; -- ensure we get consistent plans here
334+
333335
select count(*) from tenk1 a where unique1 in
334336
(select unique1 from tenk1 b join tenk1 c using (unique1)
335337
where b.unique2 = 42);
@@ -639,6 +641,27 @@ LEFT JOIN
639641
) sub2
640642
ON sub1.key1 = sub2.key3;
641643

644+
--
645+
-- test case where a PlaceHolderVar is used as a nestloop parameter
646+
--
647+
648+
EXPLAIN (COSTS OFF)
649+
SELECT qq, unique1
650+
FROM
651+
( SELECT COALESCE(q1, 0) AS qq FROM int8_tbl a ) AS ss1
652+
FULL OUTER JOIN
653+
( SELECT COALESCE(q2, -1) AS qq FROM int8_tbl b ) AS ss2
654+
USING (qq)
655+
INNER JOIN tenk1 c ON qq = unique2;
656+
657+
SELECT qq, unique1
658+
FROM
659+
( SELECT COALESCE(q1, 0) AS qq FROM int8_tbl a ) AS ss1
660+
FULL OUTER JOIN
661+
( SELECT COALESCE(q2, -1) AS qq FROM int8_tbl b ) AS ss2
662+
USING (qq)
663+
INNER JOIN tenk1 c ON qq = unique2;
664+
642665
--
643666
-- test the corner cases FULL JOIN ON TRUE and FULL JOIN ON FALSE
644667
--

0 commit comments

Comments
 (0)