alamb opened a new pull request, #22437: URL: https://github.com/apache/datafusion/pull/22437
## Which issue does this PR close? - Reverts #20337 - Addresses concerns raised in https://github.com/apache/datafusion/pull/22415 ## Rationale for this change `ExecutionPlan::apply_expressions()` was added in #20337 with no default implementation, forcing every custom `ExecutionPlan`, `FileSource`, and `DataSource` implementor to add the method as part of upgrading to DataFusion 54. As discussed on #22415, the method is not yet called from anywhere in DataFusion: the originally intended use (dynamic-filter discovery/serialization for distributed scenarios) is blocked on other in-progress work (#20009, #21350). The combined effect on downstream users is a required code change with no immediate benefit, and ambiguity about what a "correct" implementation even means today (returning `Ok(TreeNodeRecursion::Continue)` is safe right now but becomes incorrect as soon as the method starts being used by an optimizer pass). The plan agreed in the discussion is to remove the API from the 54.0 release and re-add it together with the concrete consumer that needs it. cc @adriangb @LiaCastaneda @milenkovicm. ## What changes are included in this PR? `git revert -m 1` of the merge commit, with the following manual conflict resolutions and follow-ups: - Kept post-#20337 additions in files that the revert wanted to wholesale undo (e.g. `DataSource::with_new_state` / `create_sibling_state` / `OpenArgs` in `datafusion/datasource/src/source.rs`, `FileScanConfig::create_sibling_state`, the `output_requirements`/`limit`/`ffi` changes that landed independently). - Removed `apply_expressions` implementations from `ExecutionPlan`/`FileSource`/`DataSource` nodes added after #20337 (`scalar_subquery`, `operator_statistics`, `sorts/partitioned_topk`, the `enforce_distribution`/`roundtrip_physical_plan` test fakes). - Dropped the `apply_expressions` section from `docs/source/library-user-guide/upgrading/54.0.0.md` and the snippets from `docs/source/library-user-guide/custom-table-providers.md`. - Fixed up imports (e.g. moved `PhysicalExpr` into the `limit.rs` test module where the auto-merged revert left a now-dangling type reference). ## Are these changes tested? - `cargo check --workspace --all-targets --all-features` -- clean - `cargo clippy --all-targets --all-features -- -D warnings` -- clean - `cargo test -p datafusion-physical-plan --lib` -- 1435 passed, 0 failed ## Are there any user-facing changes? Yes -- this removes the new public API: - `ExecutionPlan::apply_expressions` - `FileSource::apply_expressions` - `DataSource::apply_expressions` These were only added in 54 and are not yet released. Custom implementors no longer need to implement these methods. -- 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]
