This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-24997-517ac6d53133621c99a9a7dda470549a26348f6a in repository https://gitbox.apache.org/repos/asf/datafusion.git
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]
