adriangb opened a new pull request, #25681: URL: https://github.com/apache/datafusion/pull/25681
## Which issue does this PR close? - Part of #22883 (experimental "optional filters" stack, design notes: https://claude.ai/artifact/SSz7t6hPyhFWp1MDPecVqt). - **Depends on #25673.** Review only the top two commits. ## Rationale for this change Dynamic filters from `HashJoinExec`, TopK (`SortExec` with a fetch) and `AggregateExec` are not needed for correctness: the producer checks the same condition again. #25673 adds `OptionalFilterPhysicalExpr` to state this. This PR makes the producers use it, so that consumers (#25674 and the Parquet / `FilterExec` PRs on top) can skip these filters when they do not remove enough rows. ## What changes are included in this PR? - The pushed copy of the dynamic filter is wrapped as `Optional(DynamicFilter [...])` in: - `HashJoinExec::gather_filters_for_pushdown` (probe side self filter), - `SortExec::gather_filters_for_pushdown` (TopK), - `AggregateExec::gather_filters_for_pushdown`. Each producer keeps its own unwrapped `Arc<DynamicFilterPhysicalExpr>` for `update()` / `mark_complete()`. - Hash join key transfer (#25255) rewrites below the wrapper, so a transferred dynamic filter stays optional. A transferred *parent* filter stays required: for inner and semi joins it replaces the original filter. - Direct downcasts now look through the wrapper (`as_dynamic_filter`): hash join consumer detection in `handle_child_pushdown_result` (#24601) and the dynamic filter routing in `NestedLoopJoinExec`. - `ParquetSource::try_pushdown_filters` calls `debug_assert_optional_on_root_chain` on its stored predicate. **No behavior change**: the wrapper evaluates its child. Only `EXPLAIN` text changes. ## What is the testing strategy for this PR? - New unit tests: `test_pushed_dynamic_filters_are_optional` (hash join self filter and key transfer), `test_nlj_routes_optional_dynamic_filter`. - The #24601 consumer detection tests pass unchanged. - Proto round trips for hash join, TopK and aggregate plans check that the scan predicate stays `Optional` and that the producer and scan still share one dynamic filter. - Snapshot updates: 11 inline snapshots in `filter_pushdown.rs` and 18 `.slt` files. A script checked that every changed line equals the old line once the `Optional(...)` wrapper is removed. The tree `EXPLAIN` format does not show the wrapper. - Full sqllogictest suite, `core_integration`, `parquet_integration`, and the physical-plan, proto, datasource-parquet, physical-optimizer and pruning crate tests pass. ## Are there any user-facing changes? `EXPLAIN` shows `Optional(DynamicFilter [...])` for pushed dynamic filters. Code outside this repository that downcasts a pushed-down predicate directly to `DynamicFilterPhysicalExpr` must now look through the wrapper (use `datafusion_physical_expr::utils::as_dynamic_filter`). This may need an upgrade guide note. 🤖 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]
