Dandandan opened a new pull request, #24376: URL: https://github.com/apache/datafusion/pull/24376
## Which issue does this PR close? - Addresses https://github.com/apache/datafusion/issues/13814 - Follow-on to #24325, #24326, #24329, #24330 ## Rationale for this change `TableProvider::scan` took its projection as `Option<&Vec<usize>>`. Besides being the signature `clippy::ptr_arg` exists to discourage, it forced `TableProvider::scan_with_args`'s default body to allocate: ```rust let projection = args.projection().map(|p| p.to_vec()); let plan = self.scan(state, projection.as_ref(), filters, limit).await?; ``` `ScanArgs::projection()` already yields `Option<&[usize]>`, so that `Vec` existed only to be borrowed. Worse, it cannot outlive a hoisted future, so the body had to stay an `async fn` — and that made it **the single most expensive thing to compile in `datafusion-session`**. Proving its coroutine `Send` walks the whole `Expr`/`LogicalPlan` type graph, and `#[async_trait]`'s `'life0: 'async_trait` bounds stop rustc serving that proof from its global cache. I measured this directly before writing the change: stubbing the body out dropped `datafusion-session`'s `evaluate_obligation` from **617ms to 16.5ms** and the whole crate from 0.86s to 0.235s, while the rest of the crate's trait solving attributed to just 4ms. So this one default body was ~97% of what remained after #24326. ## What changes are included in this PR? The projection becomes `Option<&[usize]>`, and the default body hands it straight through, capturing only the already-boxed future it awaits: ```rust let plan = self.scan( state, args.projection(), args.filters().unwrap_or(&[]), args.limit(), ); Box::pin(async move { Ok(plan.await?.into()) }) ``` Also updated, because their projections feed the same call paths: `StreamingTableExec::try_new`, `FilterExec::with_projection`, `batch_filter`. `datafusion_common::project_schema` now takes `Option<&T> where T: AsRef<[usize]> + ?Sized` instead of `Option<&impl AsRef<[usize]>>`, so it accepts a `Vec` *or* a slice — no caller of it needs to change. Call sites get simpler rather than noisier: `Some(&vec![2, 1])` becomes `Some(&[2, 1])`, and three `vec!` allocations in tests disappear (clippy pointed those out on its own). ## Are these changes tested? - `cargo test -p datafusion --lib` — 442 passed - `cargo test -p datafusion-session -p datafusion-catalog -p datafusion-catalog-listing` — all passed - `cargo check --workspace --all-targets` — clean - `cargo clippy` on the affected crates with `--all-targets`, and `-p datafusion --lib` — clean - `cargo fmt --all --check` — clean Interleaved A/B of `cargo rustc -p <crate> --lib`, 3 rounds each, alternating with `main` so machine drift cancels out: | crate | before | after | `evaluate_obligation` | |---|---|---|---| | **datafusion-session** | 0.890s | **0.273s** | **624.3ms → 10.4ms** | | datafusion-catalog | 1.046s | 1.205s | 58.6 → 71.8ms | | datafusion (core) | 4.088s | 4.125s | 138.6 → 134.5ms | `datafusion-session` compiles **3.3× faster** and its trait solving drops 98%. I want to be straight about the other two rows: `datafusion-catalog` regresses ~10%. I re-measured it with the ordering reversed (change first, `main` second) in case warming was biasing it, and the regression persisted — so it is real, most likely the extra `project_schema` instantiation and the `to_vec()` calls that replace `projection.cloned()`. Core is flat. Net across the three is roughly −0.45s, dominated by session; in the feature-unified build session's saving should be larger still, since obligations are several times dearer there. ## Are there any user-facing changes? Yes — this is a **breaking change** for `TableProvider` implementors, so it has an upgrade-guide entry. Implementations only need the signature updated; two patterns inside a body need adjusting: ```rust // Before // After projection.cloned() projection.map(|p| p.to_vec()) ``` and callers holding an `Option<Vec<usize>>` pass `.as_deref()` rather than `.as_ref()`. In tree that was 47 signatures across 36 files, all mechanical. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
