Fix handling of PlaceHolderVars in nestloop parameter management.

tglsfdc · tglsfdc · commit e4e60e7b6125 · 2011-11-03T00:51:06.000-04:00
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.
diff --git a/src/backend/executor/nodeNestloop.c b/src/backend/executor/nodeNestloop.c
@@ -148,6 +148,7 @@ ExecNestLoop(NestLoopState *node)
 
 				prm = &(econtext->ecxt_param_exec_vals[paramno]);
 				/* Param value should be an OUTER var */
+				Assert(IsA(nlp->paramval, Var));
 				Assert(nlp->paramval->varno == OUTER);
 				Assert(nlp->paramval->varattno > 0);
 				prm->value = slot_getattr(outerTupleSlot,
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
@@ -27,6 +27,7 @@
 #include "optimizer/clauses.h"
 #include "optimizer/cost.h"
 #include "optimizer/paths.h"
+#include "optimizer/placeholder.h"
 #include "optimizer/plancat.h"
 #include "optimizer/planmain.h"
 #include "optimizer/predtest.h"
@@ -1886,7 +1887,20 @@ create_nestloop_plan(PlannerInfo *root,
 		NestLoopParam *nlp = (NestLoopParam *) lfirst(cell);
 
 		next = lnext(cell);
-		if (bms_is_member(nlp->paramval->varno, outerrelids))
+		if (IsA(nlp->paramval, Var) &&
+			bms_is_member(nlp->paramval->varno, outerrelids))
+		{
+			root->curOuterParams = list_delete_cell(root->curOuterParams,
+													cell, prev);
+			nestParams = lappend(nestParams, nlp);
+		}
+		else if (IsA(nlp->paramval, PlaceHolderVar) &&
+				 bms_overlap(((PlaceHolderVar *) nlp->paramval)->phrels,
+							 outerrelids) &&
+				 bms_is_subset(find_placeholder_info(root,
+													 (PlaceHolderVar *) nlp->paramval,
+													 false)->ph_eval_at,
+							   outerrelids))
 		{
 			root->curOuterParams = list_delete_cell(root->curOuterParams,
 													cell, prev);
@@ -2314,11 +2328,12 @@ create_hashjoin_plan(PlannerInfo *root,
 
 /*
  * replace_nestloop_params
- *	  Replace outer-relation Vars in the given expression with nestloop Params
+ *	  Replace outer-relation Vars and PlaceHolderVars in the given expression
+ *	  with nestloop Params
  *
- * All Vars belonging to the relation(s) identified by root->curOuterRels
- * are replaced by Params, and entries are added to root->curOuterParams if
- * not already present.
+ * All Vars and PlaceHolderVars belonging to the relation(s) identified by
+ * root->curOuterRels are replaced by Params, and entries are added to
+ * root->curOuterParams if not already present.
  */
 static Node *
 replace_nestloop_params(PlannerInfo *root, Node *expr)
@@ -2345,7 +2360,7 @@ replace_nestloop_params_mutator(Node *node, PlannerInfo *root)
 		if (!bms_is_member(var->varno, root->curOuterRels))
 			return node;
 		/* Create a Param representing the Var */
-		param = assign_nestloop_param(root, var);
+		param = assign_nestloop_param_var(root, var);
 		/* Is this param already listed in root->curOuterParams? */
 		foreach(lc, root->curOuterParams)
 		{
@@ -2365,6 +2380,48 @@ replace_nestloop_params_mutator(Node *node, PlannerInfo *root)
 		/* And return the replacement Param */
 		return (Node *) param;
 	}
+	if (IsA(node, PlaceHolderVar))
+	{
+		PlaceHolderVar *phv = (PlaceHolderVar *) node;
+		Param	   *param;
+		NestLoopParam *nlp;
+		ListCell   *lc;
+
+		/* Upper-level PlaceHolderVars should be long gone at this point */
+		Assert(phv->phlevelsup == 0);
+
+		/*
+		 * If not to be replaced, just return the PlaceHolderVar unmodified.
+		 * We use bms_overlap as a cheap/quick test to see if the PHV might
+		 * be evaluated in the outer rels, and then grab its PlaceHolderInfo
+		 * to tell for sure.
+		 */
+		if (!bms_overlap(phv->phrels, root->curOuterRels))
+			return node;
+		if (!bms_is_subset(find_placeholder_info(root, phv, false)->ph_eval_at,
+						   root->curOuterRels))
+			return node;
+		/* Create a Param representing the PlaceHolderVar */
+		param = assign_nestloop_param_placeholdervar(root, phv);
+		/* Is this param already listed in root->curOuterParams? */
+		foreach(lc, root->curOuterParams)
+		{
+			nlp = (NestLoopParam *) lfirst(lc);
+			if (nlp->paramno == param->paramid)
+			{
+				Assert(equal(phv, nlp->paramval));
+				/* Present, so we can just return the Param */
+				return (Node *) param;
+			}
+		}
+		/* No, so add it */
+		nlp = makeNode(NestLoopParam);
+		nlp->paramno = param->paramid;
+		nlp->paramval = (Var *) phv;
+		root->curOuterParams = lappend(root->curOuterParams, nlp);
+		/* And return the replacement Param */
+		return (Node *) param;
+	}
 	return expression_tree_mutator(node,
 								   replace_nestloop_params_mutator,
 								   (void *) root);
@@ -2377,7 +2434,7 @@ replace_nestloop_params_mutator(Node *node, PlannerInfo *root)
  *
  * We have four tasks here:
  *	* Remove RestrictInfo nodes from the input clauses.
- *	* Replace any outer-relation Var nodes with nestloop Params.
+ *	* Replace any outer-relation Var or PHV nodes with nestloop Params.
  *	  (XXX eventually, that responsibility should go elsewhere?)
  *	* Index keys must be represented by Var nodes with varattno set to the
  *	  index's attribute number, not the attribute number in the original rel.
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
@@ -1019,6 +1019,10 @@ set_join_references(PlannerGlobal *glob, Join *join, int rtoffset)
 												   (Node *) nlp->paramval,
 												   outer_itlist,
 												   rtoffset);
+			/* Check we replaced any PlaceHolderVar with simple Var */
+			if (!(IsA(nlp->paramval, Var) &&
+				  nlp->paramval->varno == OUTER))
+				elog(ERROR, "NestLoopParam was not reduced to a simple Var");
 		}
 	}
 	else if (IsA(join, MergeJoin))
diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c
@@ -172,7 +172,7 @@ replace_outer_var(PlannerInfo *root, Var *var)
  * the Var to be local to the current query level.
  */
 Param *
-assign_nestloop_param(PlannerInfo *root, Var *var)
+assign_nestloop_param_var(PlannerInfo *root, Var *var)
 {
 	Param	   *retval;
 	int			i;
@@ -192,6 +192,65 @@ assign_nestloop_param(PlannerInfo *root, Var *var)
 	return retval;
 }
 
+/*
+ * Generate a Param node to replace the given PlaceHolderVar, which will be
+ * supplied from an upper NestLoop join node.
+ *
+ * This is just like assign_nestloop_param_var, except for PlaceHolderVars.
+ */
+Param *
+assign_nestloop_param_placeholdervar(PlannerInfo *root, PlaceHolderVar *phv)
+{
+	Param	   *retval;
+	ListCell   *ppl;
+	PlannerParamItem *pitem;
+	Index		abslevel;
+	int			i;
+
+	Assert(phv->phlevelsup == 0);
+	abslevel = root->query_level;
+
+	/* If there's already a paramlist entry for this same PHV, just use it */
+	/* We assume comparing the PHIDs is sufficient */
+	i = 0;
+	foreach(ppl, root->glob->paramlist)
+	{
+		pitem = (PlannerParamItem *) lfirst(ppl);
+		if (pitem->abslevel == abslevel && IsA(pitem->item, PlaceHolderVar))
+		{
+			PlaceHolderVar *pphv = (PlaceHolderVar *) pitem->item;
+
+			if (pphv->phid == phv->phid)
+				break;
+		}
+		i++;
+	}
+
+	if (ppl == NULL)
+	{
+		/* Nope, so make a new one */
+		phv = (PlaceHolderVar *) copyObject(phv);
+
+		pitem = makeNode(PlannerParamItem);
+		pitem->item = (Node *) phv;
+		pitem->abslevel = abslevel;
+
+		root->glob->paramlist = lappend(root->glob->paramlist, pitem);
+
+		/* i is already the correct list index for the new item */
+	}
+
+	retval = makeNode(Param);
+	retval->paramkind = PARAM_EXEC;
+	retval->paramid = i;
+	retval->paramtype = exprType((Node *) phv->phexpr);
+	retval->paramtypmod = exprTypmod((Node *) phv->phexpr);
+	retval->paramcollid = exprCollation((Node *) phv->phexpr);
+	retval->location = -1;
+
+	return retval;
+}
+
 /*
  * Generate a Param node to replace the given Aggref
  * which is expected to have agglevelsup > 0 (ie, it is not local).
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
@@ -488,7 +488,9 @@ typedef struct Join
  * The nestParams list identifies any executor Params that must be passed
  * into execution of the inner subplan carrying values from the current row
  * of the outer subplan.  Currently we restrict these values to be simple
- * Vars, but perhaps someday that'd be worth relaxing.
+ * Vars, but perhaps someday that'd be worth relaxing.  (Note: during plan
+ * creation, the paramval can actually be a PlaceHolderVar expression; but it
+ * must be a Var with varno OUTER_VAR by the time it gets to the executor.)
  * ----------------
  */
 typedef struct NestLoop
diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
@@ -1432,13 +1432,16 @@ typedef struct MinMaxAggInfo
  *
  * Each paramlist item shows the absolute query level it is associated with,
  * where the outermost query is level 1 and nested subqueries have higher
- * numbers.  The item the parameter slot represents can be one of three kinds:
+ * numbers.  The item the parameter slot represents can be one of four kinds:
  *
  * A Var: the slot represents a variable of that level that must be passed
  * down because subqueries have outer references to it, or must be passed
  * from a NestLoop node of that level to its inner scan.  The varlevelsup
  * value in the Var will always be zero.
  *
+ * A PlaceHolderVar: this works much like the Var case.  It is currently
+ * only needed for NestLoop parameters, not outer references.
+ *
  * An Aggref (with an expression tree representing its argument): the slot
  * represents an aggregate expression that is an outer reference for some
  * subquery.  The Aggref itself has agglevelsup = 0, and its argument tree
@@ -1448,20 +1451,20 @@ typedef struct MinMaxAggInfo
  * for that subplan).  The absolute level shown for such items corresponds
  * to the parent query of the subplan.
  *
