Skip to content

Fix COUNT(DISTINCT ...) collision for multi-column comma values - #3781

Closed
chiliec wants to merge 1 commit into
dolthub:mainfrom
chiliec:fix-count-distinct-multicol-collision
Closed

Fix COUNT(DISTINCT ...) collision for multi-column comma values#3781
chiliec wants to merge 1 commit into
dolthub:mainfrom
chiliec:fix-count-distinct-multicol-collision

Conversation

@chiliec

@chiliec chiliec commented Sep 3, 2026

Copy link
Copy Markdown

What

Fixes a COUNT(DISTINCT a, b) correctness bug where distinct multi-column tuples are counted as one. Reported downstream at dolthub/dolt#11562.

CREATE TABLE t(id INT PRIMARY KEY, a VARCHAR(16), b VARCHAR(16));
INSERT INTO t VALUES (1, 'a,', 'b'), (2, 'a', ',b');
SELECT COUNT(DISTINCT a, b) FROM t;   -- MySQL: 2, was returning 1

Fix

The multi-column path in countDistinctBuffer.Update built its hash key by joining each converted value with a , separator (str += vv + ","). That encoding is not injective: ('a,','b') and ('a',',b') both produce "a,,b,", so distinct tuples collide into one hash bucket. Length-prefixing each value ("%d:%s") makes the encoding self-delimiting, so different tuples always produce different keys.

Tests

Added TestCountDistinctEvalMultiColumn in count_test.go using the reporter's exact tuples.

Validation

go test -tags gms_pure_go ./sql/expression/function/aggregation/
  • New test: RED on main (returns 1, expected 2), GREEN with the fix.
  • Full aggregation package suite passes.
  • gofmt/goimports -local github.com/dolthub/go-mysql-server clean.

Happy to adjust.

The multi-column path joined each converted value with a comma separator,
so distinct tuples like ('a,','b') and ('a',',b') produced the identical
string 'a,,b,' and hashed to the same bucket. Length-prefix each value so
the encoding is injective.
@elianddb elianddb closed this Sep 5, 2026
@elianddb elianddb self-assigned this Sep 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants