DeviousCardi opened a new pull request, #6732:
URL: https://github.com/apache/datafusion-comet/pull/6732
## Which issue does this PR close?
Closes #6661.
## Rationale for this change
Spark's integral `SUM` returns `LONG` and adds through `Add` in both its
update and merge expressions, so in ANSI mode an overflow is reported as `long
overflow` with the `Use 'try_add' to tolerate overflow and return NULL
instead.` suggestion. Comet's `SumInteger` raised `integer overflow` with no
suggestion. #6249 added `function_name` to `ArithmeticOverflow` and the
JVM-side conversion already passes it through, but `SUM` was never switched
over.
## What changes are included in this PR?
- `native/spark-expr/src/agg_funcs/sum_int.rs`: a small
`sum_overflow_error()` helper builds `ArithmeticOverflow { from_type: "long",
function_name: "try_add" }`, used at all three overflow sites (ungrouped
update/merge, grouped update, grouped merge).
- `CometAggregateSuite` "ANSI support - SUM function": instead of only
checking that both messages contain `ARITHMETIC_OVERFLOW`, it now compares
Comet's exception with Spark's (class, error class, SQL state, message
parameters), following the approach from #6249. Adds a case where each of two
scan partitions holds one row, so the overflow only happens when the partial
sums are merged, with and without `GROUP BY`. The test asserts the
per-partition row counts are `[1, 1]` so it cannot silently pass through the
update path.
## How are these changes tested?
- New Rust unit test in `sum_int.rs` covering update and merge, grouped and
ungrouped, overflow and underflow, checking the error class, parameters and
message.
- `cargo test -p datafusion-comet-spark-expr`, `cargo clippy --all-targets
-- -D warnings`, `cargo fmt --all -- --check`.
- `CometAggregateSuite` "ANSI support - SUM function" on the default profile
(Spark 4.1) and `-Pspark-3.5 -Pscala-2.12`; `spotless` and the CI `scalafix`
check.
- Verified the merge case reaches the grouped-merge site: restoring the old
error only there makes the test fail with `"integer overflow"` vs `"long
overflow"`.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
