Skip to content

Commit 58e37a0

Browse files
authored
Clearly gate sliding SUM(DISTINCT) type support (#22866)
## Which issue does this PR close? <!-- We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123. --> - Closes #22820. ## Rationale for this change Sliding `SUM(DISTINCT)` only supports `Int64`, but it was routed through the wider `SUM` type dispatch path. This made unsupported types fail with a less clear accumulator error. <!-- Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed. Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes. --> ## What changes are included in this PR? This PR adds an explicit `Int64` gate for sliding `SUM(DISTINCT)`. Unsupported types now return a clear feature error that names the operation and type. The existing `Int64` path is unchanged. <!-- There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR. --> ## Are these changes tested? Yes <!-- We typically require tests for all PRs in order to: 1. Prevent the code from being accidentally broken by subsequent changes 2. Serve as another way to document the expected behavior of the code If tests are not included in your PR, please explain why (for example, are they covered by existing tests)? --> ## Are there any user-facing changes? No public API change <!-- If there are user-facing changes then we may require documentation to be updated before approving the PR. --> <!-- If there are any breaking changes to public APIs, please add the `api change` label. -->
1 parent 3bece3d commit 58e37a0

2 files changed

Lines changed: 49 additions & 7 deletions

File tree

datafusion/functions-aggregate/src/sum.rs

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -303,13 +303,18 @@ impl AggregateUDFImpl for Sum {
303303
args: AccumulatorArgs,
304304
) -> Result<Box<dyn Accumulator>> {
305305
if args.is_distinct {
306-
// distinct path: use our sliding‐window distinct‐sum
307-
macro_rules! helper_distinct {
308-
($t:ty, $dt:expr) => {
309-
Ok(Box::new(SlidingDistinctSumAccumulator::try_new(&$dt)?))
310-
};
306+
// distinct path: [`SlidingDistinctSumAccumulator`] only implements
307+
// Int64, so gate the supported type here rather than dispatching
308+
// through `downcast_sum!`, which accepts every SUM type
309+
match args.return_field.data_type() {
310+
DataType::Int64 => Ok(Box::new(SlidingDistinctSumAccumulator::try_new(
311+
&DataType::Int64,
312+
)?)),
313+
_ => not_impl_err!(
314+
"SUM(DISTINCT) over sliding window frames is only supported for Int64, got {}",
315+
args.expr_fields[0].data_type()
316+
),
311317
}
312-
downcast_sum!(args, helper_distinct)
313318
} else {
314319
// non‐distinct path: existing sliding sum
315320
macro_rules! helper {
@@ -525,7 +530,9 @@ impl SlidingDistinctSumAccumulator {
525530
pub fn try_new(data_type: &DataType) -> Result<Self> {
526531
// TODO support other numeric types
527532
if *data_type != DataType::Int64 {
528-
return exec_err!("SlidingDistinctSumAccumulator only supports Int64");
533+
return exec_err!(
534+
"SlidingDistinctSumAccumulator only supports Int64, got {data_type}"
535+
);
529536
}
530537
Ok(Self {
531538
counts: HashMap::default(),

datafusion/sqllogictest/test_files/window.slt

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5981,6 +5981,41 @@ FROM table_distinct_sum_nulls;
59815981
5 5
59825982

59835983

5984+
# SUM(DISTINCT) over sliding (bounded) window frames is only implemented
5985+
# for Int64. Other SUM-supported input types must fail with a clear
5986+
# capability error instead of an accumulator-internal one.
5987+
statement ok
5988+
CREATE TABLE table_distinct_sum_types(ts INT, f DOUBLE, d DECIMAL(10, 2)) AS VALUES
5989+
(1, 1.5, 1.50), (2, 2.5, 2.50), (3, 1.5, 1.50);
5990+
5991+
query error DataFusion error: This feature is not implemented: SUM\(DISTINCT\) over sliding window frames is only supported for Int64, got Float64
5992+
SELECT SUM(DISTINCT f) OVER (
5993+
ORDER BY ts ROWS BETWEEN 1 PRECEDING AND CURRENT ROW
5994+
) FROM table_distinct_sum_types;
5995+
5996+
query error DataFusion error: This feature is not implemented: SUM\(DISTINCT\) over sliding window frames is only supported for Int64, got Decimal128\(10, 2\)
5997+
SELECT SUM(DISTINCT d) OVER (
5998+
ORDER BY ts ROWS BETWEEN 1 PRECEDING AND CURRENT ROW
5999+
) FROM table_distinct_sum_types;
6000+
6001+
query error DataFusion error: This feature is not implemented: SUM\(DISTINCT\) over sliding window frames is only supported for Int64, got UInt64
6002+
SELECT SUM(DISTINCT arrow_cast(ts, 'UInt64')) OVER (
6003+
ORDER BY ts ROWS BETWEEN 1 PRECEDING AND CURRENT ROW
6004+
) FROM table_distinct_sum_types;
6005+
6006+
# Unbounded frames take the regular distinct-sum path and keep
6007+
# supporting all SUM input types.
6008+
query R
6009+
SELECT SUM(DISTINCT f) OVER (ORDER BY ts) FROM table_distinct_sum_types;
6010+
----
6011+
1.5
6012+
4
6013+
4
6014+
6015+
statement ok
6016+
DROP TABLE table_distinct_sum_types;
6017+
6018+
59846019
# FILTER clause with window functions
59856020

59866021
# Verify FILTER clause with non-aggregate window functions fails with a clear message

0 commit comments

Comments
 (0)