This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion.git
The following commit(s) were added to refs/heads/main by this push:
new 224cc563cc fix: prevent panic and incorrect results for COUNT with
ORDER BY (#24997)
224cc563cc is described below
commit 224cc563cc38617f85a5c22f63265961dde2b57e
Author: Geoffrey Claude <[email protected]>
AuthorDate: Fri Sep 11 14:07:00 2026 +0000
fix: prevent panic and incorrect results for COUNT with ORDER BY (#24997)
## Which issue does this PR close?
- Closes #25055.
## Rationale for this change
`COUNT` does not depend on input order, but it inherited the default
`AggregateOrderSensitivity::HardRequirement`. As detailed in #25055,
this could pass ordering keys to count accumulators as additional
arguments, causing incorrect results or a panic, and could introduce
unnecessary sorting.
## What changes are included in this PR?
Override `Count::order_sensitivity` to return
`AggregateOrderSensitivity::Insensitive`. This makes aggregate ordering
keys ineffective for physical planning: they are excluded from
accumulator inputs and no `SortExec` is required solely for `COUNT`'s
`ORDER BY`.
## What is the testing strategy for this PR?
Regression tests in `aggregate.slt` cover grouped, multi-argument, and
non-grouped counts. The data includes both a null ordering key, which
must not affect the count, and a null counted argument, which must still
be excluded.
A bare global `COUNT(a ORDER BY b)` over the in-memory test table is
folded by `AggregateStatistics` to `num_rows - null_count(a)`, so
`CountAccumulator` never runs. The test uses `a + 0`, which preserves
`a`'s nullness but prevents that fold, ensuring the regression exercises
the accumulator.
An `EXPLAIN` assertion verifies that the count does not require a sort.
## Are there any user-facing changes?
`COUNT(... ORDER BY ...)` returns the correct count without panicking.
Nulls in counted arguments retain their usual behavior; nulls in
ordering keys no longer incorrectly exclude rows.
---
datafusion/functions-aggregate/src/count.rs | 6 +++-
datafusion/sqllogictest/test_files/aggregate.slt | 43 ++++++++++++++++++++++++
2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/datafusion/functions-aggregate/src/count.rs
b/datafusion/functions-aggregate/src/count.rs
index 2086c4c41b..ed2abe8864 100644
--- a/datafusion/functions-aggregate/src/count.rs
+++ b/datafusion/functions-aggregate/src/count.rs
@@ -40,7 +40,7 @@ use datafusion_expr::{
TypeSignature, Volatility, WindowFunctionDefinition,
expr::WindowFunction,
function::{AccumulatorArgs, StateFieldsArgs},
- utils::format_state_name,
+ utils::{AggregateOrderSensitivity, format_state_name},
};
use
datafusion_functions_aggregate_common::aggregate::count_distinct::PrimitiveDistinctCountGroupsAccumulator;
use datafusion_functions_aggregate_common::aggregate::{
@@ -396,6 +396,10 @@ impl AggregateUDFImpl for Count {
ReversedUDAF::Identical
}
+ fn order_sensitivity(&self) -> AggregateOrderSensitivity {
+ AggregateOrderSensitivity::Insensitive
+ }
+
fn default_value(&self, _data_type: &DataType) -> Result<ScalarValue> {
Ok(ScalarValue::Int64(Some(0)))
}
diff --git a/datafusion/sqllogictest/test_files/aggregate.slt
b/datafusion/sqllogictest/test_files/aggregate.slt
index 8eb3e9734d..edc493cfa1 100644
--- a/datafusion/sqllogictest/test_files/aggregate.slt
+++ b/datafusion/sqllogictest/test_files/aggregate.slt
@@ -9883,3 +9883,46 @@ SELECT sum(s) FROM (SELECT sum(column2) OVER () AS s
FROM nested_agg_t);
statement ok
DROP TABLE nested_agg_t;
+
+# Tests for COUNT(... ORDER BY ...)
+statement ok
+CREATE TABLE count_order_by_t (a INT, b INT, c INT) AS VALUES
+(1, NULL, 10),
+(2, 20, 10),
+(3, 30, 20),
+(NULL, 40, 20);
+
+# Single-argument grouped count with ORDER BY
+query II rowsort
+SELECT c, COUNT(a ORDER BY b) FROM count_order_by_t GROUP BY c;
+----
+10 2
+20 1
+
+# Multi-argument count with nullable ORDER BY key
+query I
+SELECT COUNT(a, c ORDER BY b) FROM count_order_by_t;
+----
+3
+
+# Single-argument non-grouped count with nullable ORDER BY key
+# A bare COUNT(a ORDER BY b) is folded to num_rows - null_count(a), bypassing
+# CountAccumulator. a + 0 preserves nullness and forces execution.
+query I
+SELECT COUNT(a + 0 ORDER BY b) FROM count_order_by_t;
+----
+3
+
+# COUNT does not require sorting its input, even with an explicit ORDER BY.
+query TT
+EXPLAIN SELECT COUNT(a + 0 ORDER BY b) FROM count_order_by_t;
+----
+logical_plan
+01)Aggregate: groupBy=[[]], aggr=[[count(CAST(count_order_by_t.a AS Int64) +
Int64(0)) ORDER BY [count_order_by_t.b ASC NULLS LAST]]]
+02)--TableScan: count_order_by_t projection=[a, b]
+physical_plan
+01)AggregateExec: mode=Single, gby=[], aggr=[count(count_order_by_t.a +
Int64(0)) ORDER BY [count_order_by_t.b ASC NULLS LAST]]
+02)--DataSourceExec: partitions=1, partition_sizes=[1]
+
+statement ok
+DROP TABLE count_order_by_t;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
