Repository navigation
Conversation
QinXi-ai
marked this pull request as ready for review
November 13, 2026 16:03
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.
Which issue does this PR close?
Part of #25936. This implements the accumulator step in the follow-up plan.
Rationale for this change
The native grouped integer
COUNT(DISTINCT)accumulator currently probes the shared distinct hash set for every non-null input row. For Q10, which has a million groups with four copies of one value per group, keeping that value inline avoids those probes.I built both versions from
c8265b68on a shared Linux host, using a Xeon Gold 6238T, Rust 1.99.1 andrelease-nonlto. The suite used four million rows, one target partition, greedy / 1 GiB, withsingle_distinct_prefer_nativeenabled. These are medians from three alternating rounds, six iterations per round with the first discarded, giving 15 retained samples per version. CPU affinity was 0-7, with builds stopped during timing.The inline buffer allocates a typed slot for every group, including groups with only NULL arguments. Q12 exposes that cost, with about 6.7 MiB more peak pool memory.
What changes are included in this PR?
Keep each group's first non-null value in a typed vector and put additional values in the shared hash set. After a batch where the average number of additional distinct values per group exceeds eight, move the inline values into the set and release the vector. Subsequent batches use the hash-only loop. This avoids the per-row cache check for the large sets in Q07 and Q09. CPU-0 repeats measured -2.9% and -3.5% elapsed changes for those two queries.
Both modes use the existing List state format. State emission, partial emission, merge and memory accounting handle the inline values and the switch to the hash-only mode.
Add Q11 and Q12 to the SQL benchmark suite to cover small distinct sets and NULL-heavy groups.
What is the testing strategy for this PR?
Added unit tests for partial emission, overlapping merges, group-index shifts, promotion, buffer accounting and reuse. SQL tests exercise small batches and overlapping partial states through the normal query path.
Validated on Linux with Rust 1.99.1.
uv run ./dev/rust_lint.shsuite passed.Are there any user-facing changes?
Native grouped integer
COUNT(DISTINCT)has the runtime and memory changes shown above.Prepared with Codex assistance. Opening as a draft for author review.