From 0b04ce9bfcfdb6807acd989788727052dd0d76aa Mon Sep 17 00:00:00 2001 From: Ayush Tiwari Date: Thu, 5 Nov 2026 20:35:41 +0530 Subject: [PATCH v1] Fix single-input aggregates with multiple sort keys An aggregate's ORDER BY can list the same argument 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 used a Datum sort whenever there was a single input, which only sorts on the first key. The remaining keys were silently ignored, and with DISTINCT the backend crashed: process_ordered_aggregate_single() called equalfnOne, which is only set up for a single DISTINCT column, and with presorted input ExecEvalPreOrderedDistinctMulti() dereferenced uniqslot, which was only created for multiple inputs. Fix by using the Datum sort only for a single input with a single sort key, and by creating uniqslot whenever there is more than one DISTINCT column. This is the same problem that b715192e52e fixed in nodeSort.c. Author: Ayush Tiwari --- src/backend/executor/execExpr.c | 8 ++++---- src/backend/executor/nodeAgg.c | 16 +++++++++------- src/test/regress/expected/aggregates.out | 19 +++++++++++++++++++ src/test/regress/sql/aggregates.sql | 10 ++++++++++ 4 files changed, 42 insertions(+), 11 deletions(-) diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index fd216c4978d..8877fc02bd7 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. + * 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. + * 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..6a67d6f8888 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,7 +4333,9 @@ build_pertrans_for_aggref(AggStatePerTrans pertrans, &pertrans->inputtypeLen, &pertrans->inputtypeByVal); } - else if (numDistinctCols > 0) + + /* Even a single input can have several DISTINCT columns */ + if (numDistinctCols > 1) { /* we will need an extra slot to store prior values */ pertrans->uniqslot = diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out index 7d07619956f..d660f40899a 100644 --- a/src/test/regress/expected/aggregates.out +++ b/src/test/regress/expected/aggregates.out @@ -2156,6 +2156,25 @@ 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) + +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) + +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..37d82ef345a 100644 --- a/src/test/regress/sql/aggregates.sql +++ b/src/test/regress/sql/aggregates.sql @@ -824,6 +824,16 @@ 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; +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; +reset enable_presorted_aggregate; + -- test a more complex permutation that has previous caused issues select string_agg(distinct 'a', ','), -- 2.34.1