stag7824 opened a new pull request, #2373: URL: https://github.com/apache/datafusion-ballista/pull/2373
# Which issue does this PR close? Closes #2368. # Rationale for this change `ballista-core` and `ballista-executor` each declare a feature named `arrow-ipc-optimizations`, both on by default, but they are unrelated features that happen to share a name. The executor's feature did not forward to core's — unlike `spark-compat` immediately below it in the same `[features]` table. The two halves gate different code: | Feature | Gated site | |---|---| | `ballista-executor/arrow-ipc-optimizations` | `ballista/executor/src/flight_service.rs:159` | | `ballista-core/arrow-ipc-optimizations` | `ballista/core/src/client.rs:453` | | | `ballista/core/src/execution_plans/shuffle_reader.rs:1188` | | | `ballista/core/src/execution_plans/sort_shuffle/multi_stream_reader.rs:105` | So building the executor standalone with `--no-default-features --features arrow-ipc-optimizations` skips IPC validation at **one** of the four decode sites and validates at the other three, with nothing to indicate the difference. `ballista/client/Cargo.toml:33-35` pulls the executor with `default-features = false, features = ["arrow-ipc-optimizations"]`, which reads as "keep the IPC optimisation on" but only enabled the executor's half. Nothing is broken today, because the client also depends on `ballista-core` directly and core's feature arrives through core's own defaults — but it works by accident. # What changes are included in this PR? One line in `ballista/executor/Cargo.toml`: ```toml -arrow-ipc-optimizations = [] +arrow-ipc-optimizations = ["ballista-core/arrow-ipc-optimizations"] ``` # Are these changes tested? Yes — verified with cargo's own feature resolver, using `-i ballista-core` to show what enables core's feature under `-p ballista-executor --no-default-features --features arrow-ipc-optimizations`. **Before** — core's feature is reachable only through core's own defaults: ``` ballista-core v54.0.0 ├── ballista-core feature "arrow-ipc-optimizations" │ └── ballista-core feature "default" │ └── ballista-executor v54.0.0 │ └── ballista-executor feature "arrow-ipc-optimizations" (command-line) └── ballista-core feature "default" (*) ``` **After** — the executor's feature now enables core's directly: ``` ballista-core v54.0.0 ├── ballista-core feature "arrow-ipc-optimizations" │ ├── ballista-executor feature "arrow-ipc-optimizations" (command-line) │ └── ballista-core feature "default" │ └── ballista-executor v54.0.0 │ └── ballista-executor feature "arrow-ipc-optimizations" (command-line) └── ballista-core feature "default" (*) ``` The new `ballista-executor feature "arrow-ipc-optimizations" (command-line)` edge directly under core's feature is the fix. # Are there any user-facing changes? No. Default behaviour is unchanged, since both features are already on by default. Worth noting for anyone reading the issue later: this still does not give a full off switch. Cargo features are additive, so anything else in the graph that enables `ballista-core`'s default features will turn core's half back on regardless. -- 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]
