Fix ORDER BY NULLS FIRST in an aggregate's ORDER BY.

When we merged with PostgreSQL 8.3, and got support for NULLS FIRST, we
missed or stubbed out some code, that is required to honor NULLS FIRST in an
aggregate's ORDER BY, e.g. "select array_agg(a order by b desc nulls last)"

src/backend/executor/nodeAgg.c

@@ -249,20 +249,19 @@ initialize_aggregates(AggState *aggstate,
 					peraggstate->sortstate =
 						tuplesort_begin_datum_mk(&aggstate->ss,
 												 peraggstate->evaldesc->attrs[0]->atttypid,
-												 peraggstate->sortOperators[0], false,
+												 peraggstate->sortOperators[0],
+												 peraggstate->sortNullsFirst[0],
 												 PlanStateOperatorMemKB((PlanState *) aggstate), false);
 				}
 				else
 				{
-					bool	   *nullsFirstFlags = palloc0(peraggstate->numSortCols * sizeof(bool));
-
 					peraggstate->sortstate =
 						tuplesort_begin_heap_mk(&aggstate->ss,
 												peraggstate->evaldesc,
 												peraggstate->numSortCols, peraggstate->sortColIdx,
-												peraggstate->sortOperators, nullsFirstFlags,
+												peraggstate->sortOperators,
+												peraggstate->sortNullsFirst,
 												PlanStateOperatorMemKB((PlanState *) aggstate), false);
-					pfree(nullsFirstFlags);
 				}
 
 				/* 
@@ -288,17 +287,17 @@ initialize_aggregates(AggState *aggstate,
 				{
 					peraggstate->sortstate =
 						tuplesort_begin_datum(peraggstate->evaldesc->attrs[0]->atttypid,
-											  peraggstate->sortOperators[0], false,
+											  peraggstate->sortOperators[0],
+											  peraggstate->sortNullsFirst[0],
 											  PlanStateOperatorMemKB((PlanState *) aggstate), false);
 				}
 				else
 				{
-					bool	   *nullsFirstFlags = palloc0(peraggstate->numSortCols * sizeof(bool));
-
 					peraggstate->sortstate =
 						tuplesort_begin_heap(peraggstate->evaldesc,
 											 peraggstate->numSortCols, peraggstate->sortColIdx,
-											 peraggstate->sortOperators, nullsFirstFlags,
+											 peraggstate->sortOperators,
+											 peraggstate->sortNullsFirst,
 											 PlanStateOperatorMemKB((PlanState *) aggstate), false);
 				}
 
@@ -2193,6 +2192,8 @@ ExecInitAgg(Agg *node, EState *estate, int eflags)
 				(AttrNumber *) palloc(numSortCols * sizeof(AttrNumber));
 			peraggstate->sortOperators =
 				(Oid *) palloc(numSortCols * sizeof(Oid));
+			peraggstate->sortNullsFirst =
+				(bool *) palloc(numSortCols * sizeof(bool));
 
 			i = 0;
 			foreach(lc, sortlist)
@@ -2206,6 +2207,7 @@ ExecInitAgg(Agg *node, EState *estate, int eflags)
 
 				peraggstate->sortColIdx[i] = tle->resno;
 				peraggstate->sortOperators[i] = sortcl->sortop;
+				peraggstate->sortNullsFirst[i] = sortcl->nulls_first;
 				i++;
 			}
 			Assert(i == numSortCols);

src/backend/utils/sort/tuplesort_mk.c

@@ -950,6 +950,7 @@ tuplesort_begin_datum_mk(ScanState *ss,
 
 	state->sortOperator = sortOperator;
 	state->cmpScanKey = NULL;
+	state->nullfirst = nullsFirstFlag;
 	create_mksort_context(
 						  &state->mkctxt,
 						  1, NULL,

src/include/executor/nodeAgg.h

@@ -81,6 +81,7 @@ typedef struct AggStatePerAggData
 	/* deconstructed sorting information (arrays of length numSortCols) */
 	AttrNumber *sortColIdx;
 	Oid		   *sortOperators;
+	bool	   *sortNullsFirst;
 
 	/* --- Ordered Aggregate Additions ) --- */
 

src/test/regress/expected/bfv_aggregate.out

