npaincomplet opened a new pull request, #5261: URL: https://github.com/apache/calcite/pull/5261
## Jira Link [CALCITE-7779](https://issues.apache.org/jira/browse/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 ``` SELECT a, b FROM (VALUES (0, 1, 2)) AS t(unused, a, b) GROUP BY GROUPING SETS ((a), (a, b)) HAVING b IS NULL ``` After applying AGGREGATE_PROJECT_MERGE followed by FILTER_AGGREGATE_TRANSPOSE, the resulting plan is equivalent to: ``` SELECT a, b FROM (VALUES (0, 1, 2)) AS t(unused, a, b) WHERE b IS NULL GROUP BY GROUPING SETS ((a), (a, b)) ``` which isn't semantically equivalent to the first query. ## Fix The fix uses `Mappings.target` and `ImmutableBitSet.permute` to 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_MERGE` first 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 because `a` belongs to every grouping set. All three regression tests failed against the original rule. -- 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]
