Support ORDER BY within aggregate function calls, at long last providing a

non-kluge method for controlling the order in which values are fed to an
aggregate function.  At the same time eliminate the old implementation
restriction that DISTINCT was only supported for single-argument aggregates.

Possibly release-notable behavioral change: formerly, agg(DISTINCT x)
dropped null values of x unconditionally.  Now, it does so only if the
agg transition function is strict; otherwise nulls are treated as DISTINCT
normally would, ie, you get one copy.

Andrew Gierth, reviewed by Hitoshi Harada

doc/src/sgml/func.sgml

-<!-- $PostgreSQL: pgsql/doc/src/sgml/func.sgml,v 1.492 2009/11/24 19:21:15 petere Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/func.sgml,v 1.493 2009/12/15 17:57:46 tgl Exp $ -->
 
  <chapter id="functions">
   <title>Functions and Operators</title>
@@ -8440,7 +8440,8 @@ SELECT xmlroot(xmlparse(document '<?xml version="1.1"?><content>abc</content>'),
      The function <function>xmlagg</function> is, unlike the other
      functions described here, an aggregate function.  It concatenates the
      input values to the aggregate function call,
-     like <function>xmlconcat</function> does.
+     much like <function>xmlconcat</function> does, except that concatenation
+     occurs across rows rather than across expressions in a single row.
      See <xref linkend="functions-aggregate"> for additional information
      about aggregate functions.
     </para>
@@ -8459,18 +8460,29 @@ SELECT xmlagg(x) FROM test;
     </para>
 
     <para>
-     To determine the order of the concatenation, something like the
-     following approach can be used:
+     To determine the order of the concatenation, an <literal>ORDER BY</>
+     clause may be added to the aggregate call as described in
+     <xref linkend="syntax-aggregates">. For example:
 
 <screen><![CDATA[
-SELECT xmlagg(x) FROM (SELECT * FROM test ORDER BY y DESC) AS tab;
+SELECT xmlagg(x ORDER BY y DESC) FROM test;
         xmlagg
 ----------------------
  <bar/><foo>abc</foo>
 ]]></screen>
+    </para>
 
-     Again, see <xref linkend="functions-aggregate"> for additional
-     information.
+    <para>
+     The following non-standard approach used to be recommended
+     in previous versions, and may still be useful in specific
+     cases:
+
+<screen><![CDATA[
+SELECT xmlagg(x) FROM (SELECT * FROM test ORDER BY y DESC) AS tab;
+        xmlagg
+----------------------
+ <bar/><foo>abc</foo>
+]]></screen>
     </para>
    </sect3>
 
@@ -9887,20 +9899,19 @@ SELECT count(*) FROM sometable;
    The aggregate functions <function>array_agg</function>
    and <function>xmlagg</function>, as well as similar user-defined
    aggregate functions, produce meaningfully different result values
-   depending on the order of the input values.  In the current
-   implementation, the order of the input is in principle unspecified.
-   Supplying the input values from a sorted subquery
-   will usually work, however.  For example:
+   depending on the order of the input values.  This ordering is
+   unspecified by default, but can be controlled by writing an
+   <literal>ORDER BY</> clause within the aggregate call, as shown in
+   <xref linkend="syntax-aggregates">.
+   Alternatively, supplying the input values from a sorted subquery
+   will usually work.  For example:
 
 <screen><![CDATA[
 SELECT xmlagg(x) FROM (SELECT x FROM test ORDER BY y DESC) AS tab;
 ]]></screen>
 
    But this syntax is not allowed in the SQL standard, and is
-   not portable to other database systems.  A future version of
-   <productname>PostgreSQL</> might provide an additional feature to control
-   the order in a better-defined way (<literal>xmlagg(expr ORDER BY expr, expr,
-   ...)</literal>).
+   not portable to other database systems.
   </para>
 
   <para>

doc/src/sgml/syntax.sgml

-<!-- $PostgreSQL: pgsql/doc/src/sgml/syntax.sgml,v 1.138 2009/11/05 23:24:22 tgl Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/syntax.sgml,v 1.139 2009/12/15 17:57:46 tgl Exp $ -->
 
 <chapter id="sql-syntax">
  <title>SQL Syntax</title>
@@ -1525,17 +1525,19 @@ sqrt(2)
     syntax of an aggregate expression is one of the following:
 
 <synopsis>
-<replaceable>aggregate_name</replaceable> (<replaceable>expression</replaceable> [ , ... ] )
-<replaceable>aggregate_name</replaceable> (ALL <replaceable>expression</replaceable> [ , ... ] )
-<replaceable>aggregate_name</replaceable> (DISTINCT <replaceable>expression</replaceable>)
+<replaceable>aggregate_name</replaceable> (<replaceable>expression</replaceable> [ , ... ] [ <replaceable>order_by_clause</replaceable> ] )
+<replaceable>aggregate_name</replaceable> (ALL <replaceable>expression</replaceable> [ , ... ] [ <replaceable>order_by_clause</replaceable> ] )
+<replaceable>aggregate_name</replaceable> (DISTINCT <replaceable>expression</replaceable> [ , ... ] [ <replaceable>order_by_clause</replaceable> ] )
 <replaceable>aggregate_name</replaceable> ( * )
 </synopsis>
 
     where <replaceable>aggregate_name</replaceable> is a previously
-    defined aggregate (possibly qualified with a schema name), and
+    defined aggregate (possibly qualified with a schema name),
     <replaceable>expression</replaceable> is
     any value expression that does not itself contain an aggregate
-    expression or a window function call.
+    expression or a window function call, and
+    <replaceable>order_by_clause</replaceable> is a optional
+    <literal>ORDER BY</> clause as described below.
    </para>
 
    <para>
@@ -1545,8 +1547,9 @@ sqrt(2)
     whether to ignore null values or not &mdash; but all the standard ones do.)
     The second form is the same as the first, since
     <literal>ALL</literal> is the default.  The third form invokes the
-    aggregate for all distinct non-null values of the expressions found
-    in the input rows.  The last form invokes the aggregate once for
+    aggregate for all distinct values of the expressions found
+    in the input rows (ignoring nulls if the function chooses to do so).
+    The last form invokes the aggregate once for
     each input row regardless of null or non-null values; since no
     particular input value is specified, it is generally only useful
     for the <function>count(*)</function> aggregate function.
@@ -1560,6 +1563,40 @@ sqrt(2)
     distinct non-null values of <literal>f1</literal>.
    </para>
 
+   <para>
+    Ordinarily, the input rows are fed to the aggregate function in an
+    unspecified order.  In many cases this does not matter; for example,
+    <function>min</> produces the same result no matter what order it
+    receives the inputs in.  However, some aggregate functions
+    (such as <function>array_agg</> and <function>xmlagg</>) produce
+    results that depend on the ordering of the input rows.  When using
+    such an aggregate, the optional <replaceable>order_by_clause</> can be
+    used to specify the desired ordering.  The <replaceable>order_by_clause</>
+    has the same syntax as for a query-level <literal>ORDER BY</> clause, as
+    described in <xref linkend="queries-order">, except that its expressions
+    are always just expressions and cannot be output-column names or numbers.
+    For example:
+
+<programlisting>
+SELECT array_agg(a ORDER BY b DESC) FROM table;
+</programlisting>
+   </para>
+
+   <para>
+    If <literal>DISTINCT</> is specified in addition to an
+    <replaceable>order_by_clause</>, then all the <literal>ORDER BY</>
+    expressions must match regular arguments of the aggregate; that is,
+    you cannot sort on an expression that is not included in the
+    <literal>DISTINCT</> list.
+   </para>
+
+   <note>
+    <para>
+     The ability to specify both <literal>DISTINCT</> and <literal>ORDER BY</>
+     in an aggregate function is a <productname>PostgreSQL</> extension.
+    </para>
+   </note>
+
    <para>
     The predefined aggregate functions are described in <xref
     linkend="functions-aggregate">.  Other aggregate functions can be added
@@ -1588,13 +1625,6 @@ sqrt(2)
     appearing only in the result list or <literal>HAVING</> clause
     applies with respect to the query level that the aggregate belongs to.
    </para>
-
-   <note>
-    <para>
-     <productname>PostgreSQL</productname> currently does not support
-     <literal>DISTINCT</> with more than one input expression.
-    </para>
-   </note>
   </sect2>
 
   <sect2 id="syntax-window-functions">
@@ -1697,7 +1727,8 @@ ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING
     <literal>count(*) OVER (PARTITION BY x ORDER BY y)</>.
     <literal>*</> is customarily not used for non-aggregate window functions.
     Aggregate window functions, unlike normal aggregate functions, do not
-    allow <literal>DISTINCT</> to be used within the function argument list.
+    allow <literal>DISTINCT</> or <literal>ORDER BY</> to be used within the
+    function argument list.
    </para>
 
    <para>

src/backend/nodes/copyfuncs.c

@@ -15,7 +15,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.453 2009/12/07 05:22:22 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.454 2009/12/15 17:57:46 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1023,9 +1023,10 @@ _copyAggref(Aggref *from)
 	COPY_SCALAR_FIELD(aggfnoid);
 	COPY_SCALAR_FIELD(aggtype);
 	COPY_NODE_FIELD(args);
-	COPY_SCALAR_FIELD(agglevelsup);
+	COPY_NODE_FIELD(aggorder);
+	COPY_NODE_FIELD(aggdistinct);
 	COPY_SCALAR_FIELD(aggstar);
-	COPY_SCALAR_FIELD(aggdistinct);
+	COPY_SCALAR_FIELD(agglevelsup);
 	COPY_LOCATION_FIELD(location);
 
 	return newnode;
@@ -1968,6 +1969,7 @@ _copyFuncCall(FuncCall *from)
 
 	COPY_NODE_FIELD(funcname);
 	COPY_NODE_FIELD(args);
+	COPY_NODE_FIELD(agg_order);
 	COPY_SCALAR_FIELD(agg_star);
 	COPY_SCALAR_FIELD(agg_distinct);
 	COPY_SCALAR_FIELD(func_variadic);

src/backend/nodes/equalfuncs.c

@@ -22,7 +22,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.375 2009/12/07 05:22:22 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.376 2009/12/15 17:57:46 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -183,9 +183,10 @@ _equalAggref(Aggref *a, Aggref *b)
 	COMPARE_SCALAR_FIELD(aggfnoid);
 	COMPARE_SCALAR_FIELD(aggtype);
 	COMPARE_NODE_FIELD(args);
-	COMPARE_SCALAR_FIELD(agglevelsup);
+	COMPARE_NODE_FIELD(aggorder);
+	COMPARE_NODE_FIELD(aggdistinct);
 	COMPARE_SCALAR_FIELD(aggstar);
-	COMPARE_SCALAR_FIELD(aggdistinct);
+	COMPARE_SCALAR_FIELD(agglevelsup);
 	COMPARE_LOCATION_FIELD(location);
 
 	return true;
@@ -1943,6 +1944,7 @@ _equalFuncCall(FuncCall *a, FuncCall *b)
 {
 	COMPARE_NODE_FIELD(funcname);
 	COMPARE_NODE_FIELD(args);
+	COMPARE_NODE_FIELD(agg_order);
 	COMPARE_SCALAR_FIELD(agg_star);
 	COMPARE_SCALAR_FIELD(agg_distinct);
 	COMPARE_SCALAR_FIELD(func_variadic);

src/backend/nodes/nodeFuncs.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/nodes/nodeFuncs.c,v 1.43 2009/10/08 02:39:21 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/nodes/nodeFuncs.c,v 1.44 2009/12/15 17:57:46 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -882,6 +882,7 @@ exprLocation(Node *expr)
 				FuncCall   *fc = (FuncCall *) expr;
 
 				/* consider both function name and leftmost arg */
+				/* (we assume any ORDER BY nodes must be to right of name) */
 				loc = leftmostLoc(fc->location,
 								  exprLocation((Node *) fc->args));
 			}
