u70b3 opened a new pull request, #23987:
URL: https://github.com/apache/datafusion/pull/23987
## Which issue does this PR close?
- Closes #23894
## Rationale for this change
`datafusion-spark`'s `mod` routes through the shared `try_rem` helper in
`datafusion/spark/src/function/math/modulus.rs`, which has two gaps in its
zero-divisor handling (both verified against Spark 3.5.8 – 4.2.0 in the
issue):
1. **ANSI mode, floating-point divisor.** `try_rem` delegates to Arrow's
`rem`
kernel in ANSI mode, but Arrow only reports division by zero for integer
and
decimal types. Floating-point divisors follow IEEE 754 and quietly produce
`NaN`, while Spark raises `REMAINDER_BY_ZERO` for a zero divisor of any
numeric type:
```sql
set datafusion.execution.enable_ansi_mode = true;
SELECT mod(10.5::float8, 0.0::float8); -- NaN, Spark raises
```
2. **`-0.0` divisor, both modes.** The legacy path nulls out zero divisors
via
`eq(right, 0)`, but Arrow's floating-point comparisons use a total order
in
which `-0.0` is distinct from `0.0`, so a `-0.0` divisor goes
unrecognised.
Spark's `isZero` is a numeric comparison and treats `-0.0` as zero:
```sql
SELECT mod(10.5::float8, -0.0::float8); -- NaN, Spark returns NULL
(legacy)
```
## What changes are included in this PR?
`try_rem` now detects zero divisors itself, mirroring the shape #23898
established for `pmod`:
- A new `is_zero` helper counts `-0.0` as zero for the floating-point types
(via a `negative_zero` companion, same as #23898).
- In ANSI mode, any row with a zero divisor raises `ArrowError::DivideByZero`
— the same error Arrow's `rem` already produces for integers today, so the
message stays uniform across types. The check is masked by the validity of
the dividend because Spark's remainder expressions are null intolerant: a
NULL dividend short-circuits to NULL before the divisor is validated, so
`mod(NULL, 0)` must return NULL rather than raise.
- Both modes substitute NULL for zero divisors before calling Arrow's `rem`,
so the kernel never sees a zero divisor: legacy mode gets NULLs, and ANSI
mode has already raised on the rows that required it.
Note on overlap with #23898: that PR rewrites `pmod` to no longer use
`try_rem` and adds identical `is_zero`/`negative_zero` helpers. This PR is
independent of it — `mod` is fixed either way — but whichever lands second
should dedupe the helpers in a rebase. Until #23898 lands, `pmod` also picks
up the `-0.0` and ANSI floating-point zero-divisor fixes through the shared
helper.
Out of scope: #23897 (reproducing Spark's exact ANSI error text) is a
repository-wide error-message policy question, as noted in that issue.
## Are these changes tested?
Yes:
- New unit tests in `modulus.rs`: ANSI floating-point zero divisor raises;
`-0.0` divisor returns NULL in legacy mode and raises in ANSI mode; a NULL
dividend with a zero divisor returns NULL in ANSI mode (integer and float).
- New sqllogictest cases in `spark/math/mod.slt` covering the same behavior
at
SQL level.
- Verified `cargo test -p datafusion-spark`, the `spark/math/mod.slt` and
`spark/math/pmod.slt` sqllogictests, `./dev/rust_lint.sh`, and the extended
workspace suite (ci profile with
`avro,json,backtrace,extended_tests,recursive_protection,parquet_encryption`)
— all green.
## Are there any user-facing changes?
Only the bug fixes, and only for the Spark `mod`/`pmod` functions: in ANSI
mode a floating-point zero divisor now raises instead of returning NaN, and a
`-0.0` divisor is treated as zero in both modes (NULL in legacy mode, an
error
in ANSI mode), matching Spark. No API changes.
--
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]
