fix: distinct_eliminated is rewritten as distinct_on_group_key - #19142
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where count(distinct key), uniq(key), and count_distinct(key) on a grouping key were incorrectly converted to count(), which would return the group size instead of the correct value (1 for non-null values, 0 for null values).
- The optimization now correctly rewrites these aggregate functions to scalar expressions that return 1 for non-null grouping keys and 0 for null grouping keys
- Added logic test cases to verify the correct behavior for both non-nullable and nullable grouping keys
- Refactored the scalar item handling to support post-aggregate scalar expressions
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/sqllogictests/suites/base/03_common/03_0001_select_aggregator.test | Added test cases to verify count(distinct k) returns 1 per group (or 0 for NULL) when k is the grouping key |
| src/query/sql/src/planner/optimizer/optimizers/operator/aggregate/normalize_aggregate.rs | Fixed the optimization logic to rewrite distinct aggregates on grouping keys as conditional expressions (1 or 0) instead of incorrectly converting to count() |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.github.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…endlabs#19142) * fix: distinct_eliminated is rewritten as distinct_on_group_key * chore: codefmt * chore: codefmt * fix: group by set * fix: explain for agg * chore: fix explain_native/aggregate.test * chore: fix explain agg * fix: stop collapsing distinct on grouping sets to 0/1
I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/
Summary
The previous
distinct_eliminatedmethod would incorrectly convert tocountwhengroup_distinctis used andarg0is a group by key. However, in this case,group_distinctcan only be 0 or 1.Tests
Type of change
This change is