diff --git a/src/backend/optimizer/plan/planmain.c b/src/backend/optimizer/plan/planmain.c
index 0e0842f357ff2b7ced54c6b10ed945ea022c8acc..0708e963b4b1aff6cabb0c2c8a702cde080074a9 100644
--- a/src/backend/optimizer/plan/planmain.c
+++ b/src/backend/optimizer/plan/planmain.c
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planmain.c,v 1.34 1999/02/21 03:48:49 scrappy Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planmain.c,v 1.35 1999/05/03 00:38:43 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -44,9 +44,6 @@
 static Plan *subplanner(Query *root, List *flat_tlist, List *qual);
 static Result *make_result(List *tlist, Node *resconstantqual, Plan *subplan);
 
-extern Plan *make_groupPlan(List **tlist, bool tuplePerGroup,
-			   List *groupClause, Plan *subplan);
-
 /*
  * query_planner
  *	  Routine to create a query plan.  It does so by first creating a
@@ -177,18 +174,14 @@ query_planner(Query *root,
 	 */
 	if (constant_qual)
 	{
-		subplan = (Plan *) make_result((!root->hasAggs &&
-										!root->groupClause &&
-										!root->havingQual)
-									   ? tlist : subplan->targetlist,
+		subplan = (Plan *) make_result(tlist,
 									   (Node *) constant_qual,
 									   subplan);
 
 		/*
-		 * Change all varno's of the Result's node target list.
+		 * Fix all varno's of the Result's node target list.
 		 */
-		if (!root->hasAggs && !root->groupClause && !root->havingQual)
-			set_tlist_references(subplan);
+		set_tlist_references(subplan);
 
 		return subplan;
 	}
@@ -201,16 +194,15 @@ query_planner(Query *root,
 	 * responsibility to optimally push these expressions down the plan
 	 * tree.  -- Wei
 	 *
-	 * But now nothing to do if there are GroupBy and/or Aggregates: 1.
-	 * make_groupPlan fixes tlist; 2. flatten_tlist_vars does nothing with
-	 * aggregates fixing only other entries (i.e. - GroupBy-ed and so
-	 * fixed by make_groupPlan).	 - vadim 04/05/97
+	 * Note: formerly there was a test here to skip the flatten call if we
+	 * expected union_planner to insert a Group or Agg node above our result.
+	 * However, now union_planner tells us exactly what it wants returned,
+	 * and we just do it.  Much cleaner.
 	 */
 	else
 	{
-		if (!root->hasAggs && !root->groupClause && !root->havingQual)
-			subplan->targetlist = flatten_tlist_vars(tlist,
-													 subplan->targetlist);
+		subplan->targetlist = flatten_tlist_vars(tlist,
+												 subplan->targetlist);
 		return subplan;
 	}
 
@@ -321,201 +313,3 @@ make_result(List *tlist,
 
 	return node;
 }
-
-/*****************************************************************************
- *
- *****************************************************************************/
-
-Plan *
-make_groupPlan(List **tlist,
-			   bool tuplePerGroup,
-			   List *groupClause,
-			   Plan *subplan)
-{
-	List	   *sort_tlist;
-	List	   *sl,
-			   *gl;
-	List	   *glc = listCopy(groupClause);
-	List	   *otles = NIL;	/* list of removed non-GroupBy entries */
-	List	   *otlvars = NIL;	/* list of var in them */
-	int			otlvcnt;
-	Sort	   *sortplan;
-	Group	   *grpplan;
-	int			numCols;
-	AttrNumber *grpColIdx;
-	int			last_resno = 1;
-
-	numCols = length(groupClause);
-	grpColIdx = (AttrNumber *) palloc(sizeof(AttrNumber) * numCols);
-
-	sort_tlist = new_unsorted_tlist(*tlist);	/* it's copy */
-
-	/*
-	 * Make template TL for subplan, Sort & Group: 1. If there are
-	 * aggregates (tuplePerGroup is true) then take away non-GroupBy
-	 * entries and re-set resno-s accordantly. 2. Make grpColIdx
-	 *
-	 * Note: we assume that TLEs in *tlist are ordered in accordance with
-	 * their resdom->resno.
-	 */
-	foreach(sl, sort_tlist)
-	{
-		Resdom	   *resdom = NULL;
-		TargetEntry *te = (TargetEntry *) lfirst(sl);
-		int			keyno = 0;
-
-		foreach(gl, groupClause)
-		{
-			GroupClause *grpcl = (GroupClause *) lfirst(gl);
-
-			keyno++;
-			if (grpcl->entry->resdom->resno == te->resdom->resno)
-			{
-
-				resdom = te->resdom;
-				resdom->reskey = keyno;
-				resdom->reskeyop = get_opcode(grpcl->grpOpoid);
-				resdom->resno = last_resno;		/* re-set */
-				grpColIdx[keyno - 1] = last_resno++;
-				glc = lremove(lfirst(gl), glc); /* TLE found for it */
-				break;
-			}
-		}
-
-		/*
-		 * Non-GroupBy entry: remove it from Group/Sort TL if there are
-		 * aggregates in query - it will be evaluated by Aggregate plan
-		 */
-		if (resdom == NULL)
-		{
-			if (tuplePerGroup)
-			{
-				otlvars = nconc(otlvars, pull_var_clause(te->expr));
-				otles = lcons(te, otles);
-				sort_tlist = lremove(te, sort_tlist);
-			}
-			else
-				te->resdom->resno = last_resno++;
-		}
-	}
-
-	if (length(glc) != 0)
-		elog(ERROR, "group attribute disappeared from target list");
-
-	/*
-	 * If non-GroupBy entries were removed from TL - we are to add Vars
-	 * for them to the end of TL if there are no such Vars in TL already.
-	 */
-
-	otlvcnt = length(otlvars);
-	foreach(gl, otlvars)
-	{
-		Var		   *v = (Var *) lfirst(gl);
-
-		if (tlist_member(v, sort_tlist) == NULL)
-		{
-			sort_tlist = lappend(sort_tlist,
-								 create_tl_element(v, last_resno));
-			last_resno++;
-		}
-		else
-/* already in TL */
-			otlvcnt--;
-	}
-	/* Now otlvcnt is number of Vars added in TL for non-GroupBy entries */
-
-	/* Make TL for subplan: substitute Vars from subplan TL into new TL */
-	sl = flatten_tlist_vars(sort_tlist, subplan->targetlist);
-
-	subplan->targetlist = new_unsorted_tlist(sl);		/* there */
-
-	/*
-	 * Make Sort/Group TL : 1. make Var nodes (with varno = 1 and varnoold
-	 * = -1) for all functions, 'couse they will be evaluated by subplan;
-	 * 2. for real Vars: set varno = 1 and varattno to its resno in
-	 * subplan
-	 */
-	foreach(sl, sort_tlist)
-	{
-		TargetEntry *te = (TargetEntry *) lfirst(sl);
-		Resdom	   *resdom = te->resdom;
-		Node	   *expr = te->expr;
-
-		if (IsA(expr, Var))
-		{
-#ifdef NOT_USED							/* subplanVar->resdom->resno expected to
-								 * be = te->resdom->resno */
-			TargetEntry *subplanVar;
-
-			subplanVar = match_varid((Var *) expr, subplan->targetlist);
-			((Var *) expr)->varattno = subplanVar->resdom->resno;
-#endif
-			((Var *) expr)->varattno = te->resdom->resno;
-			((Var *) expr)->varno = 1;
-		}
-		else
-			te->expr = (Node *) makeVar(1, resdom->resno,
-										resdom->restype,
-										resdom->restypmod,
-										0, -1, resdom->resno);
-	}
-
-	sortplan = make_sort(sort_tlist,
-						 _NONAME_RELATION_ID_,
-						 subplan,
-						 numCols);
-	sortplan->plan.cost = subplan->cost;		/* XXX assume no cost */
-
-	/*
-	 * make the Group node
-	 */
-	sort_tlist = copyObject(sort_tlist);
-	grpplan = make_group(sort_tlist, tuplePerGroup, numCols,
-						 grpColIdx, sortplan);
-
-	/*
-	 * Make TL for parent: "restore" non-GroupBy entries (if they were
-	 * removed) and set resno-s of others accordantly.
-	 */
-	sl = sort_tlist;
-	sort_tlist = NIL;			/* to be new parent TL */
-	foreach(gl, *tlist)
-	{
-		List	   *temp = NIL;
-		TargetEntry *te = (TargetEntry *) lfirst(gl);
-
-		foreach(temp, otles)	/* Is it removed non-GroupBy entry ? */
-		{
-			TargetEntry *ote = (TargetEntry *) lfirst(temp);
-
-			if (ote->resdom->resno == te->resdom->resno)
-			{
-				otles = lremove(ote, otles);
-				break;
-			}
-		}
-		if (temp == NIL)		/* It's "our" TLE - we're to return */
-		{						/* it from Sort/Group plans */
-			TargetEntry *my = (TargetEntry *) lfirst(sl);		/* get it */
-
-			sl = sl->next;		/* prepare for the next "our" */
-			my = copyObject(my);
-			my->resdom->resno = te->resdom->resno;		/* order of parent TL */
-			sort_tlist = lappend(sort_tlist, my);
-			continue;
-		}
-		/* else - it's TLE of an non-GroupBy entry */
-		sort_tlist = lappend(sort_tlist, copyObject(te));
-	}
-
-	/*
-	 * Pure non-GroupBy entries Vars were at the end of Group' TL. They
-	 * shouldn't appear in parent TL, all others shouldn't disappear.
-	 */
-	Assert(otlvcnt == length(sl));
-	Assert(length(otles) == 0);
-
-	*tlist = sort_tlist;
-
-	return (Plan *) grpplan;
-}
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 543b5d6d7c6aaa8e971191e4704340b68f6f7e59..aeaa07658cc5aa3539d60039d2ab366e1e7099fc 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.47 1999/04/19 01:43:11 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.48 1999/05/03 00:38:43 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -55,10 +55,13 @@
 #include "access/genam.h"
 #include "parser/parse_oper.h"
 
+static List *make_subplanTargetList(Query *parse, List *tlist,
+									AttrNumber **groupColIdx);
+static Plan *make_groupplan(List *group_tlist, bool tuplePerGroup,
+							List *groupClause, AttrNumber *grpColIdx,
+							Plan *subplan);
 static bool need_sortplan(List *sortcls, Plan *plan);
 static Plan *make_sortplan(List *tlist, List *sortcls, Plan *plannode);
-extern Plan *make_groupPlan(List **tlist, bool tuplePerGroup,
-			   List *groupClause, Plan *subplan);
 
 /*****************************************************************************
  *
@@ -103,16 +106,9 @@ Plan *
 union_planner(Query *parse)
 {
 	List	   *tlist = parse->targetList;
-
-	/***S*H***/
-	/* copy the original tlist, we will need the original one 
-	 * for the AGG node later on */
-	List    *new_tlist = new_unsorted_tlist(tlist);
-		
 	List	   *rangetable = parse->rtable;
-
 	Plan	   *result_plan = (Plan *) NULL;
-
+	AttrNumber *groupColIdx = NULL;
 	Index		rt_index;
 
 	if (parse->unionClause)
@@ -138,36 +134,35 @@ union_planner(Query *parse)
 	else
 	{
 	  List  **vpm = NULL;
+	  List	*sub_tlist;
 
-	  /*
-	   * If there is a HAVING clause, make sure all vars referenced in it
-	   * are included in the target list for query_planner().
-	   */
-	  if (parse->havingQual)
-		  new_tlist = check_having_qual_for_vars(parse->havingQual, new_tlist);
-
-	  new_tlist = preprocess_targetlist(new_tlist,
-					    parse->commandType,
-					    parse->resultRelation,
-					    parse->rtable);
+	  /* Preprocess targetlist in case we are inside an INSERT/UPDATE. */
+	  tlist = preprocess_targetlist(tlist,
+									parse->commandType,
+									parse->resultRelation,
+									parse->rtable);
 
-	  /* FOR UPDATE ... */
+	  /* Add row-mark targets for UPDATE
+	   * (should this be done in preprocess_targetlist?)
+	   */
 	  if (parse->rowMark != NULL)
 	  {
 	  	List		   *l;
-		TargetEntry	   *ctid;
-		Resdom		   *resdom;
-		Var			   *var;
-		char		   *resname;
 
 		foreach (l, parse->rowMark)
 		{
-			if (!(((RowMark*)lfirst(l))->info & ROW_MARK_FOR_UPDATE))
+			RowMark		   *rowmark = (RowMark*) lfirst(l);
+			TargetEntry	   *ctid;
+			Resdom		   *resdom;
+			Var			   *var;
+			char		   *resname;
+
+			if (!(rowmark->info & ROW_MARK_FOR_UPDATE))
 				continue;
 
 			resname = (char*) palloc(32);
-			sprintf(resname, "ctid%u", ((RowMark*)lfirst(l))->rti);
-			resdom = makeResdom(length(new_tlist) + 1,
+			sprintf(resname, "ctid%u", rowmark->rti);
+			resdom = makeResdom(length(tlist) + 1,
 								TIDOID,
 								-1,
 								resname,
@@ -175,20 +170,20 @@ union_planner(Query *parse)
 								0,
 								1);
 
-			var = makeVar(((RowMark*)lfirst(l))->rti, -1, TIDOID, 
-							-1, 0, ((RowMark*)lfirst(l))->rti, -1);
+			var = makeVar(rowmark->rti, -1, TIDOID, 
+						  -1, 0, rowmark->rti, -1);
 
 			ctid = makeTargetEntry(resdom, (Node *) var);
-			new_tlist = lappend(new_tlist, ctid);
+			tlist = lappend(tlist, ctid);
 		}
 	  }
-	  
-	  /* Here starts the original (pre having) code */
-	  tlist = preprocess_targetlist(tlist,
-					parse->commandType,
-					parse->resultRelation,
-					parse->rtable);
-	  
+
+	  /* Generate appropriate target list for subplan; may be different
+	   * from tlist if grouping or aggregation is needed.
+	   */
+	  sub_tlist = make_subplanTargetList(parse, tlist, &groupColIdx);
+
+	  /* Generate the (sub) plan */
 	  if (parse->rtable != NULL)
 	  {
 	      vpm = (List **) palloc(length(parse->rtable) * sizeof(List *));
@@ -196,9 +191,9 @@ union_planner(Query *parse)
 	  }
 	  PlannerVarParam = lcons(vpm, PlannerVarParam);
 	  result_plan = query_planner(parse,
-				      parse->commandType,
-				      new_tlist,
-				      (List *) parse->qual);
+								  parse->commandType,
+								  sub_tlist,
+								  (List *) parse->qual);
 	  PlannerVarParam = lnext(PlannerVarParam);
 	  if (vpm != NULL)
 	    pfree(vpm);		 
@@ -211,21 +206,28 @@ union_planner(Query *parse)
 	if (parse->groupClause)
 	{
 		bool		tuplePerGroup;
+		List	   *group_tlist;
 
 		/*
-		 * decide whether how many tuples per group the Group node needs
+		 * Decide whether how many tuples per group the Group node needs
 		 * to return. (Needs only one tuple per group if no aggregate is
 		 * present. Otherwise, need every tuple from the group to do the
-		 * aggregation.)
+		 * aggregation.)  Note tuplePerGroup is named backwards :-(
 		 */
 		tuplePerGroup = parse->hasAggs;
 
-		/***S*H***/
-		/* Use 'new_tlist' instead of 'tlist' */
-		result_plan = make_groupPlan(&new_tlist,
-								   tuplePerGroup,
-								   parse->groupClause,
-								   result_plan);
+		/* If there are aggregates then the Group node should just return
+		 * the same (simplified) tlist as the subplan, which we indicate
+		 * to make_groupplan by passing NIL.  If there are no aggregates
+		 * then the Group node had better compute the final tlist.
+		 */
+		group_tlist = parse->hasAggs ? NIL : tlist;
+
+		result_plan = make_groupplan(group_tlist,
+									 tuplePerGroup,
+									 parse->groupClause,
+									 groupColIdx,
+									 result_plan);
 	}
 
 	/*
@@ -274,10 +276,6 @@ union_planner(Query *parse)
 	 */
 	if (parse->hasAggs)
 	{
-		/* Use 'tlist' not 'new_tlist' as target list because we
-		 * don't want the additional attributes used for the havingQual
-		 * (see above) to show up in the result
-		 */
 		result_plan = (Plan *) make_agg(tlist, result_plan);
 
 		/* HAVING clause, if any, becomes qual of the Agg node */
@@ -289,6 +287,16 @@ union_planner(Query *parse)
 		 */
 		if (! set_agg_tlist_references((Agg *) result_plan))
 			elog(ERROR, "SELECT/HAVING requires aggregates to be valid");
+
+		/*
+		 * Check that we actually found some aggregates, else executor
+		 * will die unpleasantly.  (The rewrite module currently has bugs
+		 * that allow hasAggs to be incorrectly set 'true' sometimes.
+		 * It's not easy to recover here, since we've already made decisions
+		 * assuming there will be an Agg node.)
+		 */
+		if (((Agg *) result_plan)->aggs == NIL)
+			elog(ERROR, "union_planner: query is marked hasAggs, but I don't see any");
 	}		  
 
 	/*
@@ -316,6 +324,276 @@ union_planner(Query *parse)
 
 }
 
+/*---------------
+ * make_subplanTargetList
+ *	  Generate appropriate target lists when grouping is required.
+ *
+ * When union_planner inserts Aggregate and/or Group/Sort plan nodes above
+ * the result of query_planner, we typically need to pass a different
+ * target list to query_planner than the outer plan nodes should have.
+ * This routine generates the correct target list for the subplan, and
+ * if necessary modifies the target list for the inserted nodes as well.
+ *
+ * The initial target list passed from the parser already contains entries
+ * for all ORDER BY and GROUP BY expressions, but it will not have entries
+ * for variables used only in HAVING clauses; so we need to add those
+ * variables to the subplan target list.  Also, if we are doing either
+ * grouping or aggregation, we flatten all expressions except GROUP BY items
+ * into their component variables; the other expressions will be computed by
+ * the inserted nodes rather than by the subplan.  For example,
+ * given a query like
+ *		SELECT a+b,SUM(c+d) FROM table GROUP BY a+b;
+ * we want to pass this targetlist to the subplan:
+ *		a+b,c,d
+ * where the a+b target will be used by the Sort/Group steps, and the
+ * c and d targets will be needed to compute the aggregate results.
+ *
+ * 'parse' is the query being processed.
+ * 'tlist' is the query's target list.  CAUTION: list elements may be
+ * modified by this routine!
+ * 'groupColIdx' receives an array of column numbers for the GROUP BY
+ * expressions (if there are any) in the subplan's target list.
+ *
+ * The result is the targetlist to be passed to the subplan.  Also,
+ * the parent tlist is modified so that any nontrivial targetlist items that
+ * exactly match GROUP BY items are replaced by simple Var nodes referencing
+ * those outputs of the subplan.  This avoids redundant recalculations in
+ * cases like
+ *		SELECT a+1, ... GROUP BY a+1
+ * Note, however, that other varnodes in the parent's targetlist (and
+ * havingQual, if any) will still need to be updated to refer to outputs
+ * of the subplan.  This routine is quite large enough already, so we do
+ * that later.
+ *---------------
+ */
+static List *
+make_subplanTargetList(Query *parse,
+					   List *tlist,
+					   AttrNumber **groupColIdx)
+{
+	List	   *sub_tlist;
+	List	   *prnt_tlist;
+	List	   *sl,
+			   *gl;
+	List	   *glc = NIL;
+	List	   *extravars = NIL;
+	int			numCols;
+	AttrNumber *grpColIdx = NULL;
+	int			next_resno = 1;
+
+	*groupColIdx = NULL;
+
+	/* If we're not grouping or aggregating, nothing to do here;
+	 * query_planner should receive the unmodified target list.
+	 */
+	if (!parse->hasAggs && !parse->groupClause && !parse->havingQual)
+		return tlist;
+
+	/* If grouping, make a working copy of groupClause list (which we use
+	 * just to verify that we found all the groupClause items in tlist).
+	 * Also allocate space to remember where the group columns are in the
+	 * subplan tlist.
+	 */
+	numCols = length(parse->groupClause);
+	if (numCols > 0)
+	{
+		glc = listCopy(parse->groupClause);
+		grpColIdx = (AttrNumber *) palloc(sizeof(AttrNumber) * numCols);
+		*groupColIdx = grpColIdx;
+	}
+
+	sub_tlist = new_unsorted_tlist(tlist);	/* make a modifiable copy */
+
+	/*
+	 * Step 1: build grpColIdx by finding targetlist items that match
+	 * GroupBy entries.  If there are aggregates, remove non-GroupBy items
+	 * from sub_tlist, and reset its resnos accordingly.  When we leave an
+	 * expression in the subplan tlist, modify the parent tlist to copy the
+	 * value from the subplan output rather than re-evaluating it.
+	 */
+	prnt_tlist = tlist;			/* scans parent tlist in sync with sl */
+	foreach(sl, sub_tlist)
+	{
+		TargetEntry *te = (TargetEntry *) lfirst(sl);
+		TargetEntry *parentte = (TargetEntry *) lfirst(prnt_tlist);
+		Resdom	   *resdom = te->resdom;
+		bool		keepInSubPlan = true;
+		bool		foundGroupClause = false;
+		int			keyno = 0;
+
+		foreach(gl, parse->groupClause)
+		{
+			GroupClause *grpcl = (GroupClause *) lfirst(gl);
+
+			keyno++;			/* sort key # for this GroupClause */
+			/* Is it safe to use just resno to match tlist and glist items?? */
+			if (grpcl->entry->resdom->resno == resdom->resno)
+			{
+				/* Found a matching groupclause; record info for sorting */
+				foundGroupClause = true;
+				resdom->reskey = keyno;
+				resdom->reskeyop = get_opcode(grpcl->grpOpoid);
+				grpColIdx[keyno - 1] = next_resno;
+				/* Remove groupclause from our list of unmatched groupclauses.
+				 * NB: this depends on having used a shallow listCopy() above.
+				 */
+				glc = lremove((void*) grpcl, glc);
+				break;
+			}
+		}
+
+		if (! foundGroupClause)
+		{
+			/*
+			 * Non-GroupBy entry: remove it from subplan if there are
+			 * aggregates in query - it will be evaluated by Aggregate plan.
+			 * But do not remove simple-Var entries; we'd just have to add
+			 * them back anyway, and we risk confusing INSERT/UPDATE.
+			 */
+			if (parse->hasAggs && ! IsA(te->expr, Var))
+				keepInSubPlan = false;
+		}
+
+		if (keepInSubPlan)
+		{
+			/* Assign new sequential resnos to subplan tlist items */
+			resdom->resno = next_resno++;
+			if (! IsA(parentte->expr, Var))
+			{
+				/* Since the item is being computed in the subplan,
+				 * we can just make a Var node to reference it in the
+				 * outer plan, rather than recomputing it there.
+				 * Note we use varnoold = -1 as a flag to let
+				 * replace_vars_with_subplan_refs know it needn't change
+				 * this Var node.
+				 * If it's only a Var anyway, we leave it alone for now;
+				 * replace_vars_with_subplan_refs will fix it later.
+				 */
+				parentte->expr = (Node *) makeVar(1, resdom->resno,
+												  resdom->restype,
+												  resdom->restypmod,
+												  0, -1, resdom->resno);
+			}
+		}
+		else
+		{
+			/* Remove this tlist item from the subplan, but remember the
+			 * vars it needs.  The outer tlist item probably needs changes,
+			 * but that will happen later.
+			 */
+			sub_tlist = lremove(te, sub_tlist);
+			extravars = nconc(extravars, pull_var_clause(te->expr));
+		}
+
+		prnt_tlist = lnext(prnt_tlist);
+	}
+
+	/* We should have found all the GROUP BY clauses in the tlist. */
+	if (length(glc) != 0)
+		elog(ERROR, "make_subplanTargetList: GROUP BY attribute not found in target list");
+
+	/*
+	 * Add subplan targets for any variables needed by removed tlist entries
+	 * that aren't otherwise mentioned in the subplan target list.
+	 * We'll also need targets for any variables seen only in HAVING.
+	 */
+	extravars = nconc(extravars, pull_var_clause(parse->havingQual));
+
+	foreach(gl, extravars)
+	{
+		Var		   *v = (Var *) lfirst(gl);
+
+		if (tlist_member(v, sub_tlist) == NULL)
+		{
+			sub_tlist = lappend(sub_tlist,
+								create_tl_element(v, next_resno));
+			next_resno++;
+		}
+	}
+
+	return sub_tlist;
+}
+
+static Plan *
+make_groupplan(List *group_tlist,
+			   bool tuplePerGroup,
+			   List *groupClause,
+			   AttrNumber *grpColIdx,
+			   Plan *subplan)
+{
+	List	   *sort_tlist;
+	List	   *sl;
+	Sort	   *sortplan;
+	Group	   *grpplan;
+	int			numCols = length(groupClause);
+
+	/*
+	 * Make the targetlist for the Sort node; it always just references
+	 * each of the corresponding target items of the subplan.  We need to
+	 * ensure that simple Vars in the subplan's target list are recognizable
+	 * by replace_vars_with_subplan_refs when it's applied to the Sort/Group
+	 * target list, so copy up their varnoold/varoattno.
+	 */
+	sort_tlist = NIL;
+	foreach(sl, subplan->targetlist)
+	{
+		TargetEntry *te = (TargetEntry *) lfirst(sl);
+		Resdom	   *resdom = te->resdom;
+		Var		   *newvar;
+
+		if (IsA(te->expr, Var))
+		{
+			Var	   *subvar = (Var *) te->expr;
+			newvar = makeVar(1, resdom->resno,
+							 resdom->restype, resdom->restypmod,
+							 0, subvar->varnoold, subvar->varoattno);
+		}
+		else
+		{
+			newvar = makeVar(1, resdom->resno,
+							 resdom->restype, resdom->restypmod,
+							 0, -1, resdom->resno);
+		}
+
+		sort_tlist = lappend(sort_tlist,
+							 makeTargetEntry((Resdom *) copyObject(resdom),
+											 (Node *) newvar));
+	}
+
+	/*
+	 * Make the Sort node
+	 */
+	sortplan = make_sort(sort_tlist,
+						 _NONAME_RELATION_ID_,
+						 subplan,
+						 numCols);
+	sortplan->plan.cost = subplan->cost;		/* XXX assume no cost */
+
+	/*
+	 * If the caller gave us a target list, use it after fixing the variables.
+	 * If not, we need the same sort of "repeater" tlist as for the Sort node.
+	 */
+	if (group_tlist)
+	{
+		group_tlist = copyObject(group_tlist); /* necessary?? */
+		replace_tlist_with_subplan_refs(group_tlist,
+										(Index) 0,
+										subplan->targetlist);
+	}
+	else
+	{
+		group_tlist = copyObject(sort_tlist);
+	}
+
+	/*
+	 * Make the Group node
+	 */
+	grpplan = make_group(group_tlist, tuplePerGroup, numCols,
+						 grpColIdx, sortplan);
+
+	return (Plan *) grpplan;
+}
+
 /*
  * make_sortplan
  *	  Returns a sortplan which is basically a SORT node attached to the
@@ -384,6 +662,8 @@ make_sortplan(List *tlist, List *sortcls, Plan *plannode)
  * the final query in the function.  We do some ad-hoc define-time
  * type checking here to be sure that the user is returning the
  * type he claims.
+ *
+ * XXX Why is this function in this module?
  */
 void
 pg_checkretval(Oid rettype, QueryTreeList *queryTreeList)
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index b7b8d89c5a7c1813cba161413bbbc22a75be9e9b..4506097af9f598ac3f88afc95660831a11cfbe37 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/optimizer/plan/setrefs.c,v 1.43 1999/04/29 00:20:27 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/optimizer/plan/setrefs.c,v 1.44 1999/05/03 00:38:43 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -42,9 +42,8 @@ static List *replace_subclause_joinvar_refs(List *clauses,
 							   List *outer_tlist, List *inner_tlist);
 static Var *replace_joinvar_refs(Var *var, List *outer_tlist, List *inner_tlist);
 static List *tlist_noname_references(Oid nonameid, List *tlist);
-static void replace_result_clause(Node *clause, List *subplanTargetList);
 static bool OperandIsInner(Node *opnd, int inner_relid);
-static List *replace_agg_clause(Node *expr, List *targetlist);
+static List *pull_agg_clause(Node *clause);
 static Node *del_agg_clause(Node *clause);
 static void set_result_tlist_references(Result *resultNode);
 
@@ -542,9 +541,6 @@ set_result_tlist_references(Result *resultNode)
 	Plan	   *subplan;
 	List	   *resultTargetList;
 	List	   *subplanTargetList;
-	List	   *t;
-	TargetEntry *entry;
-	Expr	   *expr;
 
 	resultTargetList = ((Plan *) resultNode)->targetlist;
 
@@ -558,33 +554,51 @@ set_result_tlist_references(Result *resultNode)
 	else
 		subplanTargetList = NIL;
 
-	/*
-	 * now for traverse all the entris of the target list. These should be
-	 * of the form (Resdom_Node Expression). For every expression clause,
-	 * call "replace_result_clause()" to appropriatelly change all the Var
-	 * nodes.
-	 */
-	foreach(t, resultTargetList)
+	replace_tlist_with_subplan_refs(resultTargetList,
+									(Index) OUTER,
+									subplanTargetList);
+}
+
+/*---------------------------------------------------------
+ *
+ * replace_tlist_with_subplan_refs
+ *
+ * Applies replace_vars_with_subplan_refs() to each entry of a targetlist.
+ */
+void
+replace_tlist_with_subplan_refs(List *tlist,
+								Index subvarno,
+								List *subplanTargetList)
+{
+	List   *t;
+
+	foreach(t, tlist)
 	{
-		entry = (TargetEntry *) lfirst(t);
-		expr = (Expr *) get_expr(entry);
-		replace_result_clause((Node *) expr, subplanTargetList);
+		TargetEntry *entry = (TargetEntry *) lfirst(t);
+		replace_vars_with_subplan_refs((Node *) get_expr(entry),
+									   subvarno, subplanTargetList);
 	}
 }
 
 /*---------------------------------------------------------
  *
- * replace_result_clause
+ * replace_vars_with_subplan_refs
+ *
+ * This routine modifies (destructively!) an expression tree so that all
+ * Var nodes reference target nodes of a subplan.  It is used to fix up
+ * target expressions of upper-level plan nodes.
  *
- * This routine is called from set_result_tlist_references().
- * and modifies the expressions of the target list of a Result
- * node so that all Var nodes reference the target list of its subplan.
+ * 'clause': the tree to be fixed
+ * 'subvarno': varno to be assigned to all Vars
+ * 'subplanTargetList': target list for subplan
  *
+ * Afterwards, all Var nodes have varno = subvarno, varattno = resno
+ * of corresponding subplan target.
  */
-static void
-replace_result_clause(Node *clause,
-					  List *subplanTargetList)	/* target list of the
-												 * subplan */
+void
+replace_vars_with_subplan_refs(Node *clause,
+							   Index subvarno,
+							   List *subplanTargetList)
 {
 	List	   *t;
 
@@ -592,82 +606,100 @@ replace_result_clause(Node *clause,
 		return;
 	if (IsA(clause, Var))
 	{
-		TargetEntry *subplanVar;
-
 		/*
 		 * Ha! A Var node!
+		 *
+		 * It could be that this varnode has been created by make_groupplan
+		 * and is already set up to reference the subplan target list.
+		 * We recognize that case by varno = 1, varnoold = -1,
+		 * varattno = varoattno, and varlevelsup = 0.  (Probably ought to
+		 * have an explicit flag, but this should do for now.)
 		 */
-		subplanVar = match_varid((Var *) clause, subplanTargetList);
+		Var			*var = (Var *) clause;
+		TargetEntry *subplanVar;
+
+		if (var->varno == (Index) 1 && 
+			var->varnoold == ((Index) -1) &&
+			var->varattno == var->varoattno &&
+			var->varlevelsup == 0)
+			return;				/* OK to leave it alone */
 
+		/* Otherwise it had better be in the subplan list. */
+		subplanVar = match_varid(var, subplanTargetList);
 		if (! subplanVar)
-			elog(ERROR, "replace_result_clause: variable not in target list");
+			elog(ERROR, "replace_vars_with_subplan_refs: variable not in target list");
 
 		/*
 		 * Change the varno & varattno fields of the var node.
 		 */
-		((Var *) clause)->varno = (Index) OUTER;
-		((Var *) clause)->varattno = subplanVar->resdom->resno;
+		var->varno = subvarno;
+		var->varattno = subplanVar->resdom->resno;
 	}
-	else if (IsA(clause, Aggref))
-		replace_result_clause(((Aggref *) clause)->target, subplanTargetList);
-	else if (is_funcclause(clause))
+	else if (single_node(clause))
+	{
+		/* do nothing! */
+	}
+	else if (IsA(clause, Iter))
+		replace_vars_with_subplan_refs(((Iter *) clause)->iterexpr,
+									   subvarno, subplanTargetList);
+	else if (is_subplan(clause))
+	{
+		foreach(t, ((Expr *) clause)->args)
+			replace_vars_with_subplan_refs(lfirst(t),
+										   subvarno, subplanTargetList);
+		foreach(t, ((SubPlan *) ((Expr *) clause)->oper)->sublink->oper)
+			replace_vars_with_subplan_refs(lfirst(((Expr *) lfirst(t))->args),
+										   subvarno, subplanTargetList);
+	}
+	else if (IsA(clause, Expr))
 	{
-		List	   *subExpr;
-
 		/*
-		 * This is a function. Recursively call this routine for its
-		 * arguments...
+		 * Recursively scan the arguments of an expression.
+		 * NOTE: this must come after is_subplan() case since
+		 * subplan is a kind of Expr node.
 		 */
-		subExpr = ((Expr *) clause)->args;
-		foreach(t, subExpr)
-			replace_result_clause(lfirst(t), subplanTargetList);
+		foreach(t, ((Expr *) clause)->args)
+			replace_vars_with_subplan_refs(lfirst(t),
+										   subvarno, subplanTargetList);
 	}
+	else if (IsA(clause, Aggref))
+		replace_vars_with_subplan_refs(((Aggref *) clause)->target,
+									   subvarno, subplanTargetList);
 	else if (IsA(clause, ArrayRef))
 	{
 		ArrayRef   *aref = (ArrayRef *) clause;
-
-		/*
-		 * This is an arrayref. Recursively call this routine for its
-		 * expression and its index expression...
-		 */
 		foreach(t, aref->refupperindexpr)
-			replace_result_clause(lfirst(t), subplanTargetList);
+			replace_vars_with_subplan_refs(lfirst(t),
+										   subvarno, subplanTargetList);
 		foreach(t, aref->reflowerindexpr)
-			replace_result_clause(lfirst(t), subplanTargetList);
-		replace_result_clause(aref->refexpr,
-							  subplanTargetList);
-		replace_result_clause(aref->refassgnexpr,
-							  subplanTargetList);
+			replace_vars_with_subplan_refs(lfirst(t),
+										   subvarno, subplanTargetList);
+		replace_vars_with_subplan_refs(aref->refexpr,
+									   subvarno, subplanTargetList);
+		replace_vars_with_subplan_refs(aref->refassgnexpr,
+									   subvarno, subplanTargetList);
 	}
-	else if (is_opclause(clause))
+	else if (case_clause(clause))
 	{
-		Node	   *subNode;
-
-		/*
-		 * This is an operator. Recursively call this routine for both its
-		 * left and right operands
-		 */
-		subNode = (Node *) get_leftop((Expr *) clause);
-		replace_result_clause(subNode, subplanTargetList);
-		subNode = (Node *) get_rightop((Expr *) clause);
-		replace_result_clause(subNode, subplanTargetList);
-	}
-	else if (IsA(clause, Param) ||IsA(clause, Const))
-	{
-		/* do nothing! */
+		foreach(t, ((CaseExpr *) clause)->args)
+		{
+			CaseWhen   *when = (CaseWhen *) lfirst(t);
+			replace_vars_with_subplan_refs(when->expr,
+										   subvarno, subplanTargetList);
+			replace_vars_with_subplan_refs(when->result,
+										   subvarno, subplanTargetList);
+		}
+		replace_vars_with_subplan_refs(((CaseExpr *) clause)->defresult,
+									   subvarno, subplanTargetList);
 	}
 	else
 	{
-
-		/*
-		 * Ooops! we can not handle that!
-		 */
-		elog(ERROR, "replace_result_clause: Can not handle this tlist!\n");
+		elog(ERROR, "replace_vars_with_subplan_refs: Cannot handle node type %d",
+			 nodeTag(clause));
 	}
 }
 
-static
-bool
+static bool
 OperandIsInner(Node *opnd, int inner_relid)
 {
 
@@ -723,15 +755,22 @@ set_agg_tlist_references(Agg *aggNode)
 	{
 		TargetEntry *tle = lfirst(tl);
 
-		aggNode->aggs = nconc(replace_agg_clause(tle->expr, subplanTargetList),
-							  aggNode->aggs);
+		replace_vars_with_subplan_refs(tle->expr,
+									   (Index) 0,
+									   subplanTargetList);
+		aggNode->aggs = nconc(pull_agg_clause(tle->expr), aggNode->aggs);
 	}
 
 	all_quals_ok = true;
 	foreach(ql, aggNode->plan.qual)
 	{
 		Node *qual = lfirst(ql);
-		List *qualaggs = replace_agg_clause(qual, subplanTargetList);
+		List *qualaggs;
+
+		replace_vars_with_subplan_refs(qual,
+									   (Index) 0,
+									   subplanTargetList);
+		qualaggs = pull_agg_clause(qual);
 		if (qualaggs == NIL)
 			all_quals_ok = false; /* this qual clause has no agg functions! */
 		else
@@ -741,34 +780,21 @@ set_agg_tlist_references(Agg *aggNode)
 	return all_quals_ok;
 }
 
+/*
+ * Make a list of all Aggref nodes contained in the given expression.
+ */
 static List *
-replace_agg_clause(Node *clause, List *subplanTargetList)
+pull_agg_clause(Node *clause)
 {
-	List	   *t;
 	List	   *agg_list = NIL;
+	List	   *t;
 
 	if (clause == NULL)
 		return NIL;
-
-	if (IsA(clause, Var))
-	{
-		TargetEntry *subplanVar;
-
-		/*
-		 * Ha! A Var node!
-		 */
-		subplanVar = match_varid((Var *) clause, subplanTargetList);
-
-		if (! subplanVar)
-			elog(ERROR, "replace_agg_clause: variable not in target list");
-
-		/*
-		 * Change the varno & varattno fields of the var node.
-		 */
-		((Var *) clause)->varattno = subplanVar->resdom->resno;
-
+	else if (single_node(clause))
 		return NIL;
-	}
+	else if (IsA(clause, Iter))
+		return pull_agg_clause(((Iter *) clause)->iterexpr);
 	else if (is_subplan(clause))
 	{
 		SubLink *sublink = ((SubPlan *) ((Expr *) clause)->oper)->sublink;
@@ -778,13 +804,10 @@ replace_agg_clause(Node *clause, List *subplanTargetList)
 		 * aggregates to be attached to the aggs list
 		 */
 		foreach(t, sublink->lefthand)
-			agg_list = nconc(replace_agg_clause(lfirst(t), subplanTargetList),
-							 agg_list);
+			agg_list = nconc(pull_agg_clause(lfirst(t)), agg_list);
 		/* The first argument of ...->oper has also to be checked */
 		foreach(t, sublink->oper)
-			agg_list = nconc(replace_agg_clause(lfirst(t), subplanTargetList),
-							 agg_list);
-		return agg_list;
+			agg_list = nconc(pull_agg_clause(lfirst(t)), agg_list);
 	}
 	else if (IsA(clause, Expr))
 	{
@@ -794,53 +817,41 @@ replace_agg_clause(Node *clause, List *subplanTargetList)
 		 * subplan is a kind of Expr node.
 		 */
 		foreach(t, ((Expr *) clause)->args)
-		{
-			agg_list = nconc(replace_agg_clause(lfirst(t), subplanTargetList),
-							 agg_list);
-		}
-		return agg_list;
+			agg_list = nconc(pull_agg_clause(lfirst(t)), agg_list);
 	}
 	else if (IsA(clause, Aggref))
 	{
 		return lcons(clause,
-					 replace_agg_clause(((Aggref *) clause)->target,
-										subplanTargetList));
+					 pull_agg_clause(((Aggref *) clause)->target));
 	}
 	else if (IsA(clause, ArrayRef))
 	{
 		ArrayRef   *aref = (ArrayRef *) clause;
-
-		/*
-		 * This is an arrayref. Recursively call this routine for its
-		 * expression and its index expression...
-		 */
 		foreach(t, aref->refupperindexpr)
-			agg_list = nconc(replace_agg_clause(lfirst(t), subplanTargetList),
-							 agg_list);
+			agg_list = nconc(pull_agg_clause(lfirst(t)), agg_list);
 		foreach(t, aref->reflowerindexpr)
-			agg_list = nconc(replace_agg_clause(lfirst(t), subplanTargetList),
-							 agg_list);
-		agg_list = nconc(replace_agg_clause(aref->refexpr, subplanTargetList),
-						 agg_list);
-		agg_list = nconc(replace_agg_clause(aref->refassgnexpr,
-											subplanTargetList),
-						 agg_list);
-		return agg_list;
+			agg_list = nconc(pull_agg_clause(lfirst(t)), agg_list);
+		agg_list = nconc(pull_agg_clause(aref->refexpr), agg_list);
+		agg_list = nconc(pull_agg_clause(aref->refassgnexpr), agg_list);
 	}
-	else if (IsA(clause, Param) || IsA(clause, Const))
+	else if (case_clause(clause))
 	{
-		/* do nothing! */
-		return NIL;
+		foreach(t, ((CaseExpr *) clause)->args)
+		{
+			CaseWhen   *when = (CaseWhen *) lfirst(t);
+			agg_list = nconc(agg_list, pull_agg_clause(when->expr));
+			agg_list = nconc(agg_list, pull_agg_clause(when->result));
+		}
+		agg_list = nconc(pull_agg_clause(((CaseExpr *) clause)->defresult),
+						 agg_list);
 	}
 	else
 	{
-		/*
-		 * Ooops! we can not handle that!
-		 */
-		elog(ERROR, "replace_agg_clause: Cannot handle node type %d",
+		elog(ERROR, "pull_agg_clause: Cannot handle node type %d",
 			 nodeTag(clause));
-		return NIL;
 	}
+
+	return agg_list;
 }
 
 
@@ -932,99 +943,6 @@ del_agg_clause(Node *clause)
 	return NULL;
 }
 
-/*
- * check_having_qual_for_vars takes the havingQual and the actual targetlist
- * as arguments and recursively scans the havingQual for attributes that are
- * not included in the targetlist yet.  This will occur with queries like:
- *
- * SELECT sid FROM part GROUP BY sid HAVING MIN(pid) > 1;
- *
- * To be able to handle queries like that correctly we have to extend the
- * actual targetlist (which will be the one used for the GROUP node later on)
- * by these attributes.  The return value is the extended targetlist.
- */
-List *
-check_having_qual_for_vars(Node *clause, List *targetlist_so_far)
-{
-	List	   *t;
-
-	if (clause == NULL)
-		return targetlist_so_far;
-
-	if (IsA(clause, Var))
-	{
-		RelOptInfo	tmp_rel;
-
-		/*
-		 * Ha! A Var node!
-		 */
-
-		tmp_rel.targetlist = targetlist_so_far;
-
-		/* Check if the VAR is already contained in the targetlist */
-		if (tlist_member((Var *) clause, (List *) targetlist_so_far) == NULL)
-			add_var_to_tlist(&tmp_rel, (Var *) clause);
-
-		return tmp_rel.targetlist;
-	}
-	else if (IsA(clause, Expr) && ! is_subplan(clause))
-	{
-		/*
-		 * Recursively scan the arguments of an expression.
-		 */
-		foreach(t, ((Expr *) clause)->args)
-			targetlist_so_far = check_having_qual_for_vars(lfirst(t), targetlist_so_far);
-		return targetlist_so_far;
-	}
-	else if (IsA(clause, Aggref))
-	{
-		targetlist_so_far = check_having_qual_for_vars(((Aggref *) clause)->target, targetlist_so_far);
-		return targetlist_so_far;
-	}
-	else if (IsA(clause, ArrayRef))
-	{
-		ArrayRef   *aref = (ArrayRef *) clause;
-
-		/*
-		 * This is an arrayref. Recursively call this routine for its
-		 * expression and its index expression...
-		 */
-		foreach(t, aref->refupperindexpr)
-			targetlist_so_far = check_having_qual_for_vars(lfirst(t), targetlist_so_far);
-		foreach(t, aref->reflowerindexpr)
-			targetlist_so_far = check_having_qual_for_vars(lfirst(t), targetlist_so_far);
-		targetlist_so_far = check_having_qual_for_vars(aref->refexpr, targetlist_so_far);
-		targetlist_so_far = check_having_qual_for_vars(aref->refassgnexpr, targetlist_so_far);
-
-		return targetlist_so_far;
-	}
-	else if (IsA(clause, Param) || IsA(clause, Const))
-	{
-		/* do nothing! */
-		return targetlist_so_far;
-	}
-	/*
-	 * If we get to a sublink, then we only have to check the lefthand
-	 * side of the expression to see if there are any additional VARs.
-	 * QUESTION: can this code actually be hit?
-	 */
-	else if (IsA(clause, SubLink))
-	{
-		foreach(t, ((SubLink *) clause)->lefthand)
-			targetlist_so_far = check_having_qual_for_vars(lfirst(t), targetlist_so_far);
-		return targetlist_so_far;
-	}
-	else
-	{
-		/*
-		 * Ooops! we can not handle that!
-		 */
-		elog(ERROR, "check_having_qual_for_vars: Cannot handle node type %d",
-			 nodeTag(clause));
-		return NIL;
-	}
-}
-
 /*
  * check_having_for_ungrouped_vars takes the havingQual and the list of
  * GROUP BY clauses and checks for subplans in the havingQual that are being
@@ -1053,6 +971,15 @@ check_having_for_ungrouped_vars(Node *clause, List *groupClause)
 		 * parser already checked 'em.
 		 */
 	}
+	else if (single_node(clause))
+	{
+		/* ignore */
+	}
+	else if (IsA(clause, Iter))
+	{
+		check_having_for_ungrouped_vars(((Iter *) clause)->iterexpr,
+										groupClause);
+	}
 	else if (is_subplan(clause))
 	{
 		/*
@@ -1098,8 +1025,8 @@ check_having_for_ungrouped_vars(Node *clause, List *groupClause)
 	}
 	else if (IsA(clause, Aggref))
 	{
-			check_having_for_ungrouped_vars(((Aggref *) clause)->target,
-											groupClause);
+		check_having_for_ungrouped_vars(((Aggref *) clause)->target,
+										groupClause);
 	}
 	else if (IsA(clause, ArrayRef))
 	{
@@ -1116,15 +1043,19 @@ check_having_for_ungrouped_vars(Node *clause, List *groupClause)
 		check_having_for_ungrouped_vars(aref->refexpr, groupClause);
 		check_having_for_ungrouped_vars(aref->refassgnexpr, groupClause);
 	}
-	else if (IsA(clause, Param) || IsA(clause, Const))
+	else if (case_clause(clause))
 	{
-		/* do nothing! */
+		foreach(t, ((CaseExpr *) clause)->args)
+		{
+			CaseWhen   *when = (CaseWhen *) lfirst(t);
+			check_having_for_ungrouped_vars(when->expr, groupClause);
+			check_having_for_ungrouped_vars(when->result, groupClause);
+		}
+		check_having_for_ungrouped_vars(((CaseExpr *) clause)->defresult,
+										groupClause);
 	}
 	else
 	{
-		/*
-		 * Ooops! we can not handle that!
-		 */
 		elog(ERROR, "check_having_for_ungrouped_vars: Cannot handle node type %d",
 			 nodeTag(clause));
 	}
diff --git a/src/backend/optimizer/util/var.c b/src/backend/optimizer/util/var.c
index 914f33f50ea4f5f10aaf3a0a47e762fd61ab2569..b906f8e4c4823bf7959eccc5355d7e0f0ae8cd3c 100644
--- a/src/backend/optimizer/util/var.c
+++ b/src/backend/optimizer/util/var.c
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/optimizer/util/var.c,v 1.17 1999/02/22 05:26:27 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/optimizer/util/var.c,v 1.18 1999/05/03 00:38:44 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -68,54 +68,56 @@ pull_varnos(Node *me)
 
 /*
  * contain_var_clause
- *	  Recursively find var nodes from a clause by pulling vars from the
- *	  left and right operands of the clause.
+ *	  Recursively scan a clause to discover whether it contains any Var nodes.
  *
  *	  Returns true if any varnode found.
  */
 bool
 contain_var_clause(Node *clause)
 {
+	List	   *temp;
+
 	if (clause == NULL)
 		return FALSE;
 	else if (IsA(clause, Var))
 		return TRUE;
-	else if (IsA(clause, Iter))
-		return contain_var_clause(((Iter *) clause)->iterexpr);
 	else if (single_node(clause))
 		return FALSE;
-	else if (or_clause(clause) || and_clause(clause) || is_funcclause(clause))
+	else if (IsA(clause, Iter))
+		return contain_var_clause(((Iter *) clause)->iterexpr);
+	else if (is_subplan(clause))
 	{
-		List	   *temp;
-
 		foreach(temp, ((Expr *) clause)->args)
 		{
 			if (contain_var_clause(lfirst(temp)))
 				return TRUE;
 		}
+		/* Also check left sides of Oper-s */
+		foreach(temp, ((SubPlan *) ((Expr *) clause)->oper)->sublink->oper)
+		{
+			if (contain_var_clause(lfirst(((Expr *) lfirst(temp))->args)))
+				return TRUE;
+		}
 		return FALSE;
 	}
-	else if (is_subplan(clause))
+	else if (IsA(clause, Expr))
 	{
-		List	   *temp;
-
+		/*
+		 * Recursively scan the arguments of an expression.
+		 * NOTE: this must come after is_subplan() case since
+		 * subplan is a kind of Expr node.
+		 */
 		foreach(temp, ((Expr *) clause)->args)
 		{
 			if (contain_var_clause(lfirst(temp)))
 				return TRUE;
 		}
-		/* Ok - check left sides of Oper-s */
-		foreach(temp, ((SubPlan *) ((Expr *) clause)->oper)->sublink->oper)
-		{
-			if (contain_var_clause(lfirst(((Expr *) lfirst(temp))->args)))
-				return TRUE;
-		}
 		return FALSE;
 	}
+	else if (IsA(clause, Aggref))
+		return contain_var_clause(((Aggref *) clause)->target);
 	else if (IsA(clause, ArrayRef))
 	{
-		List	   *temp;
-
 		foreach(temp, ((ArrayRef *) clause)->refupperindexpr)
 		{
 			if (contain_var_clause(lfirst(temp)))
@@ -132,19 +134,11 @@ contain_var_clause(Node *clause)
 			return TRUE;
 		return FALSE;
 	}
-	else if (not_clause(clause))
-		return contain_var_clause((Node *) get_notclausearg((Expr *) clause));
-	else if (is_opclause(clause))
-		return (contain_var_clause((Node *) get_leftop((Expr *) clause)) ||
-			  contain_var_clause((Node *) get_rightop((Expr *) clause)));
 	else if (case_clause(clause))
 	{
-		List	   *args;
-		CaseWhen   *when;
-
-		foreach(args, ((CaseExpr *) clause)->args)
+		foreach(temp, ((CaseExpr *) clause)->args)
 		{
-			when = lfirst(args);
+			CaseWhen   *when = (CaseWhen *) lfirst(temp);
 			if (contain_var_clause(when->expr))
 				return TRUE;
 			if (contain_var_clause(when->result))
@@ -152,6 +146,11 @@ contain_var_clause(Node *clause)
 		}
 		return (contain_var_clause(((CaseExpr *) clause)->defresult));
 	}
+	else
+	{
+		elog(ERROR, "contain_var_clause: Cannot handle node type %d",
+			 nodeTag(clause));
+	}
 
 	return FALSE;
 }
@@ -161,45 +160,46 @@ contain_var_clause(Node *clause)
  *	  Recursively pulls all var nodes from a clause by pulling vars from the
  *	  left and right operands of the clause.
  *
- *	  Returns list of varnodes found.
+ *	  Returns list of varnodes found.  Note the varnodes themselves are not
+ *	  copied, only referenced.
  */
 List *
 pull_var_clause(Node *clause)
 {
 	List	   *retval = NIL;
+	List	   *temp;
 
 	if (clause == NULL)
 		return NIL;
 	else if (IsA(clause, Var))
 		retval = lcons(clause, NIL);
-	else if (IsA(clause, Iter))
-		retval = pull_var_clause(((Iter *) clause)->iterexpr);
 	else if (single_node(clause))
 		retval = NIL;
-	else if (or_clause(clause) || and_clause(clause) || is_funcclause(clause))
-	{
-		List	   *temp;
-
-		foreach(temp, ((Expr *) clause)->args)
-			retval = nconc(retval, pull_var_clause(lfirst(temp)));
-	}
+	else if (IsA(clause, Iter))
+		retval = pull_var_clause(((Iter *) clause)->iterexpr);
 	else if (is_subplan(clause))
 	{
-		List	   *temp;
-
 		foreach(temp, ((Expr *) clause)->args)
 			retval = nconc(retval, pull_var_clause(lfirst(temp)));
-		/* Ok - get Var-s from left sides of Oper-s */
+		/* Also get Var-s from left sides of Oper-s */
 		foreach(temp, ((SubPlan *) ((Expr *) clause)->oper)->sublink->oper)
 			retval = nconc(retval,
 				 pull_var_clause(lfirst(((Expr *) lfirst(temp))->args)));
 	}
+	else if (IsA(clause, Expr))
+	{
+		/*
+		 * Recursively scan the arguments of an expression.
+		 * NOTE: this must come after is_subplan() case since
+		 * subplan is a kind of Expr node.
+		 */
+		foreach(temp, ((Expr *) clause)->args)
+			retval = nconc(retval, pull_var_clause(lfirst(temp)));
+	}
 	else if (IsA(clause, Aggref))
 		retval = pull_var_clause(((Aggref *) clause)->target);
 	else if (IsA(clause, ArrayRef))
 	{
-		List	   *temp;
-
 		foreach(temp, ((ArrayRef *) clause)->refupperindexpr)
 			retval = nconc(retval, pull_var_clause(lfirst(temp)));
 		foreach(temp, ((ArrayRef *) clause)->reflowerindexpr)
@@ -209,25 +209,21 @@ pull_var_clause(Node *clause)
 		retval = nconc(retval,
 				   pull_var_clause(((ArrayRef *) clause)->refassgnexpr));
 	}
-	else if (not_clause(clause))
-		retval = pull_var_clause((Node *) get_notclausearg((Expr *) clause));
-	else if (is_opclause(clause))
-		retval = nconc(pull_var_clause((Node *) get_leftop((Expr *) clause)),
-				 pull_var_clause((Node *) get_rightop((Expr *) clause)));
 	else if (case_clause(clause))
 	{
-		List	   *temp;
-
 		foreach(temp, ((CaseExpr *) clause)->args)
 		{
-			retval = nconc(retval, pull_var_clause(((CaseWhen *) lfirst(temp))->expr));
-			retval = nconc(retval, pull_var_clause(((CaseWhen *) lfirst(temp))->result));
+			CaseWhen   *when = (CaseWhen *) lfirst(temp);
+			retval = nconc(retval, pull_var_clause(when->expr));
+			retval = nconc(retval, pull_var_clause(when->result));
 		}
-
 		retval = nconc(retval, pull_var_clause(((CaseExpr *) clause)->defresult));
 	}
 	else
-		retval = NIL;
+	{
+		elog(ERROR, "pull_var_clause: Cannot handle node type %d",
+			 nodeTag(clause));
+	}
 
 	return retval;
 }
diff --git a/src/include/optimizer/planmain.h b/src/include/optimizer/planmain.h
index 44dad8cfd66daaa28167cc9026edc01a1542ab76..002238571c0a3baaeff9d96257960f5cb6d7ce33 100644
--- a/src/include/optimizer/planmain.h
+++ b/src/include/optimizer/planmain.h
@@ -6,7 +6,7 @@
  *
  * Copyright (c) 1994, Regents of the University of California
  *
- * $Id: planmain.h,v 1.23 1999/04/19 01:43:10 tgl Exp $
+ * $Id: planmain.h,v 1.24 1999/05/03 00:38:42 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -54,9 +54,14 @@ extern List *join_references(List *clauses, List *outer_tlist,
 							 List *inner_tlist);
 extern List *index_outerjoin_references(List *inner_indxqual,
 						   List *outer_tlist, Index inner_relid);
+extern void replace_tlist_with_subplan_refs(List *tlist,
+											Index subvarno,
+											List *subplanTargetList);
+extern void replace_vars_with_subplan_refs(Node *clause,
+										   Index subvarno,
+										   List *subplanTargetList);
 extern bool set_agg_tlist_references(Agg *aggNode);
 extern void del_agg_tlist_references(List *tlist);
-extern List *check_having_qual_for_vars(Node *clause, List *targetlist_so_far);
 extern void check_having_for_ungrouped_vars(Node *clause, List *groupClause);
 extern void transformKeySetQuery(Query *origNode);