Skip to content

fix(query): fix incorrect pushdown for grouping sets #18223

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 23, 2025

Conversation

sundy-li
Copy link
Member

@sundy-li sundy-li commented Jun 23, 2025

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/

Summary

fix(query): fix incorrect pushdown for grouping sets

If all used columns in predicate are in group columns
we can push down the predicate, otherwise we need to keep the predicate
eg:

 select * from (select number % 3 a, number % 4 b from
 range(1, 1000)t(number) group by cube(a,b))  where a is null and b is null;

we can't push down the predicate cause filter will remove all rows
but we can't remove the predicate cause the null will be generated
by group by cube

Tests

  • Unit Test
  • Logic Test
  • Benchmark Test
  • No Test - Explain why

Type of change

  • Bug Fix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Breaking Change (fix or feature that could cause existing functionality not to work as expected)
  • Documentation Update
  • Refactoring
  • Performance Improvement
  • Other (please describe):

This change is Reviewable

@github-actions github-actions bot added the pr-bugfix this PR patches a bug in codebase label Jun 23, 2025
@sundy-li sundy-li requested review from Copilot and TCeason June 23, 2025 08:29
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes an issue in query pushdown logic for grouping sets to ensure that predicates are only pushed down when all predicate columns are present in the grouping sets.

  • Added a new test case to validate the pushdown behavior for grouping sets.
  • Updated the pushdown filter logic in the aggregation optimization rule to correctly evaluate grouping sets.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
tests/sqllogictests/suites/duckdb/sql/aggregate/group/group_by_grouping_sets.test Added a test case demonstrating the predicate pushdown scenario in grouping sets.
src/query/sql/src/planner/optimizer/optimizers/rule/agg_rules/rule_push_down_filter_aggregate.rs Modified the predicate pushdown logic to conditionally push or retain predicates based on grouping set coverage.

@sundy-li sundy-li merged commit ca5a61c into databendlabs:main Jun 23, 2025
81 checks passed
@sundy-li sundy-li deleted the fix-group-filter branch June 23, 2025 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-bugfix this PR patches a bug in codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants