fix(spark): correct mod ANSI zero divisor and negative zero handling - #23987
Open
u70b3 wants to merge 1 commit into
Open
fix(spark): correct mod ANSI zero divisor and negative zero handling#23987u70b3 wants to merge 1 commit into
u70b3 wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #23987 +/- ##
=======================================
Coverage 80.87% 80.87%
=======================================
Files 1101 1101
Lines 375765 375837 +72
Branches 375765 375837 +72
=======================================
+ Hits 303915 303974 +59
- Misses 53747 53751 +4
- Partials 18103 18112 +9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
u70b3
force-pushed
the
fix/spark-mod-zero-divisor
branch
from
July 31, 2026 11:31
115cecb to
612ea63
Compare
Contributor
Contributor
Author
|
Thanks for checking, @kumarUjjawal. There is some implementation overlap, but the two PRs fix different functions. #23898 is scoped to pmod: it rewrites spark_pmod so that it no longer uses try_rem, while leaving mod behavior unchanged. This PR fixes mod by correcting try_rem and adds coverage in mod.slt, closing #23894. The shared overlap is the negative_zero/is_zero helpers. I will rebase and deduplicate those helpers after whichever PR lands first. |
In ANSI mode a floating-point zero divisor quietly returned NaN because Arrow's rem only reports division by zero for integer and decimal types, and a -0.0 divisor went unrecognised in both modes because Arrow's floating-point comparisons order totally, so -0.0 is distinct from 0.0. try_rem now detects zero divisors itself via an is_zero helper that counts -0.0 as zero, raises for a zero divisor of any numeric type in ANSI mode (masked by the validity of the dividend, since Spark's remainder is null intolerant), and substitutes NULL for zero divisors before calling Arrow's rem so the kernel never sees one. Closes apache#23894.
u70b3
force-pushed
the
fix/spark-mod-zero-divisor
branch
from
August 3, 2026 17:01
612ea63 to
f3c194b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Rationale for this change
datafusion-spark'smodroutes through the sharedtry_remhelper indatafusion/spark/src/function/math/modulus.rs, which has two gaps in itszero-divisor handling (both verified against Spark 3.5.8 – 4.2.0 in the issue):
ANSI mode, floating-point divisor.
try_remdelegates to Arrow'sremkernel 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 raisesREMAINDER_BY_ZEROfor a zero divisor of anynumeric type:
-0.0divisor, both modes. The legacy path nulls out zero divisors viaeq(right, 0), but Arrow's floating-point comparisons use a total order inwhich
-0.0is distinct from0.0, so a-0.0divisor goes unrecognised.Spark's
isZerois a numeric comparison and treats-0.0as zero:What changes are included in this PR?
try_remnow detects zero divisors itself, mirroring the shape #23898established for
pmod:is_zerohelper counts-0.0as zero for the floating-point types(via a
negative_zerocompanion, same as fix(spark): correct pmod overflow, ANSI zero divisor and negative zero handling #23898).ArrowError::DivideByZero— the same error Arrow's
remalready produces for integers today, so themessage 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.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
pmodto no longer usetry_remand adds identicalis_zero/negative_zerohelpers. This PR isindependent of it —
modis fixed either way — but whichever lands secondshould dedupe the helpers in a rebase. Until #23898 lands,
pmodalso picksup the
-0.0and ANSI floating-point zero-divisor fixes through the sharedhelper.
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:
modulus.rs: ANSI floating-point zero divisor raises;-0.0divisor returns NULL in legacy mode and raises in ANSI mode; a NULLdividend with a zero divisor returns NULL in ANSI mode (integer and float).
spark/math/mod.sltcovering the same behavior atSQL level.
cargo test -p datafusion-spark, thespark/math/mod.sltandspark/math/pmod.sltsqllogictests,./dev/rust_lint.sh, and the extendedworkspace 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/pmodfunctions: in ANSImode a floating-point zero divisor now raises instead of returning NaN, and a
-0.0divisor is treated as zero in both modes (NULL in legacy mode, an errorin ANSI mode), matching Spark. No API changes.