Skip to content

fix(spark): correct mod ANSI zero divisor and negative zero handling - #23987

Open
u70b3 wants to merge 1 commit into
apache:mainfrom
u70b3:fix/spark-mod-zero-divisor
Open

fix(spark): correct mod ANSI zero divisor and negative zero handling#23987
u70b3 wants to merge 1 commit into
apache:mainfrom
u70b3:fix/spark-mod-zero-divisor

Conversation

@u70b3

@u70b3 u70b3 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

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:

    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:

    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 fix(spark): correct pmod overflow, ANSI zero divisor and negative zero handling #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.

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.
@github-actions github-actions Bot added sqllogictest SQL Logic Tests (.slt) spark labels Jul 30, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.95062% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.75%. Comparing base (2f25454) to head (115cecb).

Files with missing lines Patch % Lines
datafusion/spark/src/function/math/modulus.rs 83.95% 3 Missing and 10 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #23987      +/-   ##
==========================================
- Coverage   80.75%   80.75%   -0.01%     
==========================================
  Files        1096     1096              
  Lines      373588   373660      +72     
  Branches   373588   373660      +72     
==========================================
+ Hits       301687   301740      +53     
- Misses      53898    53907       +9     
- Partials    18003    18013      +10     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

spark sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] mod returns NaN instead of raising for a zero floating point divisor in ANSI mode, and ignores -0.0 divisors

2 participants