Repository navigation
fix: keep constant filters above grand totals - #27117
Conversation
github-actions
Bot
added
optimizer
1312f6d to
f4f4bac
Compare
f4f4bac to
b4ad343
Compare
jayzhan211
left a comment
jayzhan211
left a comment
There was a problem hiding this comment.
@Phoenix500526 thanks for the fix! decorrelate.rs around line 891 still has its own copy of this rule (has_empty_set), and this is the fourth fix of this fact (#22039, #25781, #22926). Could it call the new helper too, so every logical check uses one rule? At that point grouping_sets is non-empty, so group_expr is always [GroupingSet] and the result is the same. Locally, clippy and the optimizer and subquery tests pass with this change.
-use crate::utils::replace_qualified_name;
+use crate::utils::{aggregate_emits_on_empty_input, replace_qualified_name};
@@
- let has_empty_set = grouping_sets.iter().any(|grouping_set| match grouping_set {
- GroupingSet::Rollup(_) | GroupingSet::Cube(_) => true,
- GroupingSet::GroupingSets(sets) => sets.iter().any(|set| set.is_empty()),
- });
+ let has_empty_set = aggregate_emits_on_empty_input(group_expr);
02aad2f to
9e52c06
Compare
Thanks @jayzhan211 Good catch. I've used |
A global aggregate or empty grouping set produces a row even on empty input. Pushing a false predicate below it can return that extra row. CLOSES apache#26545
9e52c06 to
05fec9c
Compare
jayzhan211
left a comment
jayzhan211
left a comment
There was a problem hiding this comment.
Thanks @Phoenix500526 !
Which issue does this PR close?
Closes #26545.
Rationale for this change
Filtering a constant column above a global aggregate can return an extra row. For example, this query should return no rows, but currently returns one row containing
0.Pushing the false predicate below
COUNT(*)empties its input, but the aggregate still produces a row. Aggregates with an empty grouping set, such asROLLUP(a), have the same problem.What changes are included in this PR?
PropagateEmptyRelation.ROLLUP, and update a Substrait snapshot where a redundantTRUEscan filter is now eliminated.Constant folding can still remove a true filter or replace a false-filtered aggregate with an empty relation. Ordinary grouping-key predicates can still be pushed below aggregation.
What is the testing strategy for this PR?
Added two result-based regressions in
push_down_filter_regression.slt, both verified to fail before the fix and pass afterward.Passed locally at this commit
cargo fmt --allcargo clippy --all-targets --all-features -- -D warningsavro,json,backtrace,extended_tests,recursive_protection,parquet_encryption, including all 547 SQL logic test files and 228 Substrait integration tests.Also checked optimized plans for false and true constant predicates and ordinary grouping-key filters, and checked query results for bound parameters,
CUBE, and explicit empty grouping sets.The additional
./dev/rust_lint.shsuite stopped at the circular-dependency check because the local Cargo configuration replaces crates.io with thetunaregistry. The complete lint suite therefore has not passed locally.Are there any user-facing changes?
Queries with constant filters above global aggregates or empty grouping sets now return the correct number of rows.