postgres.git / summary / log / commit / refs
commit edbe7ffdc8afe7f3c12675df6ef614bcd5017b3f
Author: David Rowley <drowley@postgresql.org>
Date: Fri Nov 06 07:18:18 2026 +0000
Fix crash with single-input aggregates with multiple sort keys
An aggregate's ORDER BY can list the same sort key more than once with
different sort operators, and transformDistinctClause() copies such
items into the DISTINCT list. So an aggregate with a single input can
have several sort keys and several DISTINCT columns.
nodeAgg.c and execExpr.c mistakenly only checked the number of inputs
when deciding if a Datum sort was possible. That check must also
consider the number of sort keys.
Fix by using the Datum sort only for a single input with a single sort
key.
Also for v16+, fix crash in pre-sorted aggregate code where the uniqslot
was only created when the number of inputs for a DISTINCT aggregate was
above 1. The uniqslot is required when there are multiple DISTINCT
columns, and an ORDER BY clause may cause more than one of these to exist
when there's only a single input column.
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/CAJTYsWXMuo5QZSy_az+86mty2XdbDw_7uB3C4FjWpFACvD_rzg@mail.gmail.com
Backpatch-through: 14
src/backend/executor/execExpr.c | 8 ++++----
src/backend/executor/nodeAgg.c | 22 ++++++++++++++--------
src/test/regress/expected/aggregates.out | 28 ++++++++++++++++++++++++++++
src/test/regress/sql/aggregates.sql | 17 +++++++++++++++++
4 files changed, 63 insertions(+), 12 deletions(-)
diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index fd216c4978d..7906f09da4b 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -3865,11 +3865,11 @@ ExecBuildAggTrans(AggState *aggstate, AggStatePerPhase phase,
}
Assert(pertrans->numTransInputs == argno);
}
- else if (pertrans->numInputs == 1)
+ else if (pertrans->numInputs == 1 && pertrans->numSortCols == 1)
{
/*
* Non-presorted DISTINCT and/or ORDER BY case, with a single
- * column sorted on.
+ * input column sorted on a single key.
*/
TargetEntry *source_tle =
(TargetEntry *) linitial(pertrans->aggref->args);
@@ -3888,7 +3888,7 @@ ExecBuildAggTrans(AggState *aggstate, AggStatePerPhase phase,
{
/*
* Non-presorted DISTINCT and/or ORDER BY case, with multiple
- * columns sorted on.
+ * input columns or sort keys.
*/
Datum *values = pertrans->sortslot->tts_values;
bool *nulls = pertrans->sortslot->tts_isnull;
@@ -4114,7 +4114,7 @@ ExecBuildAggTransCall(ExprState *state, AggState *aggstate,
scratch->opcode = EEOP_AGG_PLAIN_TRANS_BYREF;
}
}
- else if (pertrans->numInputs == 1)
+ else if (pertrans->numInputs == 1 && pertrans->numSortCols == 1)
scratch->opcode = EEOP_AGG_ORDERED_TRANS_DATUM;
else
scratch->opcode = EEOP_AGG_ORDERED_TRANS_TUPLE;
diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index e0c76c31706..16c0692ed5e 100644
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -595,11 +595,11 @@ initialize_aggregate(AggState *aggstate, AggStatePerTrans pertrans,
/*
- * We use a plain Datum sorter when there's a single input column;
- * otherwise sort the full tuple. (See comments for
- * process_ordered_aggregate_single.)
+ * We use a plain Datum sorter when there's a single input column and
+ * a single sort key; otherwise sort the full tuple. (See comments
+ * for process_ordered_aggregate_single.)
*/
- if (pertrans->numInputs == 1)
+ if (pertrans->numInputs == 1 && pertrans->numSortCols == 1)
{
Form_pg_attribute attr = TupleDescAttr(pertrans->sortdesc, 0);
@@ -935,7 +935,7 @@ process_ordered_aggregate_single(AggState *aggstate,
/*
* Run the transition function for a DISTINCT or ORDER BY aggregate
- * with more than one input. This is called after we have completed
+ * with multiple inputs or sort keys. This is called after we have completed
* entering all the input values into the sort object. We complete the
* sort, read out the values in sorted order, and run the transition
* function on each value (applying DISTINCT if appropriate).
@@ -1329,7 +1329,7 @@ finalize_aggregates(AggState *aggstate,
Assert(aggstate->aggstrategy != AGG_HASHED &&
aggstate->aggstrategy != AGG_MIXED);
- if (pertrans->numInputs == 1)
+ if (pertrans->numInputs == 1 && pertrans->numSortCols == 1)
process_ordered_aggregate_single(aggstate,
pertrans,
pergroupstate);
@@ -4333,9 +4333,15 @@ build_pertrans_for_aggref(AggStatePerTrans pertrans,
&pertrans->inputtypeLen,
&pertrans->inputtypeByVal);
}
- else if (numDistinctCols > 0)
+
+ if (numDistinctCols > 1)
{
- /* we will need an extra slot to store prior values */
+ /*
+ * With more than one DISTINCT column we'll need an extra slot to
+ * store the prior values. Note that this can happen even with a
+ * single input, since the aggregate's ORDER BY may list the same
+ * argument more than once with different sort operators.
+ */
pertrans->uniqslot =
ExecInitExtraTupleSlot(estate, pertrans->sortdesc,
&TTSOpsMinimalTuple);
diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out
index 7d07619956f..ce555a31693 100644
--- a/src/test/regress/expected/aggregates.out
+++ b/src/test/regress/expected/aggregates.out
@@ -2156,6 +2156,34 @@ select aggfns(distinct a,b,c order by a,c using ~<~,b)
{"(0,,)","(1,3,foo)","(2,2,bar)","(3,1,baz)"}
(1 row)
+-- single input sorted on two keys; 0 and -0 differ only for *<
+select array_agg(distinct r order by r using <, r using *<)
+ from (values (row(-0.0::float8)), (row(0.0::float8))) v(r),
+ generate_series(1,2) i;
+ array_agg
+------------
+ {(0),(-0)}
+(1 row)
+
+-- check again for the non-presorted aggregate code
+set enable_presorted_aggregate to off;
+select array_agg(distinct r order by r using <, r using *<)
+ from (values (row(-0.0::float8)), (row(0.0::float8))) v(r),
+ generate_series(1,2) i;
+ array_agg
+------------
+ {(0),(-0)}
+(1 row)
+
+-- likewise, but without DISTINCT; the second sort key must not be ignored
+select array_agg(r order by r using <, r using *<)
+ from (values (row(-0.0::float8)), (row(0.0::float8))) v(r);
+ array_agg
+------------
+ {(0),(-0)}
+(1 row)
+
+reset enable_presorted_aggregate;
-- test a more complex permutation that has previous caused issues
select
string_agg(distinct 'a', ','),
diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sql
index 91f8342166f..0936088b074 100644
--- a/src/test/regress/sql/aggregates.sql
+++ b/src/test/regress/sql/aggregates.sql
@@ -824,6 +824,23 @@ 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;
+-- single input sorted on two keys; 0 and -0 differ only for *<
+select array_agg(distinct r order by r using <, r using *<)
+ from (values (row(-0.0::float8)), (row(0.0::float8))) v(r),
+ generate_series(1,2) i;
+
+-- check again for the non-presorted aggregate code
+set enable_presorted_aggregate to off;
+select array_agg(distinct r order by r using <, r using *<)
+ from (values (row(-0.0::float8)), (row(0.0::float8))) v(r),
+ generate_series(1,2) i;
+
+-- likewise, but without DISTINCT; the second sort key must not be ignored
+select array_agg(r order by r using <, r using *<)
+ from (values (row(-0.0::float8)), (row(0.0::float8))) v(r);
+
+reset enable_presorted_aggregate;
+
-- test a more complex permutation that has previous caused issues
select
string_agg(distinct 'a', ','),
[parent: f8cabd1f175f]