andygrove opened a new pull request, #5156: URL: https://github.com/apache/datafusion-comet/pull/5156
## Which issue does this PR close? Backport of #5081 to `branch-1.0`. Original issue: #5067. ## Rationale for this change Under ANSI mode, `Remainder` on Float/Double operands returned `NaN` for a zero divisor instead of throwing, because Comet's native modulo implementation relies on Arrow's `rem` kernel to signal the error. `arrow-arith` only produces `DivideByZero` for integer and decimal types; for floats `Op::Rem` uses `mod_wrapping`, which returns `NaN` for `x % 0.0` and never errors, so the existing error branch was unreachable for Float32/Float64. Spark 4.1 raises `REMAINDER_BY_ZERO` (and earlier Spark versions raise `DIVIDE_BY_ZERO`) for all numeric types including floating point, so this was a native correctness gap. ## What changes are included in this PR? Clean cherry-pick of 8782d9533 (no conflicts): - Under ANSI mode, `spark_modulo` no longer routes Float32/Float64 through the `rem` kernel. It computes the remainder with `arrow::compute::kernels::arity::try_binary` and `ArrowNativeTypeOp::mod_checked` instead, the same route `checked_arithmetic.rs` already takes for the equivalent float divide-by-zero problem. `mod_checked` reports `DivideByZero` for a zero divisor (including `-0.0`, via IEEE 754 equality), and `try_binary` only invokes the closure on rows valid in both operands, so null rows produce null without raising. - Only `ArrowError::DivideByZero` maps to `remainder_by_zero_error()`; other errors pass through. - Scalar/scalar operands still evaluate to a `ColumnarValue::Scalar`, preserving the shape DataFusion's `apply` produced. Non-ANSI mode is unchanged: `create_modulo_expr` still rewrites the divisor so zeros become `NULL`, and `rem`/`mod_wrapping` handles the arithmetic. ## How are these changes tested? Tests come with the cherry-pick. Verified locally on this branch: - `cargo test -p datafusion-comet-spark-expr modulo` — 25 tests in `modulo_expr`, all passing. Covers Float32/Float64 in ANSI and non-ANSI mode across `Array/Array`, `Array/Scalar`, `Scalar/Array` and `Scalar/Scalar` operands, including zero and `-0.0` divisors, NaN/Infinity dividends, null dividends and divisors, and mixed-row batches. - `./mvnw test -Dsuites="org.apache.comet.CometSqlFileTestSuite arithmetic_ansi" -Dtest=none` — passing (Spark 4.1 / Scala 2.13). -- 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]
