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]