@@ -1082,6 +1083,7 @@ expression_tree_walker(Node *node,
 		case T_SetToDefault:
 		case T_CurrentOfExpr:
 		case T_RangeTblRef:
+		case T_SortGroupClause:
 			/* primitive node types with no expression subnodes */
 			break;
 		case T_Aggref:
@@ -1092,6 +1094,12 @@ expression_tree_walker(Node *node,
 				if (expression_tree_walker((Node *) expr->args,
 										   walker, context))
 					return true;
+				if (expression_tree_walker((Node *) expr->aggorder,
+										   walker, context))
+					return true;
+				if (expression_tree_walker((Node *) expr->aggdistinct,
+										   walker, context))
+					return true;
 			}
 			break;
 		case T_WindowFunc:
@@ -1591,6 +1599,7 @@ expression_tree_mutator(Node *node,
 		case T_SetToDefault:
 		case T_CurrentOfExpr:
 		case T_RangeTblRef:
+		case T_SortGroupClause:
 			return (Node *) copyObject(node);
 		case T_Aggref:
 			{
@@ -1599,6 +1608,8 @@ expression_tree_mutator(Node *node,
 
 				FLATCOPY(newnode, aggref, Aggref);
 				MUTATE(newnode->args, aggref->args, List *);
+				MUTATE(newnode->aggorder, aggref->aggorder, List *);
+				MUTATE(newnode->aggdistinct, aggref->aggdistinct, List *);
 				return (Node *) newnode;
 			}
 			break;
@@ -2403,6 +2414,8 @@ bool
 
 				if (walker(fcall->args, context))
 					return true;
+				if (walker(fcall->agg_order, context))
+					return true;
 				if (walker(fcall->over, context))
 					return true;
 				/* function name is deemed uninteresting */

src/backend/nodes/outfuncs.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.374 2009/12/07 05:22:22 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.375 2009/12/15 17:57:46 tgl Exp $
  *
  * NOTES
  *	  Every node type that can appear in stored rules' parsetrees *must*
@@ -869,9 +869,10 @@ _outAggref(StringInfo str, Aggref *node)
 	WRITE_OID_FIELD(aggfnoid);
 	WRITE_OID_FIELD(aggtype);
 	WRITE_NODE_FIELD(args);
-	WRITE_UINT_FIELD(agglevelsup);
+	WRITE_NODE_FIELD(aggorder);
+	WRITE_NODE_FIELD(aggdistinct);
 	WRITE_BOOL_FIELD(aggstar);
-	WRITE_BOOL_FIELD(aggdistinct);
+	WRITE_UINT_FIELD(agglevelsup);
 	WRITE_LOCATION_FIELD(location);
 }
 
@@ -1857,6 +1858,7 @@ _outFuncCall(StringInfo str, FuncCall *node)
 
 	WRITE_NODE_FIELD(funcname);
 	WRITE_NODE_FIELD(args);
+	WRITE_NODE_FIELD(agg_order);
 	WRITE_BOOL_FIELD(agg_star);
 	WRITE_BOOL_FIELD(agg_distinct);
 	WRITE_BOOL_FIELD(func_variadic);

src/backend/nodes/readfuncs.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/nodes/readfuncs.c,v 1.227 2009/10/28 14:55:38 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/nodes/readfuncs.c,v 1.228 2009/12/15 17:57:46 tgl Exp $
  *
  * NOTES
  *	  Path and Plan nodes do not have any readfuncs support, because we
@@ -461,9 +461,10 @@ _readAggref(void)
 	READ_OID_FIELD(aggfnoid);
 	READ_OID_FIELD(aggtype);
 	READ_NODE_FIELD(args);
-	READ_UINT_FIELD(agglevelsup);
+	READ_NODE_FIELD(aggorder);
+	READ_NODE_FIELD(aggdistinct);
 	READ_BOOL_FIELD(aggstar);
-	READ_BOOL_FIELD(aggdistinct);
+	READ_UINT_FIELD(agglevelsup);
 	READ_LOCATION_FIELD(location);
 
 	READ_DONE();

src/backend/optimizer/plan/planagg.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/optimizer/plan/planagg.c,v 1.46 2009/06/11 14:48:59 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/plan/planagg.c,v 1.47 2009/12/15 17:57:46 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -239,12 +239,12 @@ find_minmax_aggs_walker(Node *node, List **context)
 	{
 		Aggref	   *aggref = (Aggref *) node;
 		Oid			aggsortop;
-		Expr	   *curTarget;
+		TargetEntry *curTarget;
 		MinMaxAggInfo *info;
 		ListCell   *l;
 
 		Assert(aggref->agglevelsup == 0);
-		if (list_length(aggref->args) != 1)
+		if (list_length(aggref->args) != 1 || aggref->aggorder != NIL)
 			return true;		/* it couldn't be MIN/MAX */
 		/* note: we do not care if DISTINCT is mentioned ... */
 
@@ -255,19 +255,19 @@ find_minmax_aggs_walker(Node *node, List **context)
 		/*
 		 * Check whether it's already in the list, and add it if not.
 		 */
-		curTarget = linitial(aggref->args);
+		curTarget = (TargetEntry *) linitial(aggref->args);
 		foreach(l, *context)
 		{
 			info = (MinMaxAggInfo *) lfirst(l);
 			if (info->aggfnoid == aggref->aggfnoid &&
-				equal(info->target, curTarget))
+				equal(info->target, curTarget->expr))
 				return false;
 		}
 
 		info = (MinMaxAggInfo *) palloc0(sizeof(MinMaxAggInfo));
 		info->aggfnoid = aggref->aggfnoid;
 		info->aggsortop = aggsortop;
-		info->target = curTarget;
+		info->target = curTarget->expr;
 
 		*context = lappend(*context, info);
 
@@ -584,15 +584,15 @@ replace_aggs_with_params_mutator(Node *node, List **context)
 	if (IsA(node, Aggref))
 	{
 		Aggref	   *aggref = (Aggref *) node;
+		TargetEntry *curTarget = (TargetEntry *) linitial(aggref->args);
 		ListCell   *l;
-		Expr	   *curTarget = linitial(aggref->args);
 
 		foreach(l, *context)
 		{
 			MinMaxAggInfo *info = (MinMaxAggInfo *) lfirst(l);
 
 			if (info->aggfnoid == aggref->aggfnoid &&
-				equal(info->target, curTarget))
+				equal(info->target, curTarget->expr))
 				return (Node *) info->param;
 		}
 		elog(ERROR, "failed to re-find aggregate info record");

src/backend/optimizer/plan/planner.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.261 2009/10/28 14:55:38 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.262 2009/12/15 17:57:46 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1096,11 +1096,12 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
 			bool		can_sort;
 
 			/*
-			 * Executor doesn't support hashed aggregation with DISTINCT
-			 * aggregates.	(Doing so would imply storing *all* the input
-			 * values in the hash table, which seems like a certain loser.)
+			 * Executor doesn't support hashed aggregation with DISTINCT or
+			 * ORDER BY aggregates.  (Doing so would imply storing *all* the
+			 * input values in the hash table, and/or running many sorts in
+			 * parallel, either of which seems like a certain loser.)
 			 */
-			can_hash = (agg_counts.numDistinctAggs == 0 &&
+			can_hash = (agg_counts.numOrderedAggs == 0 &&
 						grouping_is_hashable(parse->groupClause));
 			can_sort = grouping_is_sortable(parse->groupClause);
 			if (can_hash && can_sort)

src/backend/optimizer/util/clauses.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/optimizer/util/clauses.c,v 1.280 2009/12/14 02:15:52 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/util/clauses.c,v 1.281 2009/12/15 17:57:47 tgl Exp $
  *
  * HISTORY
  *	  AUTHOR			DATE			MAJOR EVENT
@@ -471,21 +471,22 @@ count_agg_clauses_walker(Node *node, AggClauseCounts *counts)
 		HeapTuple	aggTuple;
 		Form_pg_aggregate aggform;
 		Oid			aggtranstype;
-		int			i;
 		ListCell   *l;
 
 		Assert(aggref->agglevelsup == 0);
 		counts->numAggs++;
-		if (aggref->aggdistinct)
-			counts->numDistinctAggs++;
+		if (aggref->aggorder != NIL || aggref->aggdistinct != NIL)
+			counts->numOrderedAggs++;
 
-		/* extract argument types */
-		numArguments = list_length(aggref->args);
-		inputTypes = (Oid *) palloc(sizeof(Oid) * numArguments);
-		i = 0;
+		/* extract argument types (ignoring any ORDER BY expressions) */
+		inputTypes = (Oid *) palloc(sizeof(Oid) * list_length(aggref->args));
+		numArguments = 0;
 		foreach(l, aggref->args)
 		{
-			inputTypes[i++] = exprType((Node *) lfirst(l));
+			TargetEntry *tle = (TargetEntry *) lfirst(l);
+
+			if (!tle->resjunk)
+				inputTypes[numArguments++] = exprType((Node *) tle->expr);
 		}
 
 		/* fetch aggregate transition datatype from pg_aggregate */

src/backend/parser/analyze.c

@@ -17,7 +17,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- *	$PostgreSQL: pgsql/src/backend/parser/analyze.c,v 1.396 2009/10/31 01:41:31 tgl Exp $
+ *	$PostgreSQL: pgsql/src/backend/parser/analyze.c,v 1.397 2009/12/15 17:57:47 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -828,13 +828,13 @@ transformSelectStmt(ParseState *pstate, SelectStmt *stmt)
 										  stmt->sortClause,
 										  &qry->targetList,
 										  true /* fix unknowns */,
-										  false /* not window function */);
+										  false /* allow SQL92 rules */);
 
 	qry->groupClause = transformGroupClause(pstate,
 											stmt->groupClause,
 											&qry->targetList,
 											qry->sortClause,
-											false /* not window function */);
+											false /* allow SQL92 rules */);
 
 	if (stmt->distinctClause == NIL)
 	{
@@ -846,7 +846,8 @@ transformSelectStmt(ParseState *pstate, SelectStmt *stmt)
 		/* We had SELECT DISTINCT */
 		qry->distinctClause = transformDistinctClause(pstate,
 													  &qry->targetList,
-													  qry->sortClause);
+													  qry->sortClause,
+													  false);
 		qry->hasDistinctOn = false;
 	}
 	else
