diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 426a953891f2af42d667dd878230333bf5aaa656..90b84d591a96579a875e6e0352f2f3dfb5ab2e36 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v 1.76 1999/03/03 00:02:42 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v 1.77 1999/04/19 01:43:11 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -470,7 +470,10 @@ _copyAgg(Agg *from)
 
 	CopyPlanFields((Plan *) from, (Plan *) newnode);
 
-	newnode->aggs = get_agg_tlist_references(newnode);
+	/* Cannot copy agg list; it must be rebuilt to point to subnodes of
+	 * new node.
+	 */ 
+	set_agg_tlist_references(newnode);
 
 	return newnode;
 }
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 27d5892ee0cca35ab2f920a8712015844688e850..543b5d6d7c6aaa8e971191e4704340b68f6f7e59 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.46 1999/03/19 18:56:37 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.47 1999/04/19 01:43:11 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -138,32 +138,14 @@ union_planner(Query *parse)
 	else
 	{
 	  List  **vpm = NULL;
-	  
-	  /***S*H***/
-	  /* This is only necessary if aggregates are in use in queries like:
-	   * SELECT sid 
-	   * FROM part
-	   * GROUP BY sid
-	   * HAVING MIN(pid) > 1;  (pid is used but never selected for!!!)
-	   * because the function 'query_planner' creates the plan for the lefttree
-	   * of the 'GROUP' node and returns only those attributes contained in 'tlist'.
-	   * The original 'tlist' contains only 'sid' here and that's why we have to
-	   * to extend it to attributes which are not selected but are used in the 
-	   * havingQual. */
-	  	  
-	  /* 'check_having_qual_for_vars' takes the havingQual and the actual 'tlist'
-	   * as arguments and recursively scans the havingQual for attributes 
-	   * (VAR nodes) that are not contained in 'tlist' yet. If so, it creates
-	   * a new entry and attaches it to the list 'new_tlist' (consisting of the 
-	   * VAR node and the RESDOM node as usual with tlists :-)  ) */
-	  if (parse->hasAggs)
-	  {
-	      if (parse->havingQual != NULL)
-		  {
-		  	new_tlist = check_having_qual_for_vars(parse->havingQual,new_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,
@@ -208,10 +190,10 @@ union_planner(Query *parse)
 					parse->rtable);
 	  
 	  if (parse->rtable != NULL)
-	    {
+	  {
 	      vpm = (List **) palloc(length(parse->rtable) * sizeof(List *));
 	      memset(vpm, 0, length(parse->rtable) * sizeof(List *));
-	    }
+	  }
 	  PlannerVarParam = lcons(vpm, PlannerVarParam);
 	  result_plan = query_planner(parse,
 				      parse->commandType,
@@ -246,89 +228,67 @@ union_planner(Query *parse)
 								   result_plan);
 	}
 
+	/*
+	 * If we have a HAVING clause, do the necessary things with it.
+	 */
+	if (parse->havingQual)
+	{
+		List  **vpm = NULL;
+
+		if (parse->rtable != NULL)
+		{
+			vpm = (List **) palloc(length(parse->rtable) * sizeof(List *));
+			memset(vpm, 0, length(parse->rtable) * sizeof(List *));
+		}
+		PlannerVarParam = lcons(vpm, PlannerVarParam);
+
+		/* convert the havingQual to conjunctive normal form (cnf) */
+		parse->havingQual = (Node *) cnfify((Expr *) parse->havingQual, true);
+
+		if (parse->hasSubLinks)
+		{
+			/* There is a subselect in the havingQual, so we have to process it
+			 * using the same function as for a subselect in 'where'
+			 */
+			parse->havingQual =
+				(Node *) SS_process_sublinks(parse->havingQual);
+			/* Check for ungrouped variables passed to subplans.
+			 * (Probably this should be done by the parser, but right now
+			 * the parser is not smart enough to tell which level the vars
+			 * belong to?)
+			 */
+			check_having_for_ungrouped_vars(parse->havingQual,
+											parse->groupClause);
+		}
+
+		/* Calculate the opfids from the opnos */
+		parse->havingQual = (Node *) fix_opids((List *) parse->havingQual);
+
+		PlannerVarParam = lnext(PlannerVarParam);
+		if (vpm != NULL)
+			pfree(vpm);		
+	}
+
 	/*
 	 * If aggregate is present, insert the agg node
 	 */
 	if (parse->hasAggs)
 	{
-	        int old_length=0, new_length=0;
-		
-		/* Create the Agg node but use 'tlist' not 'new_tlist' as target list because we
-		 * don't want the additional attributes (only used for the havingQual, see above)
-		 * to show up in the result */
+		/* 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 */
+		result_plan->qual = (List *) parse->havingQual;
+
 		/*
-		 * get the varno/attno entries to the appropriate references to
-		 * the result tuple of the subplans.
+		 * Update vars to refer to subplan result tuples,
+		 * find Aggrefs, make sure there is an Aggref in every HAVING clause.
 		 */
-		((Agg *) result_plan)->aggs = get_agg_tlist_references((Agg *) result_plan); 
-
-		/***S*H***/
-		if(parse->havingQual!=NULL) 
-		  {
-		    List	   *clause;
-		    List	  **vpm = NULL;
-		    
-		    
-		    /* stuff copied from above to handle the use of attributes from outside
-		     * in subselects */
-
-		    if (parse->rtable != NULL)
-		      {
-			vpm = (List **) palloc(length(parse->rtable) * sizeof(List *));
-			memset(vpm, 0, length(parse->rtable) * sizeof(List *));
-		      }
-		    PlannerVarParam = lcons(vpm, PlannerVarParam);
-		    
-
-		    /* convert the havingQual to conjunctive normal form (cnf) */
-		    parse->havingQual = (Node *) cnfify((Expr *)(Node *) parse->havingQual,true);
-
-		    /* There is a subselect in the havingQual, so we have to process it
-                     * using the same function as for a subselect in 'where' */
-		    if (parse->hasSubLinks)
-		      {
-			parse->havingQual = 
-			  (Node *) SS_process_sublinks((Node *) parse->havingQual);
-		      }
-		    		    
-		    
-		    /* Calculate the opfids from the opnos (=select the correct functions for
-		     * the used VAR datatypes) */
-		    parse->havingQual = (Node *) fix_opids((List *) parse->havingQual);
-		    
-		    ((Agg *) result_plan)->plan.qual=(List *) parse->havingQual;
-
-		    /* Check every clause of the havingQual for aggregates used and append
-		     * them to result_plan->aggs
-			 */
-		    foreach(clause, ((Agg *) result_plan)->plan.qual)
-		      {
-			/* Make sure there are aggregates in the havingQual 
-			 * if so, the list must be longer after check_having_qual_for_aggs
-			 */
-			old_length=length(((Agg *) result_plan)->aggs);			
-			
-			((Agg *) result_plan)->aggs = nconc(((Agg *) result_plan)->aggs,
-			    check_having_qual_for_aggs((Node *) lfirst(clause),
-				       ((Agg *) result_plan)->plan.lefttree->targetlist,
-				       ((List *) parse->groupClause)));
-
-			/* Have a look at the length of the returned list. If there is no
-			 * difference, no aggregates have been found and that means, that
-			 * the Qual belongs to the where clause */
-			if (((new_length=length(((Agg *) result_plan)->aggs)) == old_length) ||
-			    (new_length == 0))
-			  {
-			    elog(ERROR,"This could have been done in a where clause!!");
-			    return (Plan *)NIL;
-			  }
-		      }
-		    PlannerVarParam = lnext(PlannerVarParam);
-		    if (vpm != NULL)
-		      pfree(vpm);		
-		  }
+		if (! set_agg_tlist_references((Agg *) result_plan))
+			elog(ERROR, "SELECT/HAVING requires aggregates to be valid");
 	}		  
 
 	/*
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index e3bfc87586a42dd6cbbc1c4d117c53a5d949582e..53ebd8c34ddb8dc3b4c652617fee4298d52437e8 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.40 1999/02/15 01:06:58 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/optimizer/plan/setrefs.c,v 1.41 1999/04/19 01:43:12 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -695,32 +695,48 @@ OperandIsInner(Node *opnd, int inner_relid)
 
 /*---------------------------------------------------------
  *
- * get_agg_tlist_references -
- *	  generates the target list of an Agg node so that it points to
+ * set_agg_tlist_references -
+ *	  This routine has several responsibilities:
+ *	* Update the target list of an Agg node so that it points to
  *	  the tuples returned by its left tree subplan.
+ *	* If there is a qual list (from a HAVING clause), similarly update
+ *	  vars in it to point to the subplan target list.
+ *	* Generate the aggNode->aggs list of Aggref nodes contained in the Agg.
  *
- *	We now also generate a linked list of Aggref pointers for Agg.
- *
+ * The return value is TRUE if all qual clauses include Aggrefs, or FALSE
+ * if any do not (caller may choose to raise an error condition).
  */
-List *
-get_agg_tlist_references(Agg *aggNode)
+bool
+set_agg_tlist_references(Agg *aggNode)
 {
-	List	   *aggTargetList;
 	List	   *subplanTargetList;
 	List	   *tl;
-	List	   *aggreg_list = NIL;
+	List	   *ql;
+	bool		all_quals_ok;
 
-	aggTargetList = aggNode->plan.targetlist;
 	subplanTargetList = aggNode->plan.lefttree->targetlist;
+	aggNode->aggs = NIL;
 
-	foreach(tl, aggTargetList)
+	foreach(tl, aggNode->plan.targetlist)
 	{
 		TargetEntry *tle = lfirst(tl);
 
-		aggreg_list = nconc(
-		  replace_agg_clause(tle->expr, subplanTargetList), aggreg_list);
+		aggNode->aggs = nconc(replace_agg_clause(tle->expr, subplanTargetList),
+							  aggNode->aggs);
 	}
-	return aggreg_list;
+
+	all_quals_ok = true;
+	foreach(ql, aggNode->plan.qual)
+	{
+		Node *qual = lfirst(ql);
+		List *qualaggs = replace_agg_clause(qual, subplanTargetList);
+		if (qualaggs == NIL)
+			all_quals_ok = false; /* this qual clause has no agg functions! */
+		else
+			aggNode->aggs = nconc(qualaggs, aggNode->aggs);
+	}
+
+	return all_quals_ok;
 }
 
 static List *
@@ -740,30 +756,49 @@ replace_agg_clause(Node *clause, List *subplanTargetList)
 
 		/*
 		 * Change the varno & varattno fields of the var node.
+		 * Note we assume match_varid() will succeed ...
 		 *
 		 */
 		((Var *) clause)->varattno = subplanVar->resdom->resno;
 
 		return NIL;
 	}
-	else if (is_funcclause(clause))
+	else if (is_subplan(clause))
 	{
+		SubLink *sublink = ((SubPlan *) ((Expr *) clause)->oper)->sublink;
 
 		/*
-		 * This is a function. Recursively call this routine for its
-		 * arguments...
+		 * Only the lefthand side of the sublink should be checked for
+		 * aggregates to be attached to the aggs list
+		 */
+		foreach(t, sublink->lefthand)
+			agg_list = nconc(replace_agg_clause(lfirst(t), subplanTargetList),
+							 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;
+	}
+	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(t, ((Expr *) clause)->args)
 		{
-			agg_list = nconc(agg_list,
-					   replace_agg_clause(lfirst(t), subplanTargetList));
+			agg_list = nconc(replace_agg_clause(lfirst(t), subplanTargetList),
+							 agg_list);
 		}
 		return agg_list;
 	}
 	else if (IsA(clause, Aggref))
 	{
 		return lcons(clause,
-					 replace_agg_clause(((Aggref *) clause)->target, subplanTargetList));
+					 replace_agg_clause(((Aggref *) clause)->target,
+										subplanTargetList));
 	}
 	else if (IsA(clause, ArrayRef))
 	{
@@ -774,53 +809,30 @@ replace_agg_clause(Node *clause, List *subplanTargetList)
 		 * expression and its index expression...
 		 */
 		foreach(t, aref->refupperindexpr)
-		{
-			agg_list = nconc(agg_list,
-					   replace_agg_clause(lfirst(t), subplanTargetList));
-		}
+			agg_list = nconc(replace_agg_clause(lfirst(t), subplanTargetList),
+							 agg_list);
 		foreach(t, aref->reflowerindexpr)
-		{
-			agg_list = nconc(agg_list,
-					   replace_agg_clause(lfirst(t), subplanTargetList));
-		}
-		agg_list = nconc(agg_list,
-				   replace_agg_clause(aref->refexpr, subplanTargetList));
-		agg_list = nconc(agg_list,
-			  replace_agg_clause(aref->refassgnexpr, subplanTargetList));
-
+			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;
 	}
