You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/08/16 01:24:14 UTC
(datafusion) branch gh-readonly-queue/main/pr-23987-bf1c17ae6b41a0aa8fe95a7b20273aefb47d3b10 created (now 2da78d67ce)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-23987-bf1c17ae6b41a0aa8fe95a7b20273aefb47d3b10
in repository https://gitbox.apache.org/repos/asf/datafusion.git
at 2da78d67ce fix(spark): correct mod ANSI zero divisor and negative zero handling (#23987)
This branch includes the following new commits:
new 2da78d67ce fix(spark): correct mod ANSI zero divisor and negative zero handling (#23987)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org
(datafusion) 01/01: fix(spark): correct mod ANSI zero divisor and negative zero handling (#23987)
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-23987-bf1c17ae6b41a0aa8fe95a7b20273aefb47d3b10
in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit 2da78d67ce6dc3e2975906b3ca60e35cd1d9afed
Author: kid <19...@users.noreply.github.com>
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: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org