@@ -1044,7 +1045,7 @@ transformValuesClause(ParseState *pstate, SelectStmt *stmt)
 										  stmt->sortClause,
 										  &qry->targetList,
 										  true /* fix unknowns */,
-										  false /* not window function */);
+										  false /* allow SQL92 rules */);
 
 	qry->limitOffset = transformLimitClause(pstate, stmt->limitOffset,
 											"OFFSET");
@@ -1298,7 +1299,7 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt)
 										  sortClause,
 										  &qry->targetList,
 										  false /* no unknowns expected */,
-										  false /* not window function */);
+										  false /* allow SQL92 rules */);
 
 	pstate->p_rtable = list_truncate(pstate->p_rtable, sv_rtable_length);
 	pstate->p_relnamespace = sv_relnamespace;

src/backend/parser/parse_agg.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/parser/parse_agg.c,v 1.88 2009/06/11 14:49:00 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/parser/parse_agg.c,v 1.89 2009/12/15 17:57:47 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -19,8 +19,10 @@
 #include "optimizer/tlist.h"
 #include "optimizer/var.h"
 #include "parser/parse_agg.h"
+#include "parser/parse_clause.h"
 #include "parser/parsetree.h"
 #include "rewrite/rewriteManip.h"
+#include "utils/builtins.h"
 #include "utils/lsyscache.h"
 
 
@@ -43,15 +45,97 @@ static bool check_ungrouped_columns_walker(Node *node,
  *		Finish initial transformation of an aggregate call
  *
  * parse_func.c has recognized the function as an aggregate, and has set
- * up all the fields of the Aggref except agglevelsup.	Here we must
- * determine which query level the aggregate actually belongs to, set
- * agglevelsup accordingly, and mark p_hasAggs true in the corresponding
+ * up all the fields of the Aggref except aggdistinct and agglevelsup.
+ * However, the args list is just bare expressions, and the aggorder list
+ * hasn't been transformed at all.
+ *
+ * Here we convert the args list into a targetlist by inserting TargetEntry
+ * nodes, and then transform the aggorder and agg_distinct specifications to
+ * produce lists of SortGroupClause nodes.  (That might also result in adding
+ * resjunk expressions to the targetlist.)
+ *
+ * We must also determine which query level the aggregate actually belongs to,
+ * set agglevelsup accordingly, and mark p_hasAggs true in the corresponding
  * pstate level.
  */
 void
-transformAggregateCall(ParseState *pstate, Aggref *agg)
+transformAggregateCall(ParseState *pstate, Aggref *agg, bool agg_distinct)
 {
+	List       *tlist;
+	List       *torder;
+	List       *tdistinct = NIL;
+	AttrNumber  attno;
+	int         save_next_resno;
 	int			min_varlevel;
+	ListCell   *lc;
+
+	/*
+	 * Transform the plain list of Exprs into a targetlist.  We don't bother
+	 * to assign column names to the entries.
+	 */
+	tlist = NIL;
+	attno = 1;
+	foreach(lc, agg->args)
+	{
+		Expr        *arg  = (Expr *) lfirst(lc);
+		TargetEntry *tle = makeTargetEntry(arg, attno++, NULL, false);
+
+		tlist = lappend(tlist, tle);
+	}
+
+	/*
+	 * If we have an ORDER BY, transform it.  This will add columns to the
+	 * tlist if they appear in ORDER BY but weren't already in the arg list.
+	 * They will be marked resjunk = true so we can tell them apart from
+	 * regular aggregate arguments later.
+	 *
+	 * We need to mess with p_next_resno since it will be used to number any
+	 * new targetlist entries.
+	 */
+	save_next_resno = pstate->p_next_resno;
+	pstate->p_next_resno = attno;
+
+	torder = transformSortClause(pstate,
+								 agg->aggorder,
+								 &tlist,
+								 true /* fix unknowns */,
+								 true /* force SQL99 rules */);
+
+	/*
+	 * If we have DISTINCT, transform that to produce a distinctList.
+	 */
+	if (agg_distinct)
+	{
+		tdistinct = transformDistinctClause(pstate, &tlist, torder, true);
+
+		/*
+		 * Remove this check if executor support for hashed distinct for
+		 * aggregates is ever added.
+		 */
+		foreach(lc, tdistinct)
+		{
+			SortGroupClause *sortcl = (SortGroupClause *) lfirst(lc);
+
+			if (!OidIsValid(sortcl->sortop))
+			{
+				Node   *expr = get_sortgroupclause_expr(sortcl, tlist);
+
+				ereport(ERROR,
+						(errcode(ERRCODE_UNDEFINED_FUNCTION),
+						 errmsg("could not identify an ordering operator for type %s",
+								format_type_be(exprType(expr))),
+						 errdetail("Aggregates with DISTINCT must be able to sort their inputs."),
+						 parser_errposition(pstate, exprLocation(expr))));
+			}
+		}
+	}
+
+	/* Update the Aggref with the transformation results */
+	agg->args = tlist;
+	agg->aggorder = torder;
+	agg->aggdistinct = tdistinct;
+
+	pstate->p_next_resno = save_next_resno;
 
 	/*
 	 * The aggregate's level is the same as the level of the lowest-level

src/backend/parser/parse_clause.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/parser/parse_clause.c,v 1.193 2009/10/27 17:11:18 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/parser/parse_clause.c,v 1.194 2009/12/15 17:57:47 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1440,7 +1440,7 @@ findTargetlistEntrySQL99(ParseState *pstate, Node *node, List **tlist)
 List *
 transformGroupClause(ParseState *pstate, List *grouplist,
 					 List **targetlist, List *sortClause,
-					 bool isWindowFunc)
+					 bool useSQL99)
 {
 	List	   *result = NIL;
 	ListCell   *gl;
@@ -1451,7 +1451,7 @@ transformGroupClause(ParseState *pstate, List *grouplist,
 		TargetEntry *tle;
 		bool		found = false;
 
-		if (isWindowFunc)
+		if (useSQL99)
 			tle = findTargetlistEntrySQL99(pstate, gexpr, targetlist);
 		else
 			tle = findTargetlistEntrySQL92(pstate, gexpr, targetlist,
@@ -1510,15 +1510,15 @@ transformGroupClause(ParseState *pstate, List *grouplist,
  * ORDER BY items will be added to the targetlist (as resjunk columns)
  * if not already present, so the targetlist must be passed by reference.
  *
- * This is also used for window ORDER BY clauses (which act almost the
- * same, but are always interpreted per SQL99 rules).
+ * This is also used for window and aggregate ORDER BY clauses (which act
+ * almost the same, but are always interpreted per SQL99 rules).
  */
 List *
 transformSortClause(ParseState *pstate,
 					List *orderlist,
 					List **targetlist,
 					bool resolveUnknown,
-					bool isWindowFunc)
+					bool useSQL99)
 {
 	List	   *sortlist = NIL;
 	ListCell   *olitem;
@@ -1528,7 +1528,7 @@ transformSortClause(ParseState *pstate,
 		SortBy	   *sortby = (SortBy *) lfirst(olitem);
 		TargetEntry *tle;
 
-		if (isWindowFunc)
+		if (useSQL99)
 			tle = findTargetlistEntrySQL99(pstate, sortby->node, targetlist);
 		else
 			tle = findTargetlistEntrySQL92(pstate, sortby->node, targetlist,
@@ -1598,12 +1598,12 @@ transformWindowDefinitions(ParseState *pstate,
 										  windef->orderClause,
 										  targetlist,
 										  true /* fix unknowns */,
-										  true /* window function */);
+										  true /* force SQL99 rules */);
 		partitionClause = transformGroupClause(pstate,
 											   windef->partitionClause,
 											   targetlist,
 											   orderClause,
-											   true /* window function */);
+											   true /* force SQL99 rules */);
 
 		/*
 		 * And prepare the new WindowClause.
@@ -1684,10 +1684,14 @@ transformWindowDefinitions(ParseState *pstate,
  * and allows the user to choose the equality semantics used by DISTINCT,
  * should she be working with a datatype that has more than one equality
  * operator.
+ *
+ * is_agg is true if we are transforming an aggregate(DISTINCT ...)
+ * function call.  This does not affect any behavior, only the phrasing
+ * of error messages.
  */
 List *
 transformDistinctClause(ParseState *pstate,
-						List **targetlist, List *sortClause)
+						List **targetlist, List *sortClause, bool is_agg)
 {
 	List	   *result = NIL;
 	ListCell   *slitem;
@@ -1716,6 +1720,8 @@ transformDistinctClause(ParseState *pstate,
 		if (tle->resjunk)
 			ereport(ERROR,
 					(errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
+					 is_agg ?
+					 errmsg("in an aggregate with DISTINCT, ORDER BY expressions must appear in argument list") :
 					 errmsg("for SELECT DISTINCT, ORDER BY expressions must appear in select list"),
 					 parser_errposition(pstate,
 										exprLocation((Node *) tle->expr))));

src/backend/parser/parse_expr.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/parser/parse_expr.c,v 1.250 2009/11/13 19:48:20 heikki Exp $
+ *	  $PostgreSQL: pgsql/src/backend/parser/parse_expr.c,v 1.251 2009/12/15 17:57:47 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -431,7 +431,7 @@ transformIndirection(ParseState *pstate, Node *basenode, List *indirection)
 			newresult = ParseFuncOrColumn(pstate,
 										  list_make1(n),
 										  list_make1(result),
-										  false, false, false,
+										  NIL, false, false, false,
 										  NULL, true, location);
 			if (newresult == NULL)
 				unknown_attribute(pstate, result, strVal(n), location);
@@ -598,7 +598,7 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref)
 					node = ParseFuncOrColumn(pstate,
 											 list_make1(makeString(colname)),
 											 list_make1(node),
-											 false, false, false,
+											 NIL, false, false, false,
 											 NULL, true, cref->location);
 				}
 				break;
@@ -643,7 +643,7 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref)
 					node = ParseFuncOrColumn(pstate,
 											 list_make1(makeString(colname)),
 											 list_make1(node),
-											 false, false, false,
+											 NIL, false, false, false,
 											 NULL, true, cref->location);
 				}
 				break;
