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/10/14 02:42:28 UTC
(datafusion-comet) branch gh-readonly-queue/main/pr-6896-7d8d9b1544b734c85a93cca2cc9d5380ad70accd created (now ea42b1b4fd)
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-6896-7d8d9b1544b734c85a93cca2cc9d5380ad70accd
in repository https://gitbox.apache.org/repos/asf/datafusion-comet.git
at ea42b1b4fd fix: compare a Remainder divisor with zero in Spark's ordering without ANSI (#6896)
This branch includes the following new commits:
new ea42b1b4fd fix: compare a Remainder divisor with zero in Spark's ordering without ANSI (#6896)
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-comet) 01/01: fix: compare a Remainder divisor with zero in Spark's ordering without ANSI (#6896)
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-6896-7d8d9b1544b734c85a93cca2cc9d5380ad70accd
in repository https://gitbox.apache.org/repos/asf/datafusion-comet.git
commit ea42b1b4fdcb72b60426aca7c808859aba987271
Author: Andy Grove <ag...@apache.org>
AuthorDate: Wed Oct 14 02:42:07 2026 +0000
fix: compare a Remainder divisor with zero in Spark's ordering without ANSI (#6896)
null_if_zero_primitive built Remainder's non-ANSI zero-divisor check as a plain BinaryExpr, so
`x % -0.0` returned NULL only because DataFusion 55's apply_cmp folds signed zeros before Arrow's
eq. #6866 removed the divisor normalization that used to hide this in Projects and Filters. The
check now goes through spark_comparison, as Divide's guard does through the native comparison
builder.
---
native/spark-expr/src/math_funcs/modulo_expr.rs | 72 ++++++++++++++++++++--
.../sql-tests/expressions/math/arithmetic.sql | 4 ++
2 files changed, 70 insertions(+), 6 deletions(-)
diff --git a/native/spark-expr/src/math_funcs/modulo_expr.rs b/native/spark-expr/src/math_funcs/modulo_expr.rs
index 09d006c9e4..60bd9609d4 100644
--- a/native/spark-expr/src/math_funcs/modulo_expr.rs
+++ b/native/spark-expr/src/math_funcs/modulo_expr.rs
@@ -15,7 +15,7 @@
// specific language governing permissions and limitations
// under the License.
-use crate::{create_comet_physical_fun, IfExpr};
+use crate::{create_comet_physical_fun, spark_comparison, FloatOperands, IfExpr};
use crate::{remainder_by_zero_error, Cast, EvalMode, SparkCastOptions};
use arrow::array::{ArrayRef, ArrowNativeTypeOp, AsArray, PrimitiveArray};
use arrow::compute::kernels::arity::try_binary;
@@ -27,7 +27,7 @@ use datafusion::common::{
};
use datafusion::config::ConfigOptions;
use datafusion::execution::FunctionRegistry;
-use datafusion::physical_expr::expressions::{lit, BinaryExpr};
+use datafusion::physical_expr::expressions::lit;
use datafusion::physical_expr::ScalarFunctionExpr;
use datafusion::physical_expr_common::datum::{apply, apply_cmp_for_nested};
use datafusion::{
@@ -232,12 +232,15 @@ fn null_if_zero_primitive(
// Create an expression like - `if (eval(expr) == Literal(0)) then NULL else eval(expr)`.
// This expression evaluates to null for rows with zero values to prevent divide by zero
- // error.
- let eq_expr = Arc::new(BinaryExpr::new(
+ // error. A float divisor is compared in Spark's ordering, where `-0.0` equals `0.0` as it
+ // does in Spark's own check, while Arrow's `eq` tells the two zeros apart.
+ let eq_expr = spark_comparison(
Arc::<dyn PhysicalExpr>::clone(&expression),
Operator::Eq,
lit(zero),
- ));
+ input_schema,
+ FloatOperands::Normalize,
+ )?;
let null_literal = lit(ScalarValue::try_new_null(&expr_data_type)?);
let if_expr = Arc::new(IfExpr::new(eq_expr, null_literal, expression));
Ok(if_expr)
@@ -286,12 +289,13 @@ fn create_modulo_scalar_function(
#[cfg(test)]
mod tests {
use super::*;
+ use crate::SparkComparison;
use arrow::array::{
Array, ArrayRef, Decimal128Array, Decimal128Builder, Float32Array, Float64Array,
Int32Array, PrimitiveArray, RecordBatch,
};
use datafusion::logical_expr::ColumnarValue;
- use datafusion::physical_expr::expressions::{Column, Literal};
+ use datafusion::physical_expr::expressions::{BinaryExpr, Column, Literal};
use datafusion::prelude::SessionContext;
fn with_fail_on_error<F: Fn(bool)>(test_fn: F) {
@@ -593,6 +597,62 @@ mod tests {
);
}
+ #[test]
+ fn test_modulo_divide_by_negative_zero_float_non_ansi() {
+ // Spark treats `-0.0` as a zero divisor too, so without ANSI the result is null.
+ run_float_modulo::<Float64Type>(
+ DataType::Float64,
+ Arc::new(Float64Array::from(vec![Some(1.0), Some(1.0), Some(1.0)])),
+ Arc::new(Float64Array::from(vec![Some(-0.0), Some(0.0), Some(2.0)])),
+ false,
+ false,
+ Some(Arc::new(Float64Array::from(vec![None, None, Some(1.0)]))),
+ );
+ run_float_modulo::<Float32Type>(
+ DataType::Float32,
+ Arc::new(Float32Array::from(vec![Some(1.0_f32), Some(1.0_f32)])),
+ Arc::new(Float32Array::from(vec![Some(-0.0_f32), Some(2.0_f32)])),
+ false,
+ false,
+ Some(Arc::new(Float32Array::from(vec![None, Some(1.0_f32)]))),
+ );
+ run_float_modulo_with_literal_divisor(
+ Arc::new(Float64Array::from(vec![Some(1.0)])),
+ ScalarValue::Float64(Some(-0.0)),
+ false,
+ false,
+ Some(Arc::new(Float64Array::from(vec![None]))),
+ );
+
+ let schema = Arc::new(Schema::new(vec![Field::new("a", DataType::Float32, true)]));
+ let lhs: ArrayRef = Arc::new(Float32Array::from(vec![Some(1.0_f32)]));
+ let batch = RecordBatch::try_new(Arc::clone(&schema), vec![lhs]).unwrap();
+ let modulo_expr = create_modulo_expr(
+ Arc::new(Column::new("a", 0)),
+ Arc::new(Literal::new(ScalarValue::Float32(Some(-0.0)))),
+ DataType::Float32,
+ schema,
+ false,
+ &SessionContext::new().state(),
+ )
+ .unwrap();
+ let expected = Arc::new(Float32Array::from(vec![None]));
+ verify_result(modulo_expr, batch, false, Some(expected));
+ }
+
+ #[test]
+ fn test_non_ansi_float_divisor_guard_compares_in_spark_ordering() {
+ // DataFusion 55 folds `-0.0` into `0.0` before Arrow's `eq`, so the test above would pass
+ // with a plain `BinaryExpr` guard too. The guard must not depend on that.
+ let schema = Schema::new(vec![Field::new("b", DataType::Float64, true)]);
+ let guard = null_if_zero_primitive(Arc::new(Column::new("b", 0)), &schema).unwrap();
+ let condition = guard.children()[0];
+ assert!(
+ condition.downcast_ref::<SparkComparison>().is_some(),
+ "unexpected guard condition: {condition}"
+ );
+ }
+
#[test]
fn test_modulo_null_dividend_zero_divisor_float64_ansi() {
// A null dividend must not raise even when the divisor is zero — Spark returns null
diff --git a/spark/src/test/resources/sql-tests/expressions/math/arithmetic.sql b/spark/src/test/resources/sql-tests/expressions/math/arithmetic.sql
index a2cb88d343..edce7d1b2b 100644
--- a/spark/src/test/resources/sql-tests/expressions/math/arithmetic.sql
+++ b/spark/src/test/resources/sql-tests/expressions/math/arithmetic.sql
@@ -83,3 +83,7 @@ SELECT a / b, a % b, d / 0.0 FROM test_div_zero
-- `-0.0D` is a double literal with the sign bit set, and is a zero divisor too
query
SELECT d / -0.0D, d % -0.0D, d / double('-0.0'), d % double('-0.0') FROM test_div_zero
+
+-- So is a column that holds `-0.0`, because Spark checks the divisor with `== 0`
+query
+SELECT 1.0D / (-d), 1.0D % (-d) FROM test_div_zero
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org