-	else if (is_opclause(clause))
-	{
-
-		/*
-		 * This is an operator. Recursively call this routine for both its
-		 * left and right operands
-		 */
-		Node	   *left = (Node *) get_leftop((Expr *) clause);
-		Node	   *right = (Node *) get_rightop((Expr *) clause);
-
-		if (left != (Node *) NULL)
-			agg_list = nconc(agg_list,
-							 replace_agg_clause(left, subplanTargetList));
-		if (right != (Node *) NULL)
-			agg_list = nconc(agg_list,
-						   replace_agg_clause(right, subplanTargetList));
-
-		return agg_list;
-	}
-	else if (IsA(clause, Param) ||IsA(clause, Const))
+	else if (IsA(clause, Param) || IsA(clause, Const))
 	{
 		/* do nothing! */
 		return NIL;
 	}
 	else
 	{
-
 		/*
 		 * Ooops! we can not handle that!
 		 */
-		elog(ERROR, "replace_agg_clause: Can not handle this tlist!\n");
+		elog(ERROR, "replace_agg_clause: Cannot handle node type %d",
+			 nodeTag(clause));
 		return NIL;
 	}
 }
@@ -911,48 +923,42 @@ del_agg_clause(Node *clause)
 	return NULL;
 }
 
-/***S*H***/
-/* check_having_qual_for_vars takes the the havingQual and the actual targetlist as arguments
- * and recursively scans the havingQual for attributes that are not included in the targetlist
- * yet. Attributes contained in the havingQual but not in the targetlist show up with queries
- * like:
- * SELECT sid
- * FROM part
- * GROUP BY sid
- * HAVING MIN(pid) > 1;  (pid is used but never selected for!!!).
- * 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. */
+/*
+ * 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 (IsA(clause, Var))
 	{
 		RelOptInfo	tmp_rel;
 
-
-		tmp_rel.targetlist = targetlist_so_far;
-
 		/*
 		 * 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 (is_funcclause(clause) || not_clause(clause) ||
-			 or_clause(clause) || and_clause(clause))
+	else if (IsA(clause, Expr) && ! is_subplan(clause))
 	{
-
 		/*
-		 * This is a function. Recursively call this routine for its
-		 * arguments...
+		 * 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);
@@ -980,125 +986,105 @@ check_having_qual_for_vars(Node *clause, List *targetlist_so_far)
 
 		return targetlist_so_far;
 	}
-	else if (is_opclause(clause))
-	{
-
-		/*
-		 * This is an operator. Recursively call this routine for both its
-		 * left and right operands
-		 */
-		Node	   *left = (Node *) get_leftop((Expr *) clause);
-		Node	   *right = (Node *) get_rightop((Expr *) clause);
-
-		if (left != (Node *) NULL)
-			targetlist_so_far = check_having_qual_for_vars(left, targetlist_so_far);
-		if (right != (Node *) NULL)
-			targetlist_so_far = check_having_qual_for_vars(right, targetlist_so_far);
-
-		return targetlist_so_far;
-	}
-	else if (IsA(clause, Param) ||IsA(clause, Const))
+	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
+	 * 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, ((List *) ((SubLink *) clause)->lefthand))
+		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: Can not handle this having_qual! %d\n",
+		elog(ERROR, "check_having_qual_for_vars: Cannot handle node type %d",
 			 nodeTag(clause));
 		return NIL;
 	}
 }
 
-/* check_having_qual_for_aggs takes the havingQual, the targetlist and the groupClause
- * as arguments and scans the havingQual recursively for aggregates. If an aggregate is
- * found it is attached to a list and returned by the function. (All the returned lists
- * are concenated to result_plan->aggs in planner.c:union_planner() */
-List *
-check_having_qual_for_aggs(Node *clause, List *subplanTargetList, List *groupClause)
-{
-	List	   *t,
-			   *l1;
-	List	   *agg_list = NIL;
-
-	int			contained_in_group_clause = 0;
+/*
+ * 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
+ * passed ungrouped variables as parameters.  In other contexts, ungrouped
+ * vars in the havingQual will be detected by the parser (see parse_agg.c,
+ * exprIsAggOrGroupCol()).  But that routine currently does not check subplans,
+ * because the necessary info is not computed until the planner runs.
+ * This ought to be cleaned up someday.
+ *
+ * NOTE: the havingClause has been cnf-ified, so AND subclauses have been
+ * turned into a plain List.  Thus, this routine has to cope with List nodes
+ * where the routine above does not...
+ */
 
+void
+check_having_for_ungrouped_vars(Node *clause, List *groupClause)
+{
+	List	   *t;
 
 	if (IsA(clause, Var))
 	{
-		TargetEntry *subplanVar;
-
-		/*
-		 * Ha! A Var node!
-		 */
-		subplanVar = match_varid((Var *) clause, subplanTargetList);
-
-		/*
-		 * Change the varno & varattno fields of the var node to point to
-		 * the resdom->resno fields of the subplan (lefttree)
+		/* Ignore vars elsewhere in the having clause, since the
+		 * parser already checked 'em.
 		 */
-		((Var *) clause)->varattno = subplanVar->resdom->resno;
-
-		return NIL;
-
 	}
-	/***S*H***/
-	else if (is_funcclause(clause) || not_clause(clause) ||
-			 or_clause(clause) || and_clause(clause))
+	else if (is_subplan(clause))
 	{
-		int			new_length = 0,
-					old_length = 0;
-
 		/*
-		 * This is a function. Recursively call this routine for its
-		 * arguments... (i.e. for AND, OR, ... clauses!)
+		 * The args list of the subplan node represents attributes from outside
+		 * passed into the sublink.
 		 */
 		foreach(t, ((Expr *) clause)->args)
 		{
-			old_length = length((List *) agg_list);
-
-			agg_list = nconc(agg_list,
-				 check_having_qual_for_aggs(lfirst(t), subplanTargetList,
-											groupClause));
-
-			/*
-			 * The arguments of OR or AND clauses are comparisons or
-			 * relations and because we are in the havingQual there must
-			 * be at least one operand using an aggregate function. If so,
-			 * we will find it and the length of the agg_list will be
-			 * increased after the above call to
-			 * check_having_qual_for_aggs. If there are no aggregates
-			 * used, the query could have been formulated using the
-			 * 'where' clause
-			 */
-			if (((new_length = length((List *) agg_list)) == old_length) || (new_length == 0))
+			bool contained_in_group_clause = false;
+			List	   *gl;
+
+			foreach(gl, groupClause)
 			{
-				elog(ERROR, "This could have been done in a where clause!!");
-				return NIL;
+				if (var_equal(lfirst(t),
+							  get_expr(((GroupClause *) lfirst(gl))->entry)))
+				{
+					contained_in_group_clause = true;
+					break;
+				}
 			}
+
+			if (!contained_in_group_clause)
+				elog(ERROR, "Sub-SELECT in HAVING clause must use only GROUPed attributes from outer SELECT");
 		}
-		return agg_list;
+	}
+	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(t, ((Expr *) clause)->args)
+			check_having_for_ungrouped_vars(lfirst(t), groupClause);
+	}
+	else if (IsA(clause, List))
+	{
+		/*
+		 * Recursively scan AND subclauses (see NOTE above).
+		 */
+		foreach(t, ((List *) clause))
+			check_having_for_ungrouped_vars(lfirst(t), groupClause);
 	}
 	else if (IsA(clause, Aggref))
 	{
-		return lcons(clause,
-					 check_having_qual_for_aggs(((Aggref *) clause)->target, subplanTargetList,
-												groupClause));
+			check_having_for_ungrouped_vars(((Aggref *) clause)->target,
+											groupClause);
 	}
 	else if (IsA(clause, ArrayRef))
 	{
@@ -1109,130 +1095,22 @@ check_having_qual_for_aggs(Node *clause, List *subplanTargetList, List *groupCla
 		 * expression and its index expression...
 		 */
 		foreach(t, aref->refupperindexpr)
-		{
-			agg_list = nconc(agg_list,
-				 check_having_qual_for_aggs(lfirst(t), subplanTargetList,
-											groupClause));
-		}
+			check_having_for_ungrouped_vars(lfirst(t), groupClause);
 		foreach(t, aref->reflowerindexpr)
-		{
-			agg_list = nconc(agg_list,
-				 check_having_qual_for_aggs(lfirst(t), subplanTargetList,
-											groupClause));
-		}
-		agg_list = nconc(agg_list,
-			 check_having_qual_for_aggs(aref->refexpr, subplanTargetList,
-										groupClause));
-		agg_list = nconc(agg_list,
-		check_having_qual_for_aggs(aref->refassgnexpr, subplanTargetList,
-								   groupClause));
-
-		return agg_list;
+			check_having_for_ungrouped_vars(lfirst(t), groupClause);
+		check_having_for_ungrouped_vars(aref->refexpr, groupClause);
+		check_having_for_ungrouped_vars(aref->refassgnexpr, groupClause);
 	}
-	else if (is_opclause(clause))
-	{
-
-		/*
-		 * This is an operator. Recursively call this routine for both its
-		 * left and right operands
-		 */
-		Node	   *left = (Node *) get_leftop((Expr *) clause);
-		Node	   *right = (Node *) get_rightop((Expr *) clause);
-
-		if (left != (Node *) NULL)
-			agg_list = nconc(agg_list,
-					  check_having_qual_for_aggs(left, subplanTargetList,
-												 groupClause));
-		if (right != (Node *) NULL)
-			agg_list = nconc(agg_list,
-					 check_having_qual_for_aggs(right, subplanTargetList,
-												groupClause));
-
-		return agg_list;
-	}
-	else if (IsA(clause, Param) ||IsA(clause, Const))
+	else if (IsA(clause, Param) || IsA(clause, Const))
 	{
 		/* do nothing! */
-		return NIL;
-	}
-
-	/*
-	 * This is for Sublinks which show up as EXPR nodes. All the other
-	 * EXPR nodes (funcclauses, and_clauses, or_clauses) were caught above
-	 */
-	else if (IsA(clause, Expr))
-	{
-
-		/*
-		 * Only the lefthand side of the sublink has to be checked for
-		 * aggregates to be attached to result_plan->aggs (see
-		 * planner.c:union_planner() )
-		 */
-		foreach(t, ((List *) ((SubLink *) ((SubPlan *)
-						   ((Expr *) clause)->oper)->sublink)->lefthand))
-		{
-			agg_list = nconc(agg_list,
-					  check_having_qual_for_aggs(lfirst(t),
-										subplanTargetList, groupClause));
-		}
-
-		/* The first argument of ...->oper has also to be checked */
-		{
-			List	   *tmp_ptr;
-
-			foreach(tmp_ptr, ((SubLink *) ((SubPlan *)
-								((Expr *) clause)->oper)->sublink)->oper)
-			{
-				agg_list = nconc(agg_list,
-						 check_having_qual_for_aggs((Node *) lfirst(((Expr *)
-												 lfirst(tmp_ptr))->args),
-										subplanTargetList, groupClause));
-			}
-		}
-
-		/*
-		 * All arguments to the Sublink node are attributes from outside
-		 * used within the sublink. Here we have to check that only
-		 * attributes that is grouped for are used!
-		 */
-		foreach(t, ((Expr *) clause)->args)
-		{
-			contained_in_group_clause = 0;
-
-			foreach(l1, groupClause)
-			{
-				if (tlist_member(lfirst(t), lcons(((GroupClause *) lfirst(l1))->entry, NIL)) !=
-					NULL)
-					contained_in_group_clause = 1;
-			}
-
-			/*
-			 * If the use of the attribute is allowed (i.e. it is in the
-			 * groupClause) we have to adjust the varnos and varattnos
-			 */
-			if (contained_in_group_clause)
-			{
-				agg_list = nconc(agg_list,
-						  		 check_having_qual_for_aggs(lfirst(t),
-										subplanTargetList, groupClause));
-			}
-			else
-			{
-				elog(ERROR, "You must group by the attribute used from outside!");
-				return NIL;
-			}
-		}
-		return agg_list;
 	}
 	else
 	{
 		/*
 		 * Ooops! we can not handle that!
 		 */
-		elog(ERROR, "check_having_qual_for_aggs: Can not handle this having_qual! %d\n",
+		elog(ERROR, "check_having_for_ungrouped_vars: Cannot handle node type %d",
 			 nodeTag(clause));
-		return NIL;
 	}
 }
-
- /***S*H***//* End */
diff --git a/src/include/optimizer/planmain.h b/src/include/optimizer/planmain.h
index 42e22e150f30cb6dcb6b6289f7e62e8f183c6a24..44dad8cfd66daaa28167cc9026edc01a1542ab76 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.22 1999/02/14 04:56:58 momjian Exp $
+ * $Id: planmain.h,v 1.23 1999/04/19 01:43:10 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -54,12 +54,10 @@ 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 List *get_agg_tlist_references(Agg *aggNode);
-extern void set_agg_agglist_references(Agg *aggNode);
+extern bool set_agg_tlist_references(Agg *aggNode);
 extern void del_agg_tlist_references(List *tlist);
-extern List *check_having_qual_for_aggs(Node *clause,
-						   List *subplanTargetList, List *groupClause);
 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);
 
 #endif	 /* PLANMAIN_H */