@@ -701,7 +701,7 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref)
 					node = ParseFuncOrColumn(pstate,
 											 list_make1(makeString(colname)),
 											 list_make1(node),
-											 false, false, false,
+											 NIL, false, false, false,
 											 NULL, true, cref->location);
 				}
 				break;
@@ -1221,6 +1221,7 @@ transformFuncCall(ParseState *pstate, FuncCall *fn)
 	return ParseFuncOrColumn(pstate,
 							 fn->funcname,
 							 targs,
+							 fn->agg_order,
 							 fn->agg_star,
 							 fn->agg_distinct,
 							 fn->func_variadic,

src/backend/parser/parse_func.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/parser/parse_func.c,v 1.218 2009/10/31 01:41:31 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/parser/parse_func.c,v 1.219 2009/12/15 17:57:47 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -55,10 +55,12 @@ static Node *ParseComplexProjection(ParseState *pstate, char *funcname,
  *	reporting a no-such-function error.
  *
  *	The argument expressions (in fargs) must have been transformed already.
+ *	But the agg_order expressions, if any, have not been.
  */
 Node *
 ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
-				  bool agg_star, bool agg_distinct, bool func_variadic,
+				  List *agg_order, bool agg_star, bool agg_distinct,
+				  bool func_variadic,
 				  WindowDef *over, bool is_column, int location)
 {
 	Oid			rettype;
@@ -170,8 +172,9 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
 	 * the "function call" could be a projection.  We also check that there
 	 * wasn't any aggregate or variadic decoration, nor an argument name.
 	 */
-	if (nargs == 1 && !agg_star && !agg_distinct && over == NULL &&
-		!func_variadic && argnames == NIL && list_length(funcname) == 1)
+	if (nargs == 1 && agg_order == NIL && !agg_star && !agg_distinct &&
+		over == NULL && !func_variadic && argnames == NIL &&
+		list_length(funcname) == 1)
 	{
 		Oid			argtype = actual_arg_types[0];
 
@@ -240,6 +243,12 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
 			errmsg("DISTINCT specified, but %s is not an aggregate function",
 				   NameListToString(funcname)),
 					 parser_errposition(pstate, location)));
+		if (agg_order != NIL)
+			ereport(ERROR,
+					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+			   errmsg("ORDER BY specified, but %s is not an aggregate function",
+					  NameListToString(funcname)),
+					 parser_errposition(pstate, location)));
 		if (over)
 			ereport(ERROR,
 					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
@@ -372,9 +381,12 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
 
 		aggref->aggfnoid = funcid;
 		aggref->aggtype = rettype;
+		/* args and aggorder will be modified by transformAggregateCall */
 		aggref->args = fargs;
+		aggref->aggorder = agg_order;
+		/* aggdistinct will be set by transformAggregateCall */
 		aggref->aggstar = agg_star;
-		aggref->aggdistinct = agg_distinct;
+		/* agglevelsup will be set by transformAggregateCall */
 		aggref->location = location;
 
 		/*
@@ -407,7 +419,7 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
 					 parser_errposition(pstate, location)));
 
 		/* parse_agg.c does additional aggregate-specific processing */
-		transformAggregateCall(pstate, aggref);
+		transformAggregateCall(pstate, aggref, agg_distinct);
 
 		retval = (Node *) aggref;
 	}
