Avoid null-restrict evaluation for predicates that reference non-join columns in PushDownFilter#20961
Avoid null-restrict evaluation for predicates that reference non-join columns in PushDownFilter#20961kosiew wants to merge 15 commits intoapache:mainfrom
Conversation
Introduce a test case to assert non-restricting behavior when evaluating the predicate a > b, focusing on join keys that only include a. This directly tests the new early-return branch in the is_restrict_null_predicate function in utils.rs, enhancing overall code coverage.
Extract the column-membership check into a new helper function called `predicate_uses_only_columns` in utils.rs. Update the current implementation at utils.rs:91 to use this new helper, improving code readability and maintainability.
Add call-site contract comment in push_down_filter.rs to specify that only Ok(true) is treated as null-restricting. State that both Ok(false) and Err(_) are considered non-restricting and will be skipped during processing.
Inline iterator predicate in utils.rs and streamline the null-restrict handling in push_down_filter.rs. This reduces indirections and lines of code while maintaining the same logic and behavior. No public interface or behavior changes intended.
…te_uses_only_columns function
|
run benchmark sql_planner_extended |
|
🤖 Criterion benchmark running (GKE) | trigger |
|
show benchmark queue |
|
Hi @kosiew, you asked to view the benchmark queue (#20961 (comment)). No pending jobs. |
|
run benchmark sql_planner_extended |
|
show benchmark queue |
|
Hi @kosiew, you asked to view the benchmark queue (#20961 (comment)).
|
|
🤖 Criterion benchmark running (GKE) | trigger |
|
show benchmark queue |
|
Hi @kosiew, you asked to view the benchmark queue (#20961 (comment)).
|
|
show benchmark queue |
|
Hi @kosiew, you asked to view the benchmark queue (#20961 (comment)).
|
Implement fast path for syntactic null-restriction in utils.rs to classify predicates without evaluating physical expressions. Enhance SQL boolean handling with a large supporting evaluator, including CASE management. Retain existing branch helper styles and expand test coverage for constant simple CASE and outside-join-key fast paths. Fix correctness edge case for simple CASE indeterminate comparisons, ensuring proper tracking and fallback to Unknown.
|
I think the benchmark never completes or gets killed because it's too heavy. |
|
run benchmark sql_planner_extended --sample-size 10 |
|
show benchmark queue |
|
Hi @kosiew, you asked to view the benchmark queue (#20961 (comment)).
|
|
🤖 Criterion benchmark running (GKE) | trigger |
|
🤖 Criterion benchmark running (GKE) | trigger |
|
Benchmark for this request failed. Last 20 lines of output: Click to expand |
|
🤖 Criterion benchmark running (GKE) | trigger |
|
Benchmark for this request failed. Last 20 lines of output: Click to expand |
Implement focused regression tests in push_down_filter_regressions.rs. These tests cover the following SQL shapes: window + scalar subquery, regr_* aggregate query, correlated IN subquery, and NATURAL JOIN with UNION ALL cases. This ensures accurate reporting and enhances SQL handling reliability.
Extract authoritative_restrict_null_predicate() to directly access the old physical-evaluation path. Implement a debug check in is_restrict_null_predicate() to ensure fast-path results match the authoritative evaluator. Add a unit test to verify consistency between both paths, enhancing debug capabilities and test coverage.
Remove early-return handling for complex constructs like CASE and scalar functions. Narrow fast path to a limited subset including aliases, literals, direct join-key column substitution, and specific operators. All other cases now utilize evaluate_expr_with_null_column(...) for authoritative evaluation.
Add window_scalar_subquery_optimizer_delta to compare query execution plans with different push_down_filter settings. Implement test hook for easier comparison without manual edits. Confirm that optimizer changes the plan from Filter + Cross Join to Inner Join, indicating a key delta for this case.
Prevent conversion of post-join filters to join filters when the join lacks equijoin keys and one side is scalar. This change maintains the filter + cross join structure for failing scalar-subquery shapes. Added tests for optimizer-level guards and regression coverage.
kosiew
force-pushed
the
push-down-02-20002
branch
from
8658e02 to
9665c63
Compare
Refactor null checks in utils.rs by collapsing fast-path gates and removing duplicated branches for various null handling scenarios. Extract reusable helpers for null checks to improve readability. In push_down_filter.rs, streamline the scalar-cross-join guard and eliminate inline boolean expressions from the push_down_all_join function for clearer logic.
2 participants
Which issue does this PR close?
Rationale for this change
PushDownFilterhas been identified as a major planning-time bottleneck, with profiling showing significant time spent repeatedly evaluating expression types while reasoning about predicate pushdown and null-restriction behavior. This PR addresses a slice of that problem by reducing unnecessary work in null-restriction evaluation and by tightening join-filter conversion behavior to avoid incorrect rewrites in scalar-side join cases.In particular, the patch focuses on two goals:
Together, these changes both improve optimizer robustness and target one of the hot paths called out in the profiling for
PushDownFilter.What changes are included in this PR?
This PR includes the following changes:
Adds regression SQL tests under
datafusion/core/tests/sql/push_down_filter_regressions.rsand wires the new module into the SQL test suite.Adds end-to-end regression coverage for:
push_down_filterenabled/disabledINsubqueriesNATURAL JOINcombined withUNION ALLAdds an optimizer regression test to ensure a post-join filter is retained for a cross join where one side is scalar/aggregated.
Changes
push_down_filterso filters are only converted into join conditions for inner joins when it is safe to do so, specifically avoiding that conversion for joins with emptyONclauses when either side is known to be scalar (max_rows() == Some(1)).Updates inferred predicate handling to treat null-restriction evaluation failures conservatively via
unwrap_or(false).Refactors null-restriction evaluation in
optimizer::utilsby:NullRestrictionEvalModeand a test-only guard for controlling evaluation modeExpands unit test coverage for
is_restrict_null_predicateand for agreement between the syntactic fast path and the authoritative evaluator.Are these changes tested?
Yes.
The PR adds both logical optimizer tests and SQL-level regression coverage:
is_restrict_null_predicate, including additionalCASE,BETWEEN,IN, and null-related predicate formspush_down_filterenabled and disabledThese tests verify both correctness and protection against the regressions addressed by this patch.
Are there any user-facing changes?
There are no intended user-facing API changes.
This PR improves optimizer behavior and planner performance characteristics internally, and fixes incorrect or fragile filter-pushdown behavior for certain query shapes. Users may observe more stable planning behavior and preserved correctness for affected queries, but no documentation or API changes should be required.
LLM-generated code disclosure
This PR includes LLM-generated code and comments. All LLM-generated content has been manually reviewed and tested.