andygrove opened a new pull request, #5084:
URL: https://github.com/apache/datafusion-comet/pull/5084
## Which issue does this PR close?
Closes #5065.
## Rationale for this change
Under ANSI mode, Spark throws `ARITHMETIC_OVERFLOW` ("Overflow in integral
divide") for `Long.MinValue div -1`, since the quotient `9223372036854775808`
cannot be represented as a `LONG`. Comet performs integral division by casting
both operands to `DECIMAL(19,0)`, dividing, and casting the result back to
`LONG` with legacy (wrapping) semantics, so it silently returned
`-9223372036854775808` instead of throwing.
Spark only performs this check when the operands are `LONG`
(`checkDivideOverflow` in `arithmetic.scala`): `DECIMAL` operands wrap around
on the implicit cast to `LONG` even in ANSI mode, and smaller integer types
cannot overflow because the result type is `LONG`. The fix therefore has to
know the original operand type, which is no longer visible on the native side
after the serde casts both operands to decimal.
## What changes are included in this PR?
- Add a `check_divide_overflow` field to the `MathExpr` protobuf message,
set by `CometIntegralDivide` only when the left operand is `LONG` and eval mode
is ANSI (mirroring Spark's `checkDivideOverflow`)
- Plumb the flag through `IntegralDivideBuilder` / `BinaryExprOptions` into
the existing `fail_on_error` parameter of
`create_comet_physical_fun_with_eval_mode`, with a new `make_comet_scalar_udf!`
arm that passes both `eval_mode` and `fail_on_error`
- In `spark_decimal_div_internal`, when the flag is set and the quotient
does not fit in an `i64`, return the previously never-constructed
`SparkError::IntegralDivideOverflow`, which already maps to
`SparkArithmeticException` with error class `ARITHMETIC_OVERFLOW`
- Align the `IntegralDivideOverflow` message with Spark's wording (`Use
'try_divide' ...` with single quotes rather than backticks, matching Spark 3.4
through 4.1)
## How are these changes tested?
New cases in `expressions/math/arithmetic_ansi.sql` (verified to fail before
the fix):
- `a div b` and literal `div` for `Long.MinValue div -1` expect an error
matching `Overflow in integral divide` from both Spark and Comet
- Pinning queries for the cases that must NOT throw: `INT_MIN div -1`,
`LONG_MIN div 1`, and `DECIMAL(19,0)` operands (which wrap around even in ANSI
mode, matching Spark)
Also ran the full `CometSqlFileTestSuite` (427 tests) and the `div`-related
tests in `CometExpressionSuite`, plus `cargo clippy --all-targets --workspace
-- -D warnings`.
--
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]
