andygrove opened a new pull request, #6450:
URL: https://github.com/apache/datafusion-comet/pull/6450
## Which issue does this PR close?
Part of #5639.
## Rationale for this change
#5639 splits the native `core` crate into per-concern crates. Its step T3
moves the `PhysicalExpr`
implementations under `core/src/execution/expressions/` into the expression
crate, and leaves the
builders that turn Spark protobuf expressions into physical expressions for
the planner crate (T7).
Everything else in that directory was already builder code, so three
expressions were left to move.
## What changes are included in this PR?
- `ListPositionsExpr` moves unchanged to
`spark-expr/src/array_funcs/list_positions.rs`, together
with its unit tests.
- `Subquery` moves to `spark-expr/src/subquery.rs`. It now takes
`JVMClasses` and the JNI wrappers
from `datafusion-comet-jni-bridge` and `bytes_to_i128` from
`datafusion-comet-common`. `spark-expr`
already depends on both, and `jvm_udf/` already calls into the JVM the
same way.
- `CheckedBinaryExpr` moves from
`core/src/execution/expressions/arithmetic.rs` to
`spark-expr/src/math_funcs/checked_binary_expr.rs`. Its `child()` accessor
becomes `pub`, because
the planner's `unwrap_checked` now calls it from another crate.
- `spark-expr` gains a direct `paste` dependency, because `jni_static_call!`
expands to
`paste::paste!`, plus the same cargo-machete ignore entry `core` already
has for it.
- `core` drops its `bytes_to_i128` re-export, since `Subquery` was its last
user.
- The planner imports the three types from `datafusion_comet_spark_expr`, and
`sql_error_propagation.md` points at the new location of
`CheckedBinaryExpr`.
The expressions' code is otherwise unchanged, and git records
`list_positions.rs` and `subquery.rs`
as renames. `core/src/execution/expressions/` now holds only builders.
## How are these changes tested?
This moves code without changing behavior, so there are no new tests. The
moved `list_positions`
unit tests now run as part of `datafusion-comet-spark-expr`.
- `cargo clippy --all-targets --workspace -- -D warnings`, `cargo fmt
--check` and `cargo machete`
are clean. Without the new ignore entry, machete reports `paste` as unused
in `spark-expr`.
- `cargo test -p datafusion-comet-spark-expr` (including the moved
`list_positions` tests) and
`cargo test -p datafusion-comet` pass.
- The JVM tests that exercise the three expressions pass: the posexplode
tests in
`CometGenerateExecSuite` (`ListPositionsExpr`), the scalar subquery tests
in `CometExecSuite`
(`Subquery`, where the `scalar subquery` test reads ten result types back
over JNI), and the ANSI
tests in `CometExpressionSuite` (`CheckedBinaryExpr`).
--
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]
