compiler-segfault opened a new pull request, #66:
URL: https://github.com/apache/datafusion-spark/pull/66

   ## Which issue does this PR close?
   
   - Closes #47.
   
   ## Rationale for this change
   
   With the Spark dialect, `SELECT 1 / 2` returns `0`, while Spark returns 
`0.5`. Spark's `/` is
   `Divide`, whose `inputType` is `TypeCollection(DoubleType, DecimalType)`, so 
a non-`DECIMAL`
   pair is cast to `DOUBLE` before the division; `div` is the integer division. 
The integer
   result also makes `SELECT IF(true, 1 / 1, 1 / 0)` an `Int64` where Spark 
returns a `Double`.
   Spark also returns NULL for a zero divisor in its default (non-ANSI) mode 
("Special case
   handling due to division/remainder by 0 => null or ArithmeticException"), 
where DataFusion's
   floating point division returns an infinity, so casting alone would still 
leave `1 / 0`
   different.
   
   ## What changes are included in this PR?
   
   `SparkFunctionPlanner` now plans `/` for non-`DECIMAL` numeric operands as a 
call to a
   `divide` function that implements Spark's semantics: it casts both sides to 
`Float64`,
   returns NULL when the divisor is zero (`-0.0` included) and raises 
`DivideByZero` under
   `datafusion.execution.enable_ansi_mode`, mirroring the existing `spark_mod`. 
`DECIMAL`
   division keeps the current `/` operator and its precision and scale, and 
non-numeric
   operands such as intervals are left alone.
   
   The planner hook needs the `sql` feature of `datafusion-expr`, so 
`datafusion-spark` gains a
   `sql` feature that enables it (and `datafusion-functions-nested/sql`, which 
has the same
   requirement) and the sqllogictest crate enables it. Without the feature the 
hook is not
   compiled and the crate behaves as before.
   
   This is the same change as apache/datafusion#27203, reopened here because 
the crate moved, as
   asked in 
https://github.com/apache/datafusion/pull/27203#issuecomment-6803143787.
   
   ## What is the testing strategy for this PR?
   
   Unit tests in `crates/spark/src/function/math/divide.rs` cover the cast, the 
legacy NULL for
   `0.0` and `-0.0`, the ANSI error, and a NULL dividend with a zero divisor.
   `crates/sqllogictest/test_files/math/divide.slt` covers integer, float, 
column and
   zero-divisor cases plus the ANSI-mode error, and the existing
   `crates/sqllogictest/test_files/conditional/if.slt` expectation changes from 
`Int64` to the
   `Double` result Spark returns.
   
   Verified against `branch-56`, which is what the workspace pins:
   
   - `cargo test -p datafusion-spark --features sql` — 328 passed
   - `cargo test -p datafusion-spark --all-features` — 330 passed
   - `cargo test -p datafusion-sqllogictest --test sqllogictests -- spark` — 
246 files passed
   - `cargo clippy -p datafusion-spark --all-targets --all-features -- -D 
warnings` — clean
   - `cargo fmt --all -- --check` — clean
   
   Before the change, `SELECT 1 / 2` is `0` and `SELECT 1 / 0` is `inf`; after 
it they are
   `0.5` and `NULL`, and `SELECT IF(true, 1 / 1, 1 / 0)` is a `Double` `1.0`.
   
   ## Are there any user-facing changes?
   
   Spark-dialect queries now return `DOUBLE` from `/` on non-decimal operands 
and NULL instead
   of an infinity for a zero divisor, matching Spark. `IF(true, 1, 1 / 0)` 
still evaluates both
   branches, so in ANSI mode it raises where Spark short-circuits; that is 
existing `if`
   behaviour rather than something this change introduces.
   
   The change also affects the `datafusion-spark` crate's feature set (`sql`), 
not its default
   behaviour: without the `sql` feature no planner hook is installed.
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to