Aggregate functions now support multiple input arguments. I also took

the opportunity to treat COUNT(*) as a zero-argument aggregate instead
of the old hack that equated it to COUNT(1); this is materially cleaner
(no more weird ANYOID cases) and ought to be at least a tiny bit faster.
Original patch by Sergey Koposov; review, documentation, simple regression
tests, pg_dump and psql support by moi.

3 files changed, 56 insertions, 68 deletions

diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 754777c57bc..afd88e8125b 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/parser/gram.y,v 2.551 2006/07/03 22:45:39 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/parser/gram.y,v 2.552 2006/07/27 19:52:05 tgl Exp $
  *
  * HISTORY
  *	  AUTHOR			DATE			MAJOR EVENT
@@ -7346,10 +7346,8 @@ func_expr:	func_name '(' ')'
 			| func_name '(' '*' ')'
 				{
 					/*
-					 * For now, we transform AGGREGATE(*) into AGGREGATE(1).
-					 *
-					 * This does the right thing for COUNT(*) (in fact,
-					 * any certainly-non-null expression would do for COUNT),
+					 * We consider AGGREGATE(*) to invoke a parameterless
+					 * aggregate.  This does the right thing for COUNT(*),
 					 * and there are no other aggregates in SQL92 that accept
 					 * '*' as parameter.
 					 *
@@ -7358,12 +7356,8 @@ func_expr:	func_name '(' ')'
 					 * really was.
 					 */
 					FuncCall *n = makeNode(FuncCall);
-					A_Const *star = makeNode(A_Const);
-
-					star->val.type = T_Integer;
-					star->val.val.ival = 1;
 					n->funcname = $1;
-					n->args = list_make1(star);
+					n->args = NIL;
 					n->agg_star = TRUE;
 					n->agg_distinct = FALSE;
 					n->location = @1;
diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c
index 6d381cd2d9e..3bda907c994 100644
--- a/src/backend/parser/parse_agg.c
+++ b/src/backend/parser/parse_agg.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/parser/parse_agg.c,v 1.72 2006/07/14 14:52:21 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/parser/parse_agg.c,v 1.73 2006/07/27 19:52:05 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -55,10 +55,10 @@ transformAggregateCall(ParseState *pstate, Aggref *agg)
 
 	/*
 	 * The aggregate's level is the same as the level of the lowest-level
-	 * variable or aggregate in its argument; or if it contains no variables
+	 * variable or aggregate in its arguments; or if it contains no variables
 	 * at all, we presume it to be local.
 	 */
-	min_varlevel = find_minimum_var_level((Node *) agg->target);
+	min_varlevel = find_minimum_var_level((Node *) agg->args);
 
 	/*
 	 * An aggregate can't directly contain another aggregate call of the same
@@ -67,7 +67,7 @@ transformAggregateCall(ParseState *pstate, Aggref *agg)
 	 */
 	if (min_varlevel == 0)
 	{
-		if (checkExprHasAggs((Node *) agg->target))
+		if (checkExprHasAggs((Node *) agg->args))
 			ereport(ERROR,
 					(errcode(ERRCODE_GROUPING_ERROR),
 					 errmsg("aggregate function calls may not be nested")));
@@ -360,7 +360,7 @@ check_ungrouped_columns_walker(Node *node,
  * (The trees will never actually be executed, however, so we can skimp
  * a bit on correctness.)
  *
- * agg_input_type, agg_state_type, agg_result_type identify the input,
+ * agg_input_types, agg_state_type, agg_result_type identify the input,
  * transition, and result types of the aggregate.  These should all be
  * resolved to actual types (ie, none should ever be ANYARRAY or ANYELEMENT).
  *
@@ -371,7 +371,8 @@ check_ungrouped_columns_walker(Node *node,
  * *finalfnexpr.  The latter is set to NULL if there's no finalfn.
  */
 void
-build_aggregate_fnexprs(Oid agg_input_type,
+build_aggregate_fnexprs(Oid *agg_input_types,
+						int agg_num_inputs,
 						Oid agg_state_type,
 						Oid agg_result_type,
 						Oid transfn_oid,
@@ -379,13 +380,9 @@ build_aggregate_fnexprs(Oid agg_input_type,
 						Expr **transfnexpr,
 						Expr **finalfnexpr)
 {
-	int			transfn_nargs;
-	Param	   *arg0;
-	Param	   *arg1;
+	Param	   *argp;
 	List	   *args;
-
-	/* get the transition function arg count */
-	transfn_nargs = get_func_nargs(transfn_oid);
+	int			i;
 
 	/*
 	 * Build arg list to use in the transfn FuncExpr node. We really only care
@@ -393,22 +390,21 @@ build_aggregate_fnexprs(Oid agg_input_type,
 	 * get_fn_expr_argtype(), so it's okay to use Param nodes that don't
 	 * correspond to any real Param.
 	 */
-	arg0 = makeNode(Param);
-	arg0->paramkind = PARAM_EXEC;
-	arg0->paramid = -1;
-	arg0->paramtype = agg_state_type;
+	argp = makeNode(Param);
+	argp->paramkind = PARAM_EXEC;
+	argp->paramid = -1;
+	argp->paramtype = agg_state_type;
 
-	if (transfn_nargs == 2)
-	{
-		arg1 = makeNode(Param);
-		arg1->paramkind = PARAM_EXEC;
-		arg1->paramid = -1;
-		arg1->paramtype = agg_input_type;
+	args = list_make1(argp);
 
-		args = list_make2(arg0, arg1);
+	for (i = 0; i < agg_num_inputs; i++)
+	{
+		argp = makeNode(Param);
+		argp->paramkind = PARAM_EXEC;
+		argp->paramid = -1;
+		argp->paramtype = agg_input_types[i];
+		args = lappend(args, argp);
 	}
-	else
-		args = list_make1(arg0);
 
 	*transfnexpr = (Expr *) makeFuncExpr(transfn_oid,
 										 agg_state_type,
@@ -425,11 +421,11 @@ build_aggregate_fnexprs(Oid agg_input_type,
 	/*
 	 * Build expr tree for final function
 	 */
-	arg0 = makeNode(Param);
-	arg0->paramkind = PARAM_EXEC;
-	arg0->paramid = -1;
-	arg0->paramtype = agg_state_type;
-	args = list_make1(arg0);
+	argp = makeNode(Param);
+	argp->paramkind = PARAM_EXEC;
+	argp->paramid = -1;
+	argp->paramtype = agg_state_type;
+	args = list_make1(argp);
 
 	*finalfnexpr = (Expr *) makeFuncExpr(finalfn_oid,
 										 agg_result_type,
diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index aa0632a3898..b1b53164f80 100644
--- a/src/backend/parser/parse_func.c
+++ b/src/backend/parser/parse_func.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/parser/parse_func.c,v 1.188 2006/07/14 14:52:22 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/parser/parse_func.c,v 1.189 2006/07/27 19:52:05 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -259,10 +259,21 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
 
 		aggref->aggfnoid = funcid;
 		aggref->aggtype = rettype;
-		aggref->target = linitial(fargs);
+		aggref->args = fargs;
 		aggref->aggstar = agg_star;
 		aggref->aggdistinct = agg_distinct;
 
+		/*
+		 * Reject attempt to call a parameterless aggregate without (*)
+		 * syntax.  This is mere pedantry but some folks insisted ...
+		 */
+		if (fargs == NIL && !agg_star)
+			ereport(ERROR,
+					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+			   errmsg("%s(*) must be used to call a parameterless aggregate function",
+					  NameListToString(funcname)),
+					 parser_errposition(pstate, location)));
+
 		/* parse_agg.c does additional aggregate-specific processing */
 		transformAggregateCall(pstate, aggref);
 
@@ -1194,9 +1205,7 @@ LookupFuncNameTypeNames(List *funcname, List *argtypes, bool noError)
  *
  * This is almost like LookupFuncNameTypeNames, but the error messages refer
  * to aggregates rather than plain functions, and we verify that the found
- * function really is an aggregate, and we recognize the convention used by
- * the grammar that agg(*) translates to a NIL list, which we have to treat
- * as one ANY argument.  (XXX this ought to be changed)
+ * function really is an aggregate.
  */
 Oid
 LookupAggNameTypeNames(List *aggname, List *argtypes, bool noError)
@@ -1204,7 +1213,7 @@ LookupAggNameTypeNames(List *aggname, List *argtypes, bool noError)
 	Oid			argoids[FUNC_MAX_ARGS];
 	int			argcount;
 	int			i;
-	ListCell   *args_item;
+	ListCell   *lc;
 	Oid			oid;
 	HeapTuple	ftup;
 	Form_pg_proc pform;
@@ -1216,29 +1225,18 @@ LookupAggNameTypeNames(List *aggname, List *argtypes, bool noError)
 				 errmsg("functions cannot have more than %d arguments",
 						FUNC_MAX_ARGS)));
 
-	if (argcount == 0)
-	{
-		/* special case for agg(*) */
-		argoids[0] = ANYOID;
-		argcount = 1;
-	}
-	else
+	i = 0;
+	foreach(lc, argtypes)
 	{
-		args_item = list_head(argtypes);
-		for (i = 0; i < argcount; i++)
-		{
-			TypeName   *t = (TypeName *) lfirst(args_item);
-
-			argoids[i] = LookupTypeName(NULL, t);
-
-			if (!OidIsValid(argoids[i]))
-				ereport(ERROR,
-						(errcode(ERRCODE_UNDEFINED_OBJECT),
-						 errmsg("type \"%s\" does not exist",
-								TypeNameToString(t))));
+		TypeName   *t = (TypeName *) lfirst(lc);
 
-			args_item = lnext(args_item);
-		}
+		argoids[i] = LookupTypeName(NULL, t);
+		if (!OidIsValid(argoids[i]))
+			ereport(ERROR,
+					(errcode(ERRCODE_UNDEFINED_OBJECT),
+					 errmsg("type \"%s\" does not exist",
+							TypeNameToString(t))));
+		i++;
 	}
 
 	oid = LookupFuncName(aggname, argcount, argoids, true);
@@ -1247,7 +1245,7 @@ LookupAggNameTypeNames(List *aggname, List *argtypes, bool noError)
 	{
 		if (noError)
 			return InvalidOid;
-		if (argcount == 1 && argoids[0] == ANYOID)
+		if (argcount == 0)
 			ereport(ERROR,
 					(errcode(ERRCODE_UNDEFINED_FUNCTION),
 					 errmsg("aggregate %s(*) does not exist",