[CALCITE-7779] Optimization rule FilterAggregateTransposeRule rewrites queries to semantically non-equivalent ones - #5261
Open
npaincomplet wants to merge 1 commit into
Conversation
…s queries to semantically non-equivalent ones
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Jira Link
CALCITE-7779
Filter pushability
In the simple aggregate case, FilterAggregateTransposeRule pushes a predicate below an aggregate when the predicate's referenced columns all belong to the aggregate’s grouping set, because aggregate-function results are unavailable before aggregation.
In the non-simple aggregate case, FilterAggregateTransposeRule pushes a predicate below an aggregate when the predicate's referenced columns all belong to every grouping set, because a referenced column is replaced with NULL in rows produced by a grouping set that omits it, and evaluating the predicate before aggregation can change the result.
Bug description
The pushability check in the non-simple case currently compares filter-input column positions (= aggregate-output column positions) with aggregate-input column positions without remapping. The same position can identify different columns in these two row layouts, so this check does not establish that the predicate’s referenced columns belong to every grouping set. Consequently, the rule can incorrectly push a predicate and change the query result, or fail to push a predicate when doing so would be valid.
Reproducer
After applying AGGREGATE_PROJECT_MERGE followed by FILTER_AGGREGATE_TRANSPOSE, the resulting plan is equivalent to:
which isn't semantically equivalent to the first query.
Fix
The fix uses
Mappings.targetandImmutableBitSet.permuteto express each grouping set in aggregate-output positions before comparing it with the predicate's references. The existing simple-aggregate check is unchanged.Tests
The existing dedicated grouping-set tests use matching input and output positions. The new tests apply
AGGREGATE_PROJECT_MERGEfirst to expose the mismatch and cover:HAVING b IS NULL: keep the filter above the aggregate to preserve the subtotal row.HAVING b = 2: keep the filter above the aggregate to avoid an extra subtotal row.HAVING a = 1: allow pushdown becauseabelongs to every grouping set.All three regression tests failed against the original rule.