- * Note: we detect duplicate Var parameters and coalesce them into one slot,
- * but we do not bother to do this for Aggrefs, and it would be incorrect
- * to do so for Param slots.  Duplicate detection is actually *necessary*
- * in the case of NestLoop parameters since it serves to match up the usage
- * of a Param (in the inner scan) with the assignment of the value (in the
- * NestLoop node).	This might result in the same PARAM_EXEC slot being used
- * by multiple NestLoop nodes or SubPlan nodes, but no harm is done since
+ * Note: we detect duplicate Var and PlaceHolderVar parameters and coalesce
+ * them into one slot, but we do not bother to do this for Aggrefs, and it
+ * would be incorrect to do so for Param slots.  Duplicate detection is
+ * actually *necessary* for NestLoop parameters since it serves to match up
+ * the usage of a Param (in the inner scan) with the assignment of the value
+ * (in the NestLoop node). This might result in the same PARAM_EXEC slot being
+ * used by multiple NestLoop nodes or SubPlan nodes, but no harm is done since
  * the same value would be assigned anyway.
  */
 typedef struct PlannerParamItem
 {
 	NodeTag		type;
 
-	Node	   *item;			/* the Var, Aggref, or Param */
+	Node	   *item;			/* the Var, PlaceHolderVar, Aggref, or Param */
 	Index		abslevel;		/* its absolute query level */
 } PlannerParamItem;
 
diff --git a/src/include/optimizer/subselect.h b/src/include/optimizer/subselect.h
@@ -29,7 +29,9 @@ extern void SS_finalize_plan(PlannerInfo *root, Plan *plan,
 				 bool attach_initplans);
 extern Param *SS_make_initplan_from_plan(PlannerInfo *root, Plan *plan,
 					Oid resulttype, int32 resulttypmod, Oid resultcollation);
-extern Param *assign_nestloop_param(PlannerInfo *root, Var *var);
+extern Param *assign_nestloop_param_var(PlannerInfo *root, Var *var);
+extern Param *assign_nestloop_param_placeholdervar(PlannerInfo *root,
+												   PlaceHolderVar *phv);
 extern int	SS_assign_special_param(PlannerInfo *root);
 
 #endif   /* SUBSELECT_H */
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
@@ -2129,6 +2129,7 @@ on (x1 = xx1) where (xx2 is not null);
 -- regression test: check for bug with propagation of implied equality
 -- to outside an IN
 --
+analyze tenk1;		-- ensure we get consistent plans here
 select count(*) from tenk1 a where unique1 in
   (select unique1 from tenk1 b join tenk1 c using (unique1)
    where b.unique2 = 42);
@@ -2533,6 +2534,43 @@ ON sub1.key1 = sub2.key3;
     1 |    1 |      1 |      1
 (1 row)
 
+--
+-- test case where a PlaceHolderVar is used as a nestloop parameter
+--
+EXPLAIN (COSTS OFF)
+SELECT qq, unique1
+  FROM
+  ( SELECT COALESCE(q1, 0) AS qq FROM int8_tbl a ) AS ss1
+  FULL OUTER JOIN
+  ( SELECT COALESCE(q2, -1) AS qq FROM int8_tbl b ) AS ss2
+  USING (qq)
+  INNER JOIN tenk1 c ON qq = unique2;
+                                              QUERY PLAN                                               
+-------------------------------------------------------------------------------------------------------
+ Nested Loop
+   ->  Hash Full Join
+         Hash Cond: (COALESCE(a.q1, 0::bigint) = COALESCE(b.q2, (-1)::bigint))
+         ->  Seq Scan on int8_tbl a
+         ->  Hash
+               ->  Seq Scan on int8_tbl b
+   ->  Index Scan using tenk1_unique2 on tenk1 c
+         Index Cond: (unique2 = COALESCE((COALESCE(a.q1, 0::bigint)), (COALESCE(b.q2, (-1)::bigint))))
+(8 rows)
+
+SELECT qq, unique1
+  FROM
+  ( SELECT COALESCE(q1, 0) AS qq FROM int8_tbl a ) AS ss1
+  FULL OUTER JOIN
+  ( SELECT COALESCE(q2, -1) AS qq FROM int8_tbl b ) AS ss2
+  USING (qq)
+  INNER JOIN tenk1 c ON qq = unique2;
+ qq  | unique1 
+-----+---------
+ 123 |    4596
+ 123 |    4596
+ 456 |    7318
+(3 rows)
+
 --
 -- test the corner cases FULL JOIN ON TRUE and FULL JOIN ON FALSE
 --
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
@@ -330,6 +330,8 @@ on (x1 = xx1) where (xx2 is not null);
 -- regression test: check for bug with propagation of implied equality
 -- to outside an IN
 --
+analyze tenk1;		-- ensure we get consistent plans here
+
 select count(*) from tenk1 a where unique1 in
   (select unique1 from tenk1 b join tenk1 c using (unique1)
    where b.unique2 = 42);
@@ -639,6 +641,27 @@ LEFT JOIN
 ) sub2
 ON sub1.key1 = sub2.key3;
 
+--
+-- test case where a PlaceHolderVar is used as a nestloop parameter
+--
+
+EXPLAIN (COSTS OFF)
+SELECT qq, unique1
+  FROM
+  ( SELECT COALESCE(q1, 0) AS qq FROM int8_tbl a ) AS ss1
+  FULL OUTER JOIN
+  ( SELECT COALESCE(q2, -1) AS qq FROM int8_tbl b ) AS ss2
+  USING (qq)
+  INNER JOIN tenk1 c ON qq = unique2;
+
+SELECT qq, unique1
+  FROM
+  ( SELECT COALESCE(q1, 0) AS qq FROM int8_tbl a ) AS ss1
+  FULL OUTER JOIN
+  ( SELECT COALESCE(q2, -1) AS qq FROM int8_tbl b ) AS ss2
+  USING (qq)
+  INNER JOIN tenk1 c ON qq = unique2;
+
 --
 -- test the corner cases FULL JOIN ON TRUE and FULL JOIN ON FALSE
 --

Original file line number	Diff line number	Diff line change
`@@ -1019,6 +1019,10 @@ set_join_references(PlannerGlobal glob, Join join, int rtoffset)`
`1019`	`1019`	`(Node *) nlp->paramval,`
`1020`	`1020`	`outer_itlist,`
`1021`	`1021`	`rtoffset);`
	`1022`	`+ /* Check we replaced any PlaceHolderVar with simple Var */`
	`1023`	`+ if (!(IsA(nlp->paramval, Var) &&`
	`1024`	`+ nlp->paramval->varno == OUTER))`
	`1025`	`+ elog(ERROR, "NestLoopParam was not reduced to a simple Var");`
`1022`	`1026`	`}`
`1023`	`1027`	`}`
`1024`	`1028`	`else if (IsA(join, MergeJoin))`