aggregate(DISTINCT ...) works, per SQL spec.

Note this forces initdb because of change of Aggref node in stored rules.

Author: tglsfdc

src/backend/executor/nodeAgg.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v 1.97 1999/11/23 20:06:52 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v 1.98 1999/12/13 01:26:53 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -832,6 +832,8 @@ _copyAggref(Aggref *from)
 	newnode->aggtype = from->aggtype;
 	Node_Copy(from, newnode, target);
 	newnode->usenulls = from->usenulls;
+	newnode->aggstar = from->aggstar;
+	newnode->aggdistinct = from->aggdistinct;
 	newnode->aggno = from->aggno; /* probably not needed */
 
 	return newnode;

src/backend/nodes/copyfuncs.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/nodes/equalfuncs.c,v 1.52 1999/11/23 20:06:52 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/nodes/equalfuncs.c,v 1.53 1999/12/13 01:26:53 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -219,6 +219,10 @@ _equalAggref(Aggref *a, Aggref *b)
 		return false;
 	if (a->usenulls != b->usenulls)
 		return false;
+	if (a->aggstar != b->aggstar)
+		return false;
+	if (a->aggdistinct != b->aggdistinct)
+		return false;
 	/* ignore aggno, which is only a private field for the executor */
 	return true;
 }

src/backend/nodes/equalfuncs.c

@@ -5,7 +5,7 @@
  *
  * Copyright (c) 1994, Regents of the University of California
  *
- *	$Id: outfuncs.c,v 1.99 1999/12/10 07:37:31 tgl Exp $
+ *	$Id: outfuncs.c,v 1.100 1999/12/13 01:26:53 tgl Exp $
  *
  * NOTES
  *	  Every (plan) node in POSTGRES has an associated "out" routine which
@@ -680,14 +680,17 @@ static void
 _outAggref(StringInfo str, Aggref *node)
 {
 	appendStringInfo(str,
-				 " AGGREG :aggname %s :basetype %u :aggtype %u :target ",
+					 " AGGREG :aggname %s :basetype %u :aggtype %u :target ",
 					 stringStringInfo(node->aggname),
 					 node->basetype,
 					 node->aggtype);
 	_outNode(str, node->target);
 
-	appendStringInfo(str, " :usenulls %s ",
-					 node->usenulls ? "true" : "false");
+	appendStringInfo(str, " :usenulls %s :aggstar %s :aggdistinct %s ",
+					 node->usenulls ? "true" : "false",
+					 node->aggstar ? "true" : "false",
+					 node->aggdistinct ? "true" : "false");
+	/* aggno is not dumped */
 }
 
 /*

src/backend/nodes/outfuncs.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/nodes/readfuncs.c,v 1.75 1999/11/23 20:06:53 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/nodes/readfuncs.c,v 1.76 1999/12/13 01:26:54 tgl Exp $
  *
  * NOTES
  *	  Most of the read functions for plan nodes are tested. (In fact, they
@@ -1190,6 +1190,14 @@ _readAggref()
 	token = lsptok(NULL, &length);		/* get usenulls */
 	local_node->usenulls = (token[0] == 't') ? true : false;
 
+	token = lsptok(NULL, &length);		/* eat :aggstar */
+	token = lsptok(NULL, &length);		/* get aggstar */
+	local_node->aggstar = (token[0] == 't') ? true : false;
+
+	token = lsptok(NULL, &length);		/* eat :aggdistinct */
+	token = lsptok(NULL, &length);		/* get aggdistinct */
+	local_node->aggdistinct = (token[0] == 't') ? true : false;
+
 	return local_node;
 }
 

src/backend/nodes/readfuncs.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.56 1999/12/09 05:58:53 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.57 1999/12/13 01:26:55 tgl Exp $
  *
  * HISTORY
  *	  AUTHOR			DATE			MAJOR EVENT
@@ -45,6 +45,7 @@ typedef struct {
 	List	   *targetList;
 } check_subplans_for_ungrouped_vars_context;
 
+static bool contain_agg_clause_walker(Node *node, void *context);
 static bool pull_agg_clause_walker(Node *node, List **listptr);
 static bool check_subplans_for_ungrouped_vars_walker(Node *node,
 					check_subplans_for_ungrouped_vars_context *context);
@@ -393,12 +394,36 @@ pull_constant_clauses(List *quals, List **constantQual)
 	return restqual;
 }
 
+/*
+ * contain_agg_clause
+ *	  Recursively search for Aggref nodes within a clause.
+ *
+ *	  Returns true if any aggregate found.
+ */
+bool
+contain_agg_clause(Node *clause)
+{
+	return contain_agg_clause_walker(clause, NULL);
+}
+
+static bool
+contain_agg_clause_walker(Node *node, void *context)
+{
+	if (node == NULL)
+		return false;
+	if (IsA(node, Aggref))
+		return true;			/* abort the tree traversal and return true */
+	return expression_tree_walker(node, contain_agg_clause_walker, context);
+}
+
 /*
  * pull_agg_clause
  *	  Recursively pulls all Aggref nodes from an expression tree.
  *
  *	  Returns list of Aggref nodes found.  Note the nodes themselves are not
  *	  copied, only referenced.
+ *
+ *	  Note: this also checks for nested aggregates, which are an error.
  */
 List *
 pull_agg_clause(Node *clause)
@@ -417,9 +442,16 @@ pull_agg_clause_walker(Node *node, List **listptr)
 	if (IsA(node, Aggref))
 	{
 		*listptr = lappend(*listptr, node);
-		/* continue, to iterate over agg's arg as well (do nested aggregates
-		 * actually work?)
+		/*
+		 * Complain if the aggregate's argument contains any aggregates;
+		 * nested agg functions are semantically nonsensical.
+		 */
+		if (contain_agg_clause(((Aggref *) node)->target))
+			elog(ERROR, "Aggregate function calls may not be nested");
+		/*
+		 * Having checked that, we need not recurse into the argument.
 		 */
+		return false;
 	}
 	return expression_tree_walker(node, pull_agg_clause_walker,
 								  (void *) listptr);

src/backend/optimizer/util/clauses.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/parser/parse_agg.c,v 1.31 1999/12/10 07:37:35 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/parser/parse_agg.c,v 1.32 1999/12/13 01:26:58 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -28,38 +28,11 @@ typedef struct {
 	List	   *groupClauses;
 } check_ungrouped_columns_context;
 
-static bool contain_agg_clause(Node *clause);
-static bool contain_agg_clause_walker(Node *node, void *context);
 static void check_ungrouped_columns(Node *node, ParseState *pstate,
 									List *groupClauses);
 static bool check_ungrouped_columns_walker(Node *node,
 										   check_ungrouped_columns_context *context);
 
-/*
- * contain_agg_clause
- *	  Recursively find aggref nodes within a clause.
- *
- *	  Returns true if any aggregate found.
- *
- * NOTE: we assume that the given clause has been transformed suitably for
- * parser output.  This means we can use the planner's expression_tree_walker.
- */
-static bool
-contain_agg_clause(Node *clause)
-{
-	return contain_agg_clause_walker(clause, NULL);
-}
-
-static bool
-contain_agg_clause_walker(Node *node, void *context)
-{
-	if (node == NULL)
-		return false;
-	if (IsA(node, Aggref))
-		return true;			/* abort the tree traversal and return true */
-	return expression_tree_walker(node, contain_agg_clause_walker, context);
-}
-
 /*
  * check_ungrouped_columns -
  *	  Scan the given expression tree for ungrouped variables (variables
@@ -232,7 +205,8 @@ ParseAgg(ParseState *pstate, char *aggname, Oid basetype,
 	 * Since "1" never evaluates as null, we currently have no need of
 	 * the "usenulls" flag, but it should be kept around; in fact, we should
 	 * extend the pg_aggregate table to let usenulls be specified as an
-	 * attribute of user-defined aggregates.
+	 * attribute of user-defined aggregates.  In the meantime, usenulls
+	 * is just always set to "false".
 	 */
 
 	aggform = (Form_pg_aggregate) GETSTRUCT(theAggTuple);
@@ -264,14 +238,8 @@ ParseAgg(ParseState *pstate, char *aggname, Oid basetype,
 	aggref->aggtype = fintype;
 	aggref->target = lfirst(args);
 	aggref->usenulls = usenulls;
-
-	/*
-	 * We should store agg_star and agg_distinct into the Aggref node,
-	 * and let downstream processing deal with them.  Currently, agg_star
-	 * is ignored and agg_distinct is not implemented...
-	 */
-	if (agg_distinct)
-		elog(ERROR, "aggregate(DISTINCT ...) is not implemented yet");
+	aggref->aggstar = agg_star;
+	aggref->aggdistinct = agg_distinct;
 
 	pstate->p_hasAggs = true;
 

src/backend/parser/parse_agg.c

@@ -3,7 +3,7 @@
  *			  out of it's tuple
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/utils/adt/ruleutils.c,v 1.34 1999/12/06 02:37:17 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/utils/adt/ruleutils.c,v 1.35 1999/12/13 01:27:01 tgl Exp $
  *
  *	  This software is copyrighted by Jan Wieck - Hamburg.
  *
@@ -1352,9 +1352,13 @@ get_rule_expr(Node *node, deparse_context *context)
 			{
 				Aggref	   *aggref = (Aggref *) node;
 
-				appendStringInfo(buf, "%s(",
-								 quote_identifier(aggref->aggname));
-				get_rule_expr(aggref->target, context);
+				appendStringInfo(buf, "%s(%s",
+								 quote_identifier(aggref->aggname),
+								 aggref->aggdistinct ? "DISTINCT " : "");
+				if (aggref->aggstar)
+					appendStringInfo(buf, "*");
+				else
+					get_rule_expr(aggref->target, context);
 				appendStringInfo(buf, ")");
 			}
 			break;