andygrove opened a new issue, #6661:
URL: https://github.com/apache/datafusion-comet/issues/6661
### Describe the bug
Under `spark.sql.ansi.enabled=true`, an integral `SUM` overflow in Comet
raises `ARITHMETIC_OVERFLOW` with the message parameter `integer overflow` and
an empty `alternative`. Spark raises `long overflow` with the suggestion `Use
'try_add' to tolerate overflow and return NULL instead.`
Spark's `SUM` over integral input always returns `LONG`, and `Sum` adds
through `Add(left, right, evalContext)` in both its update and merge
expressions. So the overflow goes through `MathUtils.addExact(Long, Long,
context)` with the `try_add` hint, the same on 3.4.3 through 4.2.0. Comet's
`native/spark-expr/src/agg_funcs/sum_int.rs` raises
`arithmetic_overflow_error("integer")` at its three overflow sites: the
ungrouped update (which the ungrouped merge reuses), the grouped update, and
the grouped merge.
Comet's decision to throw matches Spark. Only the message parameters differ,
which matters to code that inspects `getMessageParameters()` or the message
text.
This was part of item 2 in #5071, which also named `SumInteger`. #6217
assumed the rest of #5071 had been fixed, and #6249 fixes the binary `+`, `-`
and `*` kernels but not `SUM`. Once #6249 removes the #6217 bullet, the
compatibility guide no longer lists this divergence.
### Steps to reproduce
```sql
SET spark.sql.ansi.enabled=true;
CREATE TABLE t (l BIGINT) USING parquet;
INSERT INTO t SELECT CASE WHEN id = 0 THEN 9223372036854775807 ELSE 1 END
FROM range(0, 3, 1, 1);
SELECT SUM(l) FROM t;
-- Spark: [ARITHMETIC_OVERFLOW] long overflow. Use 'try_add' to tolerate
overflow and return NULL instead. If necessary set "spark.sql.ansi.enabled" to
"false" to bypass this error.
-- Comet: [ARITHMETIC_OVERFLOW] integer overflow. If necessary set
"spark.sql.ansi.enabled" to "false" to bypass this error.
```
The final merge diverges the same way when each of two partitions holds part
of the overflowing sum.
### Expected behavior
The `message` parameter is `long overflow` and `alternative` carries the
`try_add` suggestion, as in Spark.
### Additional context
Reproduced locally on Spark 3.4 and 4.1 for both the partial and the merge
path. With the `function_name` field that #6249 adds to
`SparkError::ArithmeticOverflow`, raising `ArithmeticOverflow { from_type:
"long", function_name: "try_add" }` at the three call sites made Comet match
Spark's error class, parameters and message on both paths.
The existing `ANSI support - SUM function` test in `CometAggregateSuite`
only checks that both messages contain `ARITHMETIC_OVERFLOW`, so it passes
either way. It should compare the message parameters with Spark's.
--
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]
