Pastoray opened a new pull request, #27182: URL: https://github.com/apache/datafusion/pull/27182
> [!IMPORTANT] > > **Fork CI setup:** For pull requests from a fork to `main`, enable workflows in your fork's **Actions** tab before you push. > See the [fork CI setup guide](https://datafusion.apache.org/contributor-guide/#pull-requests-from-forks). ## Which issue does this PR close? - Closes #26305. ## Rationale for this change `PushDownFilter::rewrite` had grown into a single ~460-line function that mixed predicate simplification, early-outs, and the pushdown logic for every plan node. Several match arms (`SubqueryAlias`, `Unnest`, `Aggregate`, `Window`, `AsOfJoin`, `TableScan`, `Extension`) contain multi-step logic that could not be unit tested without running the whole rule, and every new plan type or pushdown tweak had to touch the same function. ## What changes are included in this PR? Pure refactor, no behavior change. `rewrite` is now a dispatcher (about 170 lines), and the logic moved into private free functions, following the existing `push_down_join` / `rewrite_projection` pattern: - `simplify_filter_predicates`: the predicate simplify/reorder preamble. - `push_down_subquery_alias`, `push_down_unnest`, `push_down_through_aggregate`, `push_down_through_window`, `push_down_through_asof_join`, `push_down_into_table_scan`, `push_down_through_extension`. Two small differences from the signatures sketched in the issue: - `simplify_filter_predicates` returns a small enum (`Unchanged` / `Changed` / `Removed`) instead of `bool`, because the preamble can also decide the filter should be dropped entirely. - The helpers take the filter's `predicate: Expr` plus the child node rather than a whole `Filter`, like `push_down_join` and `rewrite_projection` already do. `rewrite` has already moved `filter.input` out by the time it dispatches. Helpers that decline to push anything return `Transformed::no(Filter)`, and `rewrite` ORs the "predicates were simplified/reordered" flag into the result, so the `transformed` flag is the same as before. The `Union`, `Projection`, `Filter`, `Repartition`, `Distinct`, `Sort` and `MaterializedCte` arms are left inline since they are short or already delegate. Converting the file into a module directory (the optional part of the issue) is left for a follow-up to keep this diff reviewable. ## What is the testing strategy for this PR? - All 89 existing `push_down_filter` unit tests pass unchanged; they cover each moved arm through the full rule. - 7 new unit tests in `push_down_filter.rs` call the helpers directly, which was not possible before: - `simplify_filter_predicates_unchanged` - `simplify_filter_predicates_drops_redundant_conjunct` - `simplify_filter_predicates_contradiction_becomes_false` - `push_down_subquery_alias_remaps_columns` - `push_down_through_aggregate_splits_predicate` - `push_down_through_aggregate_nothing_to_push` (also asserts `transformed` is `false`) - `push_down_through_window_pushes_partition_key_predicates` - Also run locally: `cargo test -p datafusion-optimizer` (lib, integration and doctests), the full sqllogictest suite (545 files), `cargo fmt --all -- --check`, and clippy on `datafusion-optimizer`. ## Are there any user-facing changes? No. The new functions are private and the public API of `PushDownFilter` is unchanged. -- 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]
