Repository navigation
perf: use native distinct accumulator for COUNT(DISTINCT) on Decimal32/Decimal64 - #26821
Draft
tjnangosha wants to merge 3 commits into
Draft
tjnangosha wants to merge 3 commits into
tjnangosha wants to merge 3 commits into
Conversation
tjnangosha
force-pushed
the
count-distinct-decimal32-64
branch
from
October 27, 2026 20:11
c7273c2 to
1b19736
Compare
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?
Rationale for this change
COUNT(DISTINCT)on a Decimal32 or Decimal64 column is slower than the same query on a Decimal128 column, even though the values are smaller. Onmain, counting the distinct values in a batch of 8,192 Decimal32 values takes about 870 µs, compared with about 110 µs for Decimal128.The cause is in
get_count_accumulatorindatafusion/functions-aggregate/src/count.rs. Decimal128 and Decimal256 usePrimitiveDistinctCountAccumulator, which works on the raw values directly. Decimal32 and Decimal64 have no arm there, so they fall through to the genericDistinctCountAccumulator, which converts every row into aScalarValuebefore inserting it into a hash set.This looks like a spot that was missed when Decimal32/64 support was added in #17501 (issue #17489).
What changes are included in this PR?
count.rs: addDecimal32andDecimal64arms toget_count_accumulator, usingPrimitiveDistinctCountAccumulatorlike Decimal128 and Decimal256 do.benches/count_distinct.rs: addcount_distinctcases for Decimal32, Decimal64 and Decimal128 (as a reference) at 80% and 99% distinct.aggregate.slt: addCOUNT(DISTINCT)tests on Decimal32 and Decimal64 columns.Benchmark results
COUNT(DISTINCT)accumulator, one batch of 8,192 values (datafusion/functions-aggregate/benches/count_distinct.rs).What is the testing strategy for this PR?
The change doesn't affect results, only which accumulator is used. The new sqllogictest cases in
aggregate.sltrunCOUNT(DISTINCT)on Decimal32 and Decimal64 columns containing duplicates and a NULL, with and withoutGROUP BY. Each query counts two different columns, so the single-distinct rewrite (single_distinct_to_groupby) doesn't turn it into aGROUP BYand the query goes through the accumulator changed here. The new bench cases cover the performance side.Are there any user-facing changes?
No. Query results are unchanged, and
COUNT(DISTINCT)on Decimal32 and Decimal64 columns is faster. There are no public API changes.