Repair some problems in planner's handling of HAVING clauses.

This fixes a few of the problems Hiroshi Inoue complained of, but
I have not touched the rewrite-related issues.

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;
 }

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 $
  *
  *-------------------------------------------------------------------------
  */
@@ -139,30 +139,12 @@ union_planner(Query *parse)
 	{
 	  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,
@@ -247,33 +229,12 @@ union_planner(Query *parse)
 	}
 
 	/*
-	 * 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 */
-		result_plan = (Plan *) make_agg(tlist, result_plan);
-
-		/*
-		 * get the varno/attno entries to the appropriate references to
-		 * the result tuple of the subplans.
+	 * If we have a HAVING clause, do the necessary things with it.
 	 */
-		((Agg *) result_plan)->aggs = get_agg_tlist_references((Agg *) result_plan); 
-
-		/***S*H***/
-		if(parse->havingQual!=NULL) 
+	if (parse->havingQual)
 	{
-		    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 *));
@@ -281,54 +242,53 @@ union_planner(Query *parse)
 		}
 		PlannerVarParam = lcons(vpm, PlannerVarParam);
 
-
 		/* convert the havingQual to conjunctive normal form (cnf) */
-		    parse->havingQual = (Node *) cnfify((Expr *)(Node *) parse->havingQual,true);
+		parse->havingQual = (Node *) cnfify((Expr *) 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)
 		{
+			/* 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((Node *) 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 (=select the correct functions for
-		     * the used VAR datatypes) */
+		/* Calculate the opfids from the opnos */
 		parse->havingQual = (Node *) fix_opids((List *) parse->havingQual);
 
-		    ((Agg *) result_plan)->plan.qual=(List *) parse->havingQual;
+		PlannerVarParam = lnext(PlannerVarParam);
+		if (vpm != NULL)
+			pfree(vpm);		
+	}
 
-		    /* Check every clause of the havingQual for aggregates used and append
-		     * them to result_plan->aggs
+	/*
+	 * If aggregate is present, insert the agg node
 	 */
-		    foreach(clause, ((Agg *) result_plan)->plan.qual)
+	if (parse->hasAggs)
 	{
-			/* Make sure there are aggregates in the havingQual 
-			 * if so, the list must be longer after check_having_qual_for_aggs
+		/* 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
 		 */
-			old_length=length(((Agg *) result_plan)->aggs);			
+		result_plan = (Plan *) make_agg(tlist, result_plan);
 
-			((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)));
+		/* HAVING clause, if any, becomes qual of the Agg node */
+		result_plan->qual = (List *) parse->havingQual;
 
-			/* 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);		
-		  }
+		/*
+		 * Update vars to refer to subplan result tuples,
+		 * find Aggrefs, make sure there is an Aggref in every HAVING clause.
+		 */
+		if (! set_agg_tlist_references((Agg *) result_plan))
+			elog(ERROR, "SELECT/HAVING requires aggregates to be valid");
 	}		  
 
 	/*

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 */