This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion.git
The following commit(s) were added to refs/heads/main by this push:
new 2da78d67ce fix(spark): correct mod ANSI zero divisor and negative zero
handling (#23987)
2da78d67ce is described below
commit 2da78d67ce6dc3e2975906b3ca60e35cd1d9afed
Author: kid <[email protected]>
AuthorDate: Sun Aug 16 01:23:51 2026 +0000
fix(spark): correct mod ANSI zero divisor and negative zero handling
(#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.
---
datafusion/spark/src/function/math/modulus.rs | 265 +++++++++++++++++++--
.../sqllogictest/test_files/spark/math/mod.slt | 31 +++
.../sqllogictest/test_files/spark/math/pmod.slt | 25 ++
3 files changed, 302 insertions(+), 19 deletions(-)
diff --git a/datafusion/spark/src/function/math/modulus.rs
b/datafusion/spark/src/function/math/modulus.rs
index 97f59c2cbb..c37513c12c 100644
--- a/datafusion/spark/src/function/math/modulus.rs
+++ b/datafusion/spark/src/function/math/modulus.rs
@@ -15,41 +15,77 @@
// specific language governing permissions and limitations
// under the License.
-use arrow::array::{Scalar, new_null_array};
+use arrow::array::{ArrayRef, BooleanArray, Scalar, new_null_array};
use arrow::compute::kernels::numeric::add;
use arrow::compute::kernels::{
+ boolean::{and, is_not_null, or},
cmp::{eq, lt},
- numeric::rem,
+ numeric::{neg, rem},
zip::zip,
};
use arrow::datatypes::DataType;
+use arrow::error::ArrowError;
use datafusion_common::{Result, ScalarValue, assert_eq_or_internal_err};
use datafusion_expr::{
ColumnarValue, ScalarFunctionArgs, ScalarUDFImpl, Signature, Volatility,
};
+/// Returns a one element array holding negative zero, for the floating point
+/// types only.
+///
+/// Arrow's comparison kernels order floating point values totally, so `-0.0`
+/// compares as distinct from, and less than, `0.0`. Java, and therefore Spark,
+/// treats `-0.0` as equal to zero. The helper below uses this to restore the
+/// IEEE 754 answer.
+fn negative_zero(data_type: &DataType) -> Result<Option<ArrayRef>> {
+ match data_type {
+ DataType::Float16 | DataType::Float32 | DataType::Float64 => {
+ let zero = ScalarValue::new_zero(data_type)?.to_array()?;
+ Ok(Some(neg(zero.as_ref())?))
+ }
+ _ => Ok(None),
+ }
+}
+
+/// Rows of `values` that equal zero, counting `-0.0` as zero.
+fn is_zero(values: &ArrayRef) -> Result<BooleanArray> {
+ let zero = ScalarValue::new_zero(values.data_type())?.to_array()?;
+ let mask = eq(values, &Scalar::new(zero))?;
+ match negative_zero(values.data_type())? {
+ Some(negative_zero) => Ok(or(&mask, &eq(values,
&Scalar::new(negative_zero))?)?),
+ None => Ok(mask),
+ }
+}
+
/// Computes `rem(left, right)` with divide-by-zero handling.
-/// In ANSI mode, any zero divisor causes an error.
-/// In legacy mode (ANSI off), zero divisors are replaced with NULL before
-/// computing the remainder, so those positions return NULL while others
-/// compute normally.
+/// In ANSI mode, a zero divisor of any numeric type causes an error, with
+/// `-0.0` counting as zero; a row whose dividend is NULL never raises, to
+/// match Spark's null-intolerant remainder. In legacy mode (ANSI off), zero
+/// divisors are replaced with NULL before computing the remainder, so those
+/// positions return NULL while others compute normally.
fn try_rem(
- left: &arrow::array::ArrayRef,
- right: &arrow::array::ArrayRef,
+ left: &ArrayRef,
+ right: &ArrayRef,
enable_ansi_mode: bool,
-) -> Result<arrow::array::ArrayRef> {
+) -> Result<ArrayRef> {
+ let divisor_is_zero = is_zero(right)?;
+ // Null out zero divisors so that the remainder kernels never see one:
+ // division by zero then returns NULL instead of erroring (integers) or
+ // returning NaN (floats). ANSI mode reports the error itself below, so
+ // this substitution is harmless on rows that must raise.
+ let null = Scalar::new(new_null_array(right.data_type(), 1));
+ let safe_right = zip(&divisor_is_zero, &null, right)?;
if enable_ansi_mode {
- Ok(rem(left, right)?)
- } else {
- // In legacy mode, null out zero divisors so that division by zero
- // returns NULL instead of erroring (integers) or returning NaN
(floats).
- let zero = ScalarValue::new_zero(right.data_type())?.to_array()?;
- let zero = Scalar::new(zero);
- let null = Scalar::new(new_null_array(right.data_type(), 1));
- let is_zero = eq(right, &zero)?;
- let safe_right = zip(&is_zero, &null, right)?;
- Ok(rem(left, &safe_right)?)
+ // Spark's remainder expressions are null intolerant, so a row whose
+ // dividend is NULL evaluates to NULL and never raises, even when the
+ // divisor on that row is zero. Mask the check by the validity of the
+ // dividend to match.
+ let raises = and(&divisor_is_zero, &is_not_null(left.as_ref())?)?;
+ if raises.iter().flatten().any(|raises| raises) {
+ return Err(ArrowError::DivideByZero.into());
+ }
}
+ Ok(rem(left, &safe_right)?)
}
/// Spark-compatible `mod` function
@@ -418,6 +454,101 @@ mod test {
assert!(result.is_err());
}
+ #[test]
+ fn test_mod_zero_division_ansi_float() {
+ // In ANSI mode a zero divisor of any numeric type must raise,
+ // including floating point, where Arrow's `rem` follows IEEE 754
+ // and quietly returns NaN (#23894)
+ let left = Float64Array::from(vec![Some(10.5), Some(7.2)]);
+ let right = Float64Array::from(vec![Some(0.0), Some(2.0)]);
+
+ let left_value = ColumnarValue::Array(Arc::new(left));
+ let right_value = ColumnarValue::Array(Arc::new(right));
+
+ let result = spark_mod(&[left_value, right_value], true);
+ assert!(result.is_err());
+ }
+
+ #[test]
+ fn test_mod_negative_zero_divisor_legacy() {
+ // `-0.0` counts as a zero divisor, so it returns NULL in legacy
+ // mode rather than NaN (#23894)
+ let left = Float64Array::from(vec![Some(10.5), Some(7.5)]);
+ let right = Float64Array::from(vec![Some(-0.0), Some(2.0)]);
+
+ let left_value = ColumnarValue::Array(Arc::new(left));
+ let right_value = ColumnarValue::Array(Arc::new(right));
+
+ let result = spark_mod(&[left_value, right_value], false).unwrap();
+
+ if let ColumnarValue::Array(result_array) = result {
+ let result_float64 = result_array
+ .as_any()
+ .downcast_ref::<Float64Array>()
+ .unwrap();
+ assert!(result_float64.is_null(0)); // 10.5 % -0.0 = NULL
+ assert_eq!(result_float64.value(1), 1.5); // 7.5 % 2.0 = 1.5
+ } else {
+ panic!("Expected array result");
+ }
+ }
+
+ #[test]
+ fn test_mod_negative_zero_divisor_ansi() {
+ // `-0.0` counts as a zero divisor, so it raises in ANSI mode (#23894)
+ let left = Float64Array::from(vec![Some(10.5)]);
+ let right = Float64Array::from(vec![Some(-0.0)]);
+
+ let left_value = ColumnarValue::Array(Arc::new(left));
+ let right_value = ColumnarValue::Array(Arc::new(right));
+
+ let result = spark_mod(&[left_value, right_value], true);
+ assert!(result.is_err());
+ }
+
+ #[test]
+ fn test_mod_zero_division_ansi_null_dividend() {
+ // Spark's remainder expressions are null intolerant: a NULL dividend
+ // short-circuits to NULL before the divisor is validated, so a zero
+ // divisor on such a row must not raise, even in ANSI mode (#23894)
+ let left = Int32Array::from(vec![None, Some(10)]);
+ let right = Int32Array::from(vec![Some(0), Some(3)]);
+
+ let left_value = ColumnarValue::Array(Arc::new(left));
+ let right_value = ColumnarValue::Array(Arc::new(right));
+
+ let result = spark_mod(&[left_value, right_value], true).unwrap();
+
+ if let ColumnarValue::Array(result_array) = result {
+ let result_int32 =
+ result_array.as_any().downcast_ref::<Int32Array>().unwrap();
+ assert!(result_int32.is_null(0)); // NULL % 0 = NULL (no error)
+ assert_eq!(result_int32.value(1), 1); // 10 % 3 = 1
+ } else {
+ panic!("Expected array result");
+ }
+
+ // Same for floating point
+ let left = Float64Array::from(vec![None, Some(10.5)]);
+ let right = Float64Array::from(vec![Some(0.0), Some(2.0)]);
+
+ let left_value = ColumnarValue::Array(Arc::new(left));
+ let right_value = ColumnarValue::Array(Arc::new(right));
+
+ let result = spark_mod(&[left_value, right_value], true).unwrap();
+
+ if let ColumnarValue::Array(result_array) = result {
+ let result_float64 = result_array
+ .as_any()
+ .downcast_ref::<Float64Array>()
+ .unwrap();
+ assert!(result_float64.is_null(0)); // NULL % 0.0 = NULL (no error)
+ assert!((result_float64.value(1) - 0.5).abs() < f64::EPSILON); //
10.5 % 2.0 = 0.5
+ } else {
+ panic!("Expected array result");
+ }
+ }
+
// PMOD tests
#[test]
fn test_pmod_int32() {
@@ -645,6 +776,102 @@ mod test {
assert!(result.is_err());
}
+ #[test]
+ fn test_pmod_zero_division_ansi_float() {
+ // pmod routes through `try_rem` twice, so it needs the same coverage
+ // as mod: in ANSI mode a zero divisor of any numeric type must
+ // raise, including floating point, where Arrow's `rem` follows
+ // IEEE 754 and quietly returns NaN (#23894)
+ let left = Float64Array::from(vec![Some(10.5), Some(7.2)]);
+ let right = Float64Array::from(vec![Some(0.0), Some(2.0)]);
+
+ let left_value = ColumnarValue::Array(Arc::new(left));
+ let right_value = ColumnarValue::Array(Arc::new(right));
+
+ let result = spark_pmod(&[left_value, right_value], true);
+ assert!(result.is_err());
+ }
+
+ #[test]
+ fn test_pmod_negative_zero_divisor_legacy() {
+ // `-0.0` counts as a zero divisor, so it returns NULL in legacy
+ // mode rather than NaN (#23894)
+ let left = Float64Array::from(vec![Some(10.5), Some(-7.5)]);
+ let right = Float64Array::from(vec![Some(-0.0), Some(2.0)]);
+
+ let left_value = ColumnarValue::Array(Arc::new(left));
+ let right_value = ColumnarValue::Array(Arc::new(right));
+
+ let result = spark_pmod(&[left_value, right_value], false).unwrap();
+
+ if let ColumnarValue::Array(result_array) = result {
+ let result_float64 = result_array
+ .as_any()
+ .downcast_ref::<Float64Array>()
+ .unwrap();
+ assert!(result_float64.is_null(0)); // 10.5 pmod -0.0 = NULL
+ assert!((result_float64.value(1) - 0.5).abs() < f64::EPSILON); //
-7.5 pmod 2.0 = 0.5
+ } else {
+ panic!("Expected array result");
+ }
+ }
+
+ #[test]
+ fn test_pmod_negative_zero_divisor_ansi() {
+ // `-0.0` counts as a zero divisor, so it raises in ANSI mode (#23894)
+ let left = Float64Array::from(vec![Some(10.5)]);
+ let right = Float64Array::from(vec![Some(-0.0)]);
+
+ let left_value = ColumnarValue::Array(Arc::new(left));
+ let right_value = ColumnarValue::Array(Arc::new(right));
+
+ let result = spark_pmod(&[left_value, right_value], true);
+ assert!(result.is_err());
+ }
+
+ #[test]
+ fn test_pmod_zero_division_ansi_null_dividend() {
+ // Spark's remainder expressions are null intolerant: a NULL dividend
+ // short-circuits to NULL before the divisor is validated, so a zero
+ // divisor on such a row must not raise, even in ANSI mode (#23894)
+ let left = Int32Array::from(vec![None, Some(10)]);
+ let right = Int32Array::from(vec![Some(0), Some(3)]);
+
+ let left_value = ColumnarValue::Array(Arc::new(left));
+ let right_value = ColumnarValue::Array(Arc::new(right));
+
+ let result = spark_pmod(&[left_value, right_value], true).unwrap();
+
+ if let ColumnarValue::Array(result_array) = result {
+ let result_int32 =
+ result_array.as_any().downcast_ref::<Int32Array>().unwrap();
+ assert!(result_int32.is_null(0)); // NULL pmod 0 = NULL (no error)
+ assert_eq!(result_int32.value(1), 1); // 10 pmod 3 = 1
+ } else {
+ panic!("Expected array result");
+ }
+
+ // Same for floating point
+ let left = Float64Array::from(vec![None, Some(10.5)]);
+ let right = Float64Array::from(vec![Some(0.0), Some(2.0)]);
+
+ let left_value = ColumnarValue::Array(Arc::new(left));
+ let right_value = ColumnarValue::Array(Arc::new(right));
+
+ let result = spark_pmod(&[left_value, right_value], true).unwrap();
+
+ if let ColumnarValue::Array(result_array) = result {
+ let result_float64 = result_array
+ .as_any()
+ .downcast_ref::<Float64Array>()
+ .unwrap();
+ assert!(result_float64.is_null(0)); // NULL pmod 0.0 = NULL (no
error)
+ assert!((result_float64.value(1) - 0.5).abs() < f64::EPSILON); //
10.5 pmod 2.0 = 0.5
+ } else {
+ panic!("Expected array result");
+ }
+ }
+
#[test]
fn test_pmod_negative_divisor() {
// PMOD with negative divisor should still work like regular mod
diff --git a/datafusion/sqllogictest/test_files/spark/math/mod.slt
b/datafusion/sqllogictest/test_files/spark/math/mod.slt
index 8229bb0651..b9a8499939 100644
--- a/datafusion/sqllogictest/test_files/spark/math/mod.slt
+++ b/datafusion/sqllogictest/test_files/spark/math/mod.slt
@@ -160,6 +160,18 @@ SELECT MOD(10.5::float8, 0.0::float8) as
mod_div_zero_float;
----
NULL
+# A negative zero divisor counts as zero and returns NULL in legacy mode
+query R
+SELECT MOD(10.5::float8, -0.0::float8) as mod_div_zero_neg_float;
+----
+NULL
+
+# A NULL dividend evaluates to NULL regardless of the divisor
+query I
+SELECT MOD(NULL::int, 0::int) as mod_null_dividend_legacy;
+----
+NULL
+
# Division by zero errors in ANSI mode
statement ok
set datafusion.execution.enable_ansi_mode = true;
@@ -170,6 +182,25 @@ SELECT MOD(10::int, 0::int);
statement error DataFusion error: Arrow error: Divide by zero error
SELECT MOD(-7::int, 0::int);
+# A zero divisor of any numeric type raises, including floating point
+statement error DataFusion error: Arrow error: Divide by zero error
+SELECT MOD(10.5::float8, 0.0::float8);
+
+# A negative zero divisor counts as zero and raises
+statement error DataFusion error: Arrow error: Divide by zero error
+SELECT MOD(10.5::float8, -0.0::float8);
+
+# A NULL dividend short-circuits to NULL before the divisor is validated
+query I
+SELECT MOD(NULL::int, 0::int) as mod_null_dividend_ansi;
+----
+NULL
+
+query R
+SELECT MOD(NULL::float8, 0.0::float8) as mod_null_dividend_ansi_float;
+----
+NULL
+
statement ok
set datafusion.execution.enable_ansi_mode = false;
diff --git a/datafusion/sqllogictest/test_files/spark/math/pmod.slt
b/datafusion/sqllogictest/test_files/spark/math/pmod.slt
index aa4a197ba4..1165fdfba3 100644
--- a/datafusion/sqllogictest/test_files/spark/math/pmod.slt
+++ b/datafusion/sqllogictest/test_files/spark/math/pmod.slt
@@ -74,6 +74,12 @@ SELECT pmod(-7::int, 0::int) as pmod_zero_3;
----
NULL
+# A negative zero divisor counts as zero and returns NULL in legacy mode
+query R
+SELECT pmod(10.5::float8, -0.0::float8) as pmod_div_zero_neg_float;
+----
+NULL
+
# Division by zero errors in ANSI mode
statement ok
set datafusion.execution.enable_ansi_mode = true;
@@ -84,6 +90,25 @@ SELECT pmod(10::int, 0::int);
statement error DataFusion error: Arrow error: Divide by zero error
SELECT pmod(-7::int, 0::int);
+# A zero divisor of any numeric type raises, including floating point
+statement error DataFusion error: Arrow error: Divide by zero error
+SELECT pmod(10.5::float8, 0.0::float8);
+
+# A negative zero divisor counts as zero and raises
+statement error DataFusion error: Arrow error: Divide by zero error
+SELECT pmod(10.5::float8, -0.0::float8);
+
+# A NULL dividend short-circuits to NULL before the divisor is validated
+query I
+SELECT pmod(NULL::int, 0::int) as pmod_null_dividend_ansi;
+----
+NULL
+
+query R
+SELECT pmod(NULL::float8, 0.0::float8) as pmod_null_dividend_ansi_float;
+----
+NULL
+
statement ok
set datafusion.execution.enable_ansi_mode = false;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
