compiler-segfault commented on PR #66:
URL: https://github.com/apache/datafusion-spark/pull/66#issuecomment-6827615180
Thanks for the review. All four suggestions and both nits are in `5108048`:
1. The `sql` feature is documented in `crates/spark/Cargo.toml` and in the
crate docs,
including that it only covers SQL parsing and that `col("a") / col("b")`
keeps
DataFusion's operator.
2. `spark_divide` is now exported with `make_udf_function!` in
`function/math/mod.rs`, so
planning reuses the `LazyLock<Arc<ScalarUDF>>` singleton like
`date_part()` and
`substring()`, and it is listed in the package's `functions()`.
`SELECT spark_divide(1, 2)` returns `0.5` as `Float64`.
3. `divide.slt` now also covers a `Float32` pair and a mixed `INT`/`BIGINT`
column (checking
the `Float64` result type), a `NULL` dividend over a zero divisor in both
modes,
`arrow_typeof(CAST(1 AS DECIMAL(10,2)) / 2)` still `Decimal128(14, 6)`
with the value still
`0.500000`, and an interval divisor that still plans through DataFusion's
operator.
4. The fall-through for `1 / NULL` and `'4' / 2` is now a comment in
`plan_binary_op`: both
keep the `/` operator because the planner cannot coerce them without a
wider change.
Nits: the unit test is `divides_as_double`, and both assertions say
`spark_divide expects exactly two arguments`.
Re-ran on `branch-56`: 328 unit tests with `--features sql`, 330 with
`--all-features`,
246/246 sqllogictest files, `cargo check` clean for the default, `core`,
`sql` and `core,sql`
feature sets, clippy `-D warnings` and `cargo fmt` clean.
--
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]