@@ -1486,6 +1486,58 @@ SELECT substr(a, 1) as a FROM (SELECT ('-'||a)::varchar as a FROM (SELECT a FROM
  -bbbbbbb
 (3 rows)
 
+-- Check that ORDER BY NULLS FIRST/LAST in an aggregate is respected (these are
+-- variants of similar query in PostgreSQL's aggregates test)
+create temporary table aggordertest (a int4, b int4);
+insert into aggordertest values (1,1), (2,2), (1,3), (3,4), (null,5), (2,null);
+select array_agg(a order by a nulls first) from aggordertest;
+    array_agg     
+------------------
+ {NULL,1,1,2,2,3}
+(1 row)
+
+select array_agg(a order by a nulls last) from aggordertest;
+    array_agg     
+------------------
+ {1,1,2,2,3,NULL}
+(1 row)
+
+select array_agg(a order by a desc nulls first) from aggordertest;
+    array_agg     
+------------------
+ {NULL,3,2,2,1,1}
+(1 row)
+
+select array_agg(a order by a desc nulls last) from aggordertest;
+    array_agg     
+------------------
+ {3,2,2,1,1,NULL}
+(1 row)
+
+select array_agg(a order by b nulls first) from aggordertest;
+    array_agg     
+------------------
+ {2,1,2,1,3,NULL}
+(1 row)
+
+select array_agg(a order by b nulls last) from aggordertest;
+    array_agg     
+------------------
+ {1,2,1,3,NULL,2}
+(1 row)
+
+select array_agg(a order by b desc nulls first) from aggordertest;
+    array_agg     
+------------------
+ {2,NULL,3,1,2,1}
+(1 row)
+
+select array_agg(a order by b desc nulls last) from aggordertest;
+    array_agg     
+------------------
+ {NULL,3,1,2,1,2}
+(1 row)
+
 -- CLEANUP
 set client_min_messages='warning';
 drop schema bfv_aggregate cascade;

src/test/regress/expected/bfv_aggregate_optimizer.out

@@ -1485,6 +1485,58 @@ SELECT substr(a, 1) as a FROM (SELECT ('-'||a)::varchar as a FROM (SELECT a FROM
  -bbbbbbb
 (3 rows)
 
+-- Check that ORDER BY NULLS FIRST/LAST in an aggregate is respected (these are
+-- variants of similar query in PostgreSQL's aggregates test)
+create temporary table aggordertest (a int4, b int4);
+insert into aggordertest values (1,1), (2,2), (1,3), (3,4), (null,5), (2,null);
+select array_agg(a order by a nulls first) from aggordertest;
+    array_agg     
+------------------
+ {NULL,1,1,2,2,3}
+(1 row)
+
+select array_agg(a order by a nulls last) from aggordertest;
+    array_agg     
+------------------
+ {1,1,2,2,3,NULL}
+(1 row)
+
+select array_agg(a order by a desc nulls first) from aggordertest;
+    array_agg     
+------------------
+ {NULL,3,2,2,1,1}
+(1 row)
+
+select array_agg(a order by a desc nulls last) from aggordertest;
+    array_agg     
+------------------
+ {3,2,2,1,1,NULL}
+(1 row)
+
+select array_agg(a order by b nulls first) from aggordertest;
+    array_agg     
+------------------
+ {2,1,2,1,3,NULL}
+(1 row)
+
+select array_agg(a order by b nulls last) from aggordertest;
+    array_agg     
+------------------
+ {1,2,1,3,NULL,2}
+(1 row)
+
+select array_agg(a order by b desc nulls first) from aggordertest;
+    array_agg     
+------------------
+ {2,NULL,3,1,2,1}
+(1 row)
+
+select array_agg(a order by b desc nulls last) from aggordertest;
+    array_agg     
+------------------
+ {NULL,3,1,2,1,2}
+(1 row)
+
 -- CLEANUP
 set client_min_messages='warning';
 drop schema bfv_aggregate cascade;

src/test/regress/sql/bfv_aggregate.sql

@@ -1362,6 +1362,22 @@ INSERT INTO t1 VALUES ('bbbbbbb');
 INSERT INTO t1 VALUES ('bbbbb');
 SELECT substr(a, 1) as a FROM (SELECT ('-'||a)::varchar as a FROM (SELECT a FROM t1) t2) t3 GROUP BY a ORDER BY a;
 
+
+-- Check that ORDER BY NULLS FIRST/LAST in an aggregate is respected (these are
+-- variants of similar query in PostgreSQL's aggregates test)
+create temporary table aggordertest (a int4, b int4);
+insert into aggordertest values (1,1), (2,2), (1,3), (3,4), (null,5), (2,null);
+
+select array_agg(a order by a nulls first) from aggordertest;
+select array_agg(a order by a nulls last) from aggordertest;
+select array_agg(a order by a desc nulls first) from aggordertest;
+select array_agg(a order by a desc nulls last) from aggordertest;
+select array_agg(a order by b nulls first) from aggordertest;
+select array_agg(a order by b nulls last) from aggordertest;
+select array_agg(a order by b desc nulls first) from aggordertest;
+select array_agg(a order by b desc nulls last) from aggordertest;
+
+
 -- CLEANUP
 set client_min_messages='warning';
 drop schema bfv_aggregate cascade;