@@ -453,6 +465,15 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
 							NameListToString(funcname)),
 					 parser_errposition(pstate, location)));
 
+		/*
+		 * ordered aggs not allowed in windows yet
+		 */
+		if (agg_order != NIL)
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					 errmsg("aggregate ORDER BY is not implemented for window functions"),
+					 parser_errposition(pstate, location)));
+
 		if (retset)
 			ereport(ERROR,
 					(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),

src/backend/parser/parse_utilcmd.c

@@ -19,7 +19,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- *	$PostgreSQL: pgsql/src/backend/parser/parse_utilcmd.c,v 2.31 2009/12/07 05:22:22 tgl Exp $
+ *	$PostgreSQL: pgsql/src/backend/parser/parse_utilcmd.c,v 2.32 2009/12/15 17:57:47 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -388,6 +388,7 @@ transformColumnDefinition(ParseState *pstate, CreateStmtContext *cxt,
 		funccallnode = makeNode(FuncCall);
 		funccallnode->funcname = SystemFuncName("nextval");
 		funccallnode->args = list_make1(castnode);
+		funccallnode->agg_order = NIL;
 		funccallnode->agg_star = false;
 		funccallnode->agg_distinct = false;
 		funccallnode->func_variadic = false;

src/backend/utils/adt/ruleutils.c

@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.316 2009/12/07 05:22:22 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.317 2009/12/15 17:57:47 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -5442,32 +5442,44 @@ get_agg_expr(Aggref *aggref, deparse_context *context)
 {
 	StringInfo	buf = context->buf;
 	Oid			argtypes[FUNC_MAX_ARGS];
+	List       *arglist;
 	int			nargs;
 	ListCell   *l;
 
-	if (list_length(aggref->args) > FUNC_MAX_ARGS)
-		ereport(ERROR,
-				(errcode(ERRCODE_TOO_MANY_ARGUMENTS),
-				 errmsg("too many arguments")));
+	/* Extract the regular arguments, ignoring resjunk stuff for the moment */
+	arglist = NIL;
 	nargs = 0;
 	foreach(l, aggref->args)
 	{
-		Node   *arg = (Node *) lfirst(l);
+		TargetEntry *tle = (TargetEntry *) lfirst(l);
+		Node   *arg = (Node *) tle->expr;
 
 		Assert(!IsA(arg, NamedArgExpr));
+		if (tle->resjunk)
+			continue;
+		if (nargs >= FUNC_MAX_ARGS)				/* paranoia */
+			ereport(ERROR,
+					(errcode(ERRCODE_TOO_MANY_ARGUMENTS),
+					 errmsg("too many arguments")));
 		argtypes[nargs] = exprType(arg);
+		arglist = lappend(arglist, arg);
 		nargs++;
 	}
 
 	appendStringInfo(buf, "%s(%s",
 					 generate_function_name(aggref->aggfnoid, nargs,
 											NIL, argtypes, NULL),
-					 aggref->aggdistinct ? "DISTINCT " : "");
+					 (aggref->aggdistinct != NIL) ? "DISTINCT " : "");
 	/* aggstar can be set only in zero-argument aggregates */
 	if (aggref->aggstar)
 		appendStringInfoChar(buf, '*');
 	else
-		get_rule_expr((Node *) aggref->args, context, true);
+		get_rule_expr((Node *) arglist, context, true);
+	if (aggref->aggorder != NIL)
+	{
+		appendStringInfoString(buf, " ORDER BY ");
+		get_rule_orderby(aggref->aggorder, aggref->args, false, context);
+	}
 	appendStringInfoChar(buf, ')');
 }
 

src/include/catalog/catversion.h

@@ -37,7 +37,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/catalog/catversion.h,v 1.557 2009/12/11 03:34:56 itagaki Exp $
+ * $PostgreSQL: pgsql/src/include/catalog/catversion.h,v 1.558 2009/12/15 17:57:47 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -53,6 +53,6 @@
  */
 
 /*							yyyymmddN */
-#define CATALOG_VERSION_NO	200912111
+#define CATALOG_VERSION_NO	200912151
 
 #endif

src/include/nodes/parsenodes.h

@@ -13,7 +13,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.418 2009/12/11 03:34:56 itagaki Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.419 2009/12/15 17:57:47 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -264,9 +264,10 @@ typedef struct TypeCast
 /*
  * FuncCall - a function or aggregate invocation
  *
+ * agg_order (if not NIL) indicates we saw 'foo(... ORDER BY ...)'.
  * agg_star indicates we saw a 'foo(*)' construct, while agg_distinct
- * indicates we saw 'foo(DISTINCT ...)'.  In either case, the construct
- * *must* be an aggregate call.  Otherwise, it might be either an
+ * indicates we saw 'foo(DISTINCT ...)'.  In any of these cases, the
+ * construct *must* be an aggregate call.  Otherwise, it might be either an
  * aggregate or some other kind of function.  However, if OVER is present
  * it had better be an aggregate or window function.
  */
@@ -275,6 +276,7 @@ typedef struct FuncCall
 	NodeTag		type;
 	List	   *funcname;		/* qualified name of function */
 	List	   *args;			/* the arguments (list of exprs) */
+	List       *agg_order;      /* ORDER BY (list of SortBy) */
 	bool		agg_star;		/* argument was really '*' */
 	bool		agg_distinct;	/* arguments were labeled DISTINCT */
 	bool		func_variadic;	/* last argument was labeled VARIADIC */

src/include/nodes/primnodes.h

@@ -10,7 +10,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/nodes/primnodes.h,v 1.151 2009/10/08 02:39:24 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/primnodes.h,v 1.152 2009/12/15 17:57:47 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -209,16 +209,29 @@ typedef struct Param
 
 /*
  * Aggref
+ *
+ * The aggregate's args list is a targetlist, ie, a list of TargetEntry nodes
+ * (before Postgres 8.5 it was just bare expressions).  The non-resjunk TLEs
+ * represent the aggregate's regular arguments (if any) and resjunk TLEs can
+ * be added at the end to represent ORDER BY expressions that are not also
+ * arguments.  As in a top-level Query, the TLEs can be marked with
+ * ressortgroupref indexes to let them be referenced by SortGroupClause
+ * entries in the aggorder and/or aggdistinct lists.  This represents ORDER BY
+ * and DISTINCT operations to be applied to the aggregate input rows before
+ * they are passed to the transition function.  The grammar only allows a
+ * simple "DISTINCT" specifier for the arguments, but we use the full
+ * query-level representation to allow more code sharing.
  */
 typedef struct Aggref
 {
 	Expr		xpr;
 	Oid			aggfnoid;		/* pg_proc Oid of the aggregate */
 	Oid			aggtype;		/* type Oid of result of the aggregate */
-	List	   *args;			/* arguments to the aggregate */
-	Index		agglevelsup;	/* > 0 if agg belongs to outer query */
+	List	   *args;			/* arguments and sort expressions */
+	List	   *aggorder;		/* ORDER BY (list of SortGroupClause) */
+	List	   *aggdistinct;	/* DISTINCT (list of SortGroupClause) */
 	bool		aggstar;		/* TRUE if argument list was really '*' */
-	bool		aggdistinct;	/* TRUE if it's agg(DISTINCT ...) */
+	Index		agglevelsup;	/* > 0 if agg belongs to outer query */
 	int			location;		/* token location, or -1 if unknown */
 } Aggref;
 

src/include/optimizer/clauses.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/optimizer/clauses.h,v 1.98 2009/06/11 14:49:11 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/optimizer/clauses.h,v 1.99 2009/12/15 17:57:47 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -23,7 +23,7 @@
 typedef struct
 {
 	int			numAggs;		/* total number of aggregate calls */
-	int			numDistinctAggs;	/* number that use DISTINCT */
+	int			numOrderedAggs; 	/* number that use DISTINCT or ORDER BY */
 	Size		transitionSpace;	/* for pass-by-ref transition data */
 } AggClauseCounts;
 

src/include/parser/parse_agg.h

@@ -6,7 +6,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/parser/parse_agg.h,v 1.39 2009/06/11 14:49:11 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/parser/parse_agg.h,v 1.40 2009/12/15 17:57:47 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -15,7 +15,8 @@
 
 #include "parser/parse_node.h"
 
-extern void transformAggregateCall(ParseState *pstate, Aggref *agg);
+extern void transformAggregateCall(ParseState *pstate, Aggref *agg,
+								   bool agg_distinct);
 extern void transformWindowFuncCall(ParseState *pstate, WindowFunc *wfunc,
 						WindowDef *windef);
 

src/include/parser/parse_clause.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/parser/parse_clause.h,v 1.56 2009/08/27 20:08:02 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/parser/parse_clause.h,v 1.57 2009/12/15 17:57:47 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -28,16 +28,16 @@ extern Node *transformLimitClause(ParseState *pstate, Node *clause,
 					 const char *constructName);
 extern List *transformGroupClause(ParseState *pstate, List *grouplist,
 					 List **targetlist, List *sortClause,
-					 bool isWindowFunc);
+					 bool useSQL99);
 extern List *transformSortClause(ParseState *pstate, List *orderlist,
-					List **targetlist, bool resolveUnknown, bool isWindowFunc);
+					List **targetlist, bool resolveUnknown, bool useSQL99);
 
 extern List *transformWindowDefinitions(ParseState *pstate,
 						   List *windowdefs,
 						   List **targetlist);
 
 extern List *transformDistinctClause(ParseState *pstate,
-						List **targetlist, List *sortClause);
+						List **targetlist, List *sortClause, bool is_agg);
 extern List *transformDistinctOnClause(ParseState *pstate, List *distinctlist,
 						  List **targetlist, List *sortClause);
 

src/include/parser/parse_func.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/parser/parse_func.h,v 1.66 2009/10/08 02:39:25 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/parser/parse_func.h,v 1.67 2009/12/15 17:57:47 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -44,7 +44,8 @@ typedef enum
 
 extern Node *ParseFuncOrColumn(ParseState *pstate,
 				  List *funcname, List *fargs,
-				  bool agg_star, bool agg_distinct, bool func_variadic,
+				  List *agg_order, bool agg_star, bool agg_distinct,
+				  bool func_variadic,
 				  WindowDef *over, bool is_column, int location);
 
 extern FuncDetailCode func_get_detail(List *funcname,

src/test/regress/expected/create_aggregate.out

@@ -32,6 +32,10 @@ CREATE AGGREGATE newcnt ("any") (
    sfunc = int8inc_any, stype = int8,
    initcond = '0'
 );
+COMMENT ON AGGREGATE nosuchagg (*) IS 'should fail';
+ERROR:  aggregate nosuchagg(*) does not exist
+COMMENT ON AGGREGATE newcnt (*) IS 'an agg(*) comment';
+COMMENT ON AGGREGATE newcnt ("any") IS 'an agg(any) comment';
 -- multi-argument aggregate
 create function sum3(int8,int8,int8) returns int8 as
 'select $1 + $2 + $3' language sql strict immutable;
@@ -39,7 +43,19 @@ create aggregate sum2(int8,int8) (
    sfunc = sum3, stype = int8,
    initcond = '0'
 );
-COMMENT ON AGGREGATE nosuchagg (*) IS 'should fail';
-ERROR:  aggregate nosuchagg(*) does not exist
-COMMENT ON AGGREGATE newcnt (*) IS 'an agg(*) comment';
-COMMENT ON AGGREGATE newcnt ("any") IS 'an agg(any) comment';
+-- multi-argument aggregates sensitive to distinct/order, strict/nonstrict
+create type aggtype as (a integer, b integer, c text);
+create function aggf_trans(aggtype[],integer,integer,text) returns aggtype[]
+as 'select array_append($1,ROW($2,$3,$4)::aggtype)'
+language sql strict immutable;
+create function aggfns_trans(aggtype[],integer,integer,text) returns aggtype[]
+as 'select array_append($1,ROW($2,$3,$4)::aggtype)'
+language sql immutable;
+create aggregate aggfstr(integer,integer,text) (
+   sfunc = aggf_trans, stype = aggtype[],
+   initcond = '{}'
+);
+create aggregate aggfns(integer,integer,text) (
+   sfunc = aggfns_trans, stype = aggtype[],
+   initcond = '{}'
+);

src/test/regress/output/misc.source

@@ -571,6 +571,7 @@ SELECT user_relns() AS user_relns
  a_star
  abstime_tbl
  aggtest
+ aggtype
  array_index_op_test
  array_op_test
  arrtest
@@ -668,7 +669,7 @@ SELECT user_relns() AS user_relns
  toyemp
  varchar_tbl
  xacttest
-(101 rows)
+(102 rows)
 
 SELECT name(equipment(hobby_construct(text 'skywalking', text 'mer')));
  name 

src/test/regress/sql/aggregates.sql

@@ -230,3 +230,128 @@ select max(unique2) from tenk1 order by 1;
 select max(unique2) from tenk1 order by max(unique2);
 select max(unique2) from tenk1 order by max(unique2)+1;
 select max(unique2), generate_series(1,3) as g from tenk1 order by g desc;
+
+--
+-- Test combinations of DISTINCT and/or ORDER BY
+--
+
+select array_agg(a order by b)
+  from (values (1,4),(2,3),(3,1),(4,2)) v(a,b);
+select array_agg(a order by a)
+  from (values (1,4),(2,3),(3,1),(4,2)) v(a,b);
+select array_agg(a order by a desc)
+  from (values (1,4),(2,3),(3,1),(4,2)) v(a,b);
+select array_agg(b order by a desc)
+  from (values (1,4),(2,3),(3,1),(4,2)) v(a,b);
+
+select array_agg(distinct a)
+  from (values (1),(2),(1),(3),(null),(2)) v(a);
+select array_agg(distinct a order by a)
+  from (values (1),(2),(1),(3),(null),(2)) v(a);
+select array_agg(distinct a order by a desc)
+  from (values (1),(2),(1),(3),(null),(2)) v(a);
+select array_agg(distinct a order by a desc nulls last)
+  from (values (1),(2),(1),(3),(null),(2)) v(a);
+
+-- multi-arg aggs, strict/nonstrict, distinct/order by
+
+select aggfstr(a,b,c)
+  from (values (1,3,'foo'),(0,null,null),(2,2,'bar'),(3,1,'baz')) v(a,b,c);
+select aggfns(a,b,c)
+  from (values (1,3,'foo'),(0,null,null),(2,2,'bar'),(3,1,'baz')) v(a,b,c);
+
+select aggfstr(distinct a,b,c)
+  from (values (1,3,'foo'),(0,null,null),(2,2,'bar'),(3,1,'baz')) v(a,b,c),
+       generate_series(1,3) i;
+select aggfns(distinct a,b,c)
+  from (values (1,3,'foo'),(0,null,null),(2,2,'bar'),(3,1,'baz')) v(a,b,c),
+       generate_series(1,3) i;
+
+select aggfstr(distinct a,b,c order by b)
+  from (values (1,3,'foo'),(0,null,null),(2,2,'bar'),(3,1,'baz')) v(a,b,c),
+       generate_series(1,3) i;
+select aggfns(distinct a,b,c order by b)
+  from (values (1,3,'foo'),(0,null,null),(2,2,'bar'),(3,1,'baz')) v(a,b,c),
+       generate_series(1,3) i;
+
+-- test specific code paths
+
+select aggfns(distinct a,a,c order by c using ~<~,a)
+  from (values (1,3,'foo'),(0,null,null),(2,2,'bar'),(3,1,'baz')) v(a,b,c),
+       generate_series(1,2) i;
+select aggfns(distinct a,a,c order by c using ~<~)
+  from (values (1,3,'foo'),(0,null,null),(2,2,'bar'),(3,1,'baz')) v(a,b,c),
+       generate_series(1,2) i;
+select aggfns(distinct a,a,c order by a)
+  from (values (1,3,'foo'),(0,null,null),(2,2,'bar'),(3,1,'baz')) v(a,b,c),
+       generate_series(1,2) i;
+select aggfns(distinct a,b,c order by a,c using ~<~,b)
+  from (values (1,3,'foo'),(0,null,null),(2,2,'bar'),(3,1,'baz')) v(a,b,c),
+       generate_series(1,2) i;
+
+-- check node I/O via view creation and usage, also deparsing logic
+
+create view agg_view1 as
+  select aggfns(a,b,c)
+    from (values (1,3,'foo'),(0,null,null),(2,2,'bar'),(3,1,'baz')) v(a,b,c);
+
+select * from agg_view1;
+select pg_get_viewdef('agg_view1'::regclass);
+
+create or replace view agg_view1 as
+  select aggfns(distinct a,b,c)
+    from (values (1,3,'foo'),(0,null,null),(2,2,'bar'),(3,1,'baz')) v(a,b,c),
+         generate_series(1,3) i;
+
+select * from agg_view1;
+select pg_get_viewdef('agg_view1'::regclass);
+
+create or replace view agg_view1 as
+  select aggfns(distinct a,b,c order by b)
+    from (values (1,3,'foo'),(0,null,null),(2,2,'bar'),(3,1,'baz')) v(a,b,c),
+         generate_series(1,3) i;
+
+select * from agg_view1;
+select pg_get_viewdef('agg_view1'::regclass);
+
+create or replace view agg_view1 as
+  select aggfns(a,b,c order by b+1)
+    from (values (1,3,'foo'),(0,null,null),(2,2,'bar'),(3,1,'baz')) v(a,b,c);
+
+select * from agg_view1;
+select pg_get_viewdef('agg_view1'::regclass);
+
+create or replace view agg_view1 as
+  select aggfns(a,a,c order by b)
+    from (values (1,3,'foo'),(0,null,null),(2,2,'bar'),(3,1,'baz')) v(a,b,c);
+
+select * from agg_view1;
+select pg_get_viewdef('agg_view1'::regclass);
+
+create or replace view agg_view1 as
+  select aggfns(a,b,c order by c using ~<~)
+    from (values (1,3,'foo'),(0,null,null),(2,2,'bar'),(3,1,'baz')) v(a,b,c);
+
+select * from agg_view1;
+select pg_get_viewdef('agg_view1'::regclass);
+
+create or replace view agg_view1 as
+  select aggfns(distinct a,b,c order by a,c using ~<~,b)
+    from (values (1,3,'foo'),(0,null,null),(2,2,'bar'),(3,1,'baz')) v(a,b,c),
+         generate_series(1,2) i;
+
+select * from agg_view1;
+select pg_get_viewdef('agg_view1'::regclass);
+
+drop view agg_view1;
+
+-- incorrect DISTINCT usage errors
+
+select aggfns(distinct a,b,c order by i)
+  from (values (1,1,'foo')) v(a,b,c), generate_series(1,2) i;
+select aggfns(distinct a,b,c order by a,b+1)
+  from (values (1,1,'foo')) v(a,b,c), generate_series(1,2) i;
+select aggfns(distinct a,b,c order by a,b,i,c)
+  from (values (1,1,'foo')) v(a,b,c), generate_series(1,2) i;
+select aggfns(distinct a,a,c order by a,b)
+  from (values (1,1,'foo')) v(a,b,c), generate_series(1,2) i;

src/test/regress/sql/create_aggregate.sql

@@ -38,6 +38,10 @@ CREATE AGGREGATE newcnt ("any") (
    initcond = '0'
 );
 
+COMMENT ON AGGREGATE nosuchagg (*) IS 'should fail';
+COMMENT ON AGGREGATE newcnt (*) IS 'an agg(*) comment';
+COMMENT ON AGGREGATE newcnt ("any") IS 'an agg(any) comment';
+
 -- multi-argument aggregate
 create function sum3(int8,int8,int8) returns int8 as
 'select $1 + $2 + $3' language sql strict immutable;
@@ -47,6 +51,23 @@ create aggregate sum2(int8,int8) (
    initcond = '0'
 );
 
-COMMENT ON AGGREGATE nosuchagg (*) IS 'should fail';
-COMMENT ON AGGREGATE newcnt (*) IS 'an agg(*) comment';
-COMMENT ON AGGREGATE newcnt ("any") IS 'an agg(any) comment';
+-- multi-argument aggregates sensitive to distinct/order, strict/nonstrict
+create type aggtype as (a integer, b integer, c text);
+
+create function aggf_trans(aggtype[],integer,integer,text) returns aggtype[]
+as 'select array_append($1,ROW($2,$3,$4)::aggtype)'
+language sql strict immutable;
+
+create function aggfns_trans(aggtype[],integer,integer,text) returns aggtype[]
+as 'select array_append($1,ROW($2,$3,$4)::aggtype)'
+language sql immutable;
+
+create aggregate aggfstr(integer,integer,text) (
+   sfunc = aggf_trans, stype = aggtype[],
+   initcond = '{}'
+);
+
+create aggregate aggfns(integer,integer,text) (
+   sfunc = aggfns_trans, stype = aggtype[],
+   initcond = '{}'
+);