[PR] fix: compare a Remainder divisor with zero in Spark's ordering without ANSI [datafusion-comet]

["andygrove (via GitHub)" <gi...@apache.org>]

andygrove opened a new pull request, #6896:
URL: https://github.com/apache/datafusion-comet/pull/6896

   ## Which issue does this PR close?
   
   Part of #6385. Follow-up to #6866, where viirya raised this and rich7420 suggested this fix.
   
   ## Rationale for this change
   
   With ANSI off, Spark returns NULL for `x % y` when `y` is zero, and its check, `y == 0`, is true for `-0.0` as well. Comet builds that check natively in `null_if_zero_primitive` as a plain `BinaryExpr`, so `x % -0.0` returns NULL only because DataFusion 55's `apply_cmp` folds signed zeros before Arrow's `eq`, which tells the two zeros apart. The floating-point contributor guide asks us not to rely on that. Until #6866, `CometExecRule` normalized float divisors in Projects and Filters, so the common case did not depend on it. `Divide`'s check is the serde's `If(EqualTo(right, 0), null, right)`, which goes through the native comparison builder and does not depend on DataFusion either.
   
   ## What changes are included in this PR?
   
   - `null_if_zero_primitive` builds its zero check with `spark_comparison(..., FloatOperands::Normalize)`. For a `FLOAT` or `DOUBLE` divisor that is a `SparkComparison`, in which `-0.0` equals `0.0` whatever DataFusion's `apply_cmp` does. Integer and decimal divisors keep the same `BinaryExpr`, because `spark_comparison` leaves them alone.
   - `arithmetic.sql` adds `1.0D / (-d)` and `1.0D % (-d)` over a column holding `0.0`, a `-0.0` divisor that is not a literal.
   
   ## How are these changes tested?
   
   - `test_modulo_divide_by_negative_zero_float_non_ansi` checks that a `-0.0` divisor gives NULL with ANSI off, for Float32 and Float64, from a column and from a literal. DataFusion's folding makes those pass with the old `BinaryExpr` guard too, so `test_non_ansi_float_divisor_guard_compares_in_spark_ordering` checks that the guard compares with a `SparkComparison`, and it fails against the old guard.
   - All 1,179 `spark-expr` tests pass, and clippy is clean on stable and nightly.
   - On Spark 4.1, 492 tests pass: the `expressions/math/` fixtures, including `arithmetic.sql` and `nan_divisor.sql` with ANSI off and on, `CometMathExpressionSuite`, `CometFloatSemanticsSuite`, and the remainder tests in `CometExpressionSuite` and `CometCodegenSuite`.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] fix: compare a Remainder divisor with zero in Spark's ordering without ANSI [datafusion-comet]

["andygrove (via GitHub)" <gi...@apache.org>]

andygrove merged PR #6896:
URL: https://github.com/apache/datafusion-comet/pull/6896


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org