Repository navigation
fix: reject GROUPING masks wider than 64 bits - #26137
Open
alexandrefimov wants to merge 4 commits into
Open
alexandrefimov wants to merge 4 commits into
alexandrefimov wants to merge 4 commits into
Conversation
Reject grouping-set layouts with more than 64 distinct keys before the analyzer constructs a UInt64 GROUPING mask. Preserve argument validation and the ordinary GROUP BY path where GROUPING evaluates to zero. Add core API tests for unrepresentable masks, valid boundaries, ordinary 65-key grouping and COUNT-only physical-capacity refusals in both aggregate kernels.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #26137 +/- ##
==========================================
- Coverage 82.77% 82.77% -0.01%
==========================================
Files 1147 1147
Lines 450909 450913 +4
Branches 450909 450913 +4
==========================================
+ Hits 373252 373255 +3
+ Misses 54934 54930 -4
- Partials 22723 22728 +5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
neilconway
reviewed
Oct 8, 2026
neilconway
left a comment
neilconway
left a comment
Contributor
There was a problem hiding this comment.
Thanks for looking at this, @alexandrefimov !
Move the grouping bitmap capacity check to Aggregate::try_new and cover the SQL paths with logic tests instead of kernel-specific API tests.
github-actions
Bot
added
logical-expr
Contributor
Author
|
The macOS runner lost connection. Could you rerun the failed job? I don’t have permission. |
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
3 participants
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.
Which issue does this PR close?
Closes #26135.
Rationale for this change
GROUPING over 65 distinct grouping columns can panic while the analyzer constructs the mask.
What changes are included in this PR?
Reject unrepresentable grouping-set masks after argument validation and before constructing the mask. Preserve the ordinary GROUP BY path where GROUPING returns zero.
What is the testing strategy for this PR?
Five core API tests cover high-bit, low-bit and all-argument cases, 63/64-key boundaries, ordinary 65-key GROUP BY and COUNT-only physical-capacity refusal. Both aggregate kernels are covered.
Focused tests, crate tests, extended workspace tests, formatting, all-target/all-feature Clippy and
dev/rust_lint.shpassed.Are there any user-facing changes?
Unsupported masks return NotImplemented during analysis.