Tighten up SS_finalize_plan's computation of valid_params to exclude Params of

the current query level that aren't in fact output parameters of the current
initPlans.  (This means, for example, output parameters of regular subplans.)
To make this work correctly for output parameters coming from sibling
initplans requires rejiggering the API of SS_finalize_plan just a bit:
we need the siblings to be visible to it, rather than hidden as
SS_make_initplan_from_plan had been doing.  This is really part of my response
to bug #4290, but I concluded this part probably shouldn't be back-patched,
since all that it's doing is to make a debugging cross-check tighter.

Author: tglsfdc

src/backend/optimizer/plan/planagg.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/optimizer/plan/planagg.c,v 1.40 2008/07/10 01:17:29 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/plan/planagg.c,v 1.41 2008/07/10 02:14:03 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -486,7 +486,6 @@ make_agg_subplan(PlannerInfo *root, MinMaxAggInfo *info)
 	 */
 	memcpy(&subroot, root, sizeof(PlannerInfo));
 	subroot.parse = subparse = (Query *) copyObject(root->parse);
-	subroot.init_plans = NIL;
 	subparse->commandType = CMD_SELECT;
 	subparse->resultRelation = 0;
 	subparse->returningList = NIL;
@@ -564,11 +563,9 @@ make_agg_subplan(PlannerInfo *root, MinMaxAggInfo *info)
 											 -1);
 
 	/*
-	 * Make sure the InitPlan gets into the outer list.  It has to appear
-	 * after any other InitPlans it might depend on, too (see comments in
-	 * ExecReScan).
+	 * Put the updated list of InitPlans back into the outer PlannerInfo.
 	 */
-	root->init_plans = list_concat(root->init_plans, subroot.init_plans);
+	root->init_plans = subroot.init_plans;
 }
 
 /*

src/backend/optimizer/plan/planner.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.233 2008/05/02 21:26:09 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.234 2008/07/10 02:14:03 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -459,7 +459,7 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
 	 */
 	if (list_length(glob->subplans) != num_old_subplans ||
 		root->query_level > 1)
-		SS_finalize_plan(root, plan);
+		SS_finalize_plan(root, plan, true);
 
 	/* Return internal info if caller wants it */
 	if (subroot)

src/backend/optimizer/plan/subselect.c

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/optimizer/plan/subselect.c,v 1.131 2008/07/10 01:17:29 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/plan/subselect.c,v 1.132 2008/07/10 02:14:03 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1027,11 +1027,12 @@ process_sublinks_mutator(Node *node, process_sublinks_context *context)
  * SS_finalize_plan - do final sublink processing for a completed Plan.
  *
  * This recursively computes the extParam and allParam sets for every Plan
- * node in the given plan tree.  It also attaches any generated InitPlans
- * to the top plan node.
+ * node in the given plan tree.  It also optionally attaches any previously
+ * generated InitPlans to the top plan node.  (Any InitPlans should already
+ * have been put through SS_finalize_plan.)
  */
 void
-SS_finalize_plan(PlannerInfo *root, Plan *plan)
+SS_finalize_plan(PlannerInfo *root, Plan *plan, bool attach_initplans)
 {
 	Bitmapset  *valid_params,
 			   *initExtParam,
@@ -1041,14 +1042,45 @@ SS_finalize_plan(PlannerInfo *root, Plan *plan)
 	ListCell   *l;
 
 	/*
-	 * First, scan the param list to discover the sets of params that are
-	 * available from outer query levels and my own query level. We do this
-	 * once to save time in the per-plan recursion steps.  (This calculation
-	 * is overly generous: it can include a lot of params that actually
-	 * shouldn't be referenced here.  However, valid_params is just used as
-	 * a debugging crosscheck, so it's not worth trying to be exact.)
+	 * Examine any initPlans to determine the set of external params they
+	 * reference, the set of output params they supply, and their total cost.
+	 * We'll use at least some of this info below.  (Note we are assuming that
+	 * finalize_plan doesn't touch the initPlans.)
+	 *
+	 * In the case where attach_initplans is false, we are assuming that the
+	 * existing initPlans are siblings that might supply params needed by the
+	 * current plan.
+	 */
+	initExtParam = initSetParam = NULL;
+	initplan_cost = 0;
+	foreach(l, root->init_plans)
+	{
+		SubPlan    *initsubplan = (SubPlan *) lfirst(l);
+		Plan	   *initplan = planner_subplan_get_plan(root, initsubplan);
+		ListCell   *l2;
+
+		initExtParam = bms_add_members(initExtParam, initplan->extParam);
+		foreach(l2, initsubplan->setParam)
+		{
+			initSetParam = bms_add_member(initSetParam, lfirst_int(l2));
+		}
+		initplan_cost += get_initplan_cost(root, initsubplan);
+	}
+
+	/*
+	 * Now determine the set of params that are validly referenceable in this
+	 * query level; to wit, those available from outer query levels plus the
+	 * output parameters of any initPlans.  (We do not include output
+	 * parameters of regular subplans.  Those should only appear within the
+	 * testexpr of SubPlan nodes, and are taken care of locally within
+	 * finalize_primnode.)
+	 *
+	 * Note: this is a bit overly generous since some parameters of upper
+	 * query levels might belong to query subtrees that don't include this
+	 * query.  However, valid_params is only a debugging crosscheck, so it
+	 * doesn't seem worth expending lots of cycles to try to be exact.
 	 */
-	valid_params = NULL;
+	valid_params = bms_copy(initSetParam);
 	paramid = 0;
 	foreach(l, root->glob->paramlist)
 	{
@@ -1059,12 +1091,6 @@ SS_finalize_plan(PlannerInfo *root, Plan *plan)
 			/* valid outer-level parameter */
 			valid_params = bms_add_member(valid_params, paramid);
 		}
-		else if (pitem->abslevel == root->query_level &&
-				 IsA(pitem->item, Param))
-		{
-			/* valid local parameter (i.e., a setParam of my child) */
-			valid_params = bms_add_member(valid_params, paramid);
-		}
 
 		paramid++;
 	}
@@ -1088,37 +1114,25 @@ SS_finalize_plan(PlannerInfo *root, Plan *plan)
 	 * top node.  This is a conservative overestimate, since in fact each
 	 * initPlan might be executed later than plan startup, or even not at all.
 	 */
-	plan->initPlan = root->init_plans;
-	root->init_plans = NIL;		/* make sure they're not attached twice */
-
-	initExtParam = initSetParam = NULL;
-	initplan_cost = 0;
-	foreach(l, plan->initPlan)
+	if (attach_initplans)
 	{
-		SubPlan    *initsubplan = (SubPlan *) lfirst(l);
-		Plan	   *initplan = planner_subplan_get_plan(root, initsubplan);
-		ListCell   *l2;
-
-		initExtParam = bms_add_members(initExtParam, initplan->extParam);
-		foreach(l2, initsubplan->setParam)
-		{
-			initSetParam = bms_add_member(initSetParam, lfirst_int(l2));
-		}
-		initplan_cost += get_initplan_cost(root, initsubplan);
+		plan->initPlan = root->init_plans;
+		root->init_plans = NIL;		/* make sure they're not attached twice */
+
+		/* allParam must include all these params */
+		plan->allParam = bms_add_members(plan->allParam, initExtParam);
+		plan->allParam = bms_add_members(plan->allParam, initSetParam);
+		/* extParam must include any child extParam */
+		plan->extParam = bms_add_members(plan->extParam, initExtParam);
+		/* but extParam shouldn't include any setParams */
+		plan->extParam = bms_del_members(plan->extParam, initSetParam);
+		/* ensure extParam is exactly NULL if it's empty */
+		if (bms_is_empty(plan->extParam))
+			plan->extParam = NULL;
+
+		plan->startup_cost += initplan_cost;
+		plan->total_cost += initplan_cost;
 	}
-	/* allParam must include all these params */
-	plan->allParam = bms_add_members(plan->allParam, initExtParam);
-	plan->allParam = bms_add_members(plan->allParam, initSetParam);
-	/* extParam must include any child extParam */
-	plan->extParam = bms_add_members(plan->extParam, initExtParam);
-	/* but extParam shouldn't include any setParams */
-	plan->extParam = bms_del_members(plan->extParam, initSetParam);
-	/* ensure extParam is exactly NULL if it's empty */
-	if (bms_is_empty(plan->extParam))
-		plan->extParam = NULL;
-
-	plan->startup_cost += initplan_cost;
-	plan->total_cost += initplan_cost;
 }
 
 /*
@@ -1412,29 +1426,22 @@ Param *
 SS_make_initplan_from_plan(PlannerInfo *root, Plan *plan,
 						   Oid resulttype, int32 resulttypmod)
 {
-	List	   *saved_init_plans;
 	SubPlan    *node;
 	Param	   *prm;
 
 	/*
 	 * We must run SS_finalize_plan(), since that's normally done before a
-	 * subplan gets put into the initplan list.  However it will try to attach
-	 * any pre-existing initplans to this one, which we don't want (they are
-	 * siblings not children of this initplan).  So, a quick kluge to hide
-	 * them.  (This is something else that could perhaps be cleaner if we did
-	 * extParam/allParam processing in setrefs.c instead of here?  See notes
-	 * for materialize_finished_plan.)
+	 * subplan gets put into the initplan list.  Tell it not to attach any
+	 * pre-existing initplans to this one, since they are siblings not
+	 * children of this initplan.  (This is something else that could perhaps
+	 * be cleaner if we did extParam/allParam processing in setrefs.c instead
+	 * of here?  See notes for materialize_finished_plan.)
 	 */
-	saved_init_plans = root->init_plans;
-	root->init_plans = NIL;
 
 	/*
 	 * Build extParam/allParam sets for plan nodes.
 	 */
-	SS_finalize_plan(root, plan);
-
-	/* Restore outer initplan list */
-	root->init_plans = saved_init_plans;
+	SS_finalize_plan(root, plan, false);
 
 	/*
 	 * Add the subplan and its rtable to the global lists.
@@ -1446,6 +1453,8 @@ SS_make_initplan_from_plan(PlannerInfo *root, Plan *plan,
 
 	/*
 	 * Create a SubPlan node and add it to the outer list of InitPlans.
+	 * Note it has to appear after any other InitPlans it might depend on
+	 * (see comments in ExecReScan).
 	 */
 	node = makeNode(SubPlan);
 	node->subLinkType = EXPR_SUBLINK;

src/include/optimizer/subselect.h

@@ -5,7 +5,7 @@
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/optimizer/subselect.h,v 1.30 2008/01/01 19:45:58 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/optimizer/subselect.h,v 1.31 2008/07/10 02:14:03 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -18,7 +18,8 @@
 extern Node *convert_IN_to_join(PlannerInfo *root, SubLink *sublink);
 extern Node *SS_replace_correlation_vars(PlannerInfo *root, Node *expr);
 extern Node *SS_process_sublinks(PlannerInfo *root, Node *expr, bool isQual);
-extern void SS_finalize_plan(PlannerInfo *root, Plan *plan);
+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);