Skip to content

Support COUNT(DISTINCT uuid) - #3182

Merged
fulghum merged 1 commit into
mainfrom
fulghum/doltgres-3099-count-distinct-uuid
Aug 25, 2026
Merged

Support COUNT(DISTINCT uuid)#3182
fulghum merged 1 commit into
mainfrom
fulghum/doltgres-3099-count-distinct-uuid

Conversation

@fulghum

@fulghum fulghum commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Adds PostgreSQL regression coverage for COUNT(DISTINCT ...) over UUID values and pins the GMS implementation that hashes extended types using their canonical serialized representation.

Part of #3099

Depends on: dolthub/go-mysql-server#3727

@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor
Main PR
Total 42090 42090
Successful 19038 19037
Failures 23052 23053
Partial Successes1 5477 5477
Main PR
Successful 45.2316% 45.2293%
Failures 54.7684% 54.7707%

${\color{red}Regressions (1)}$

random

QUERY:          (SELECT unique1 AS random
  FROM onek ORDER BY random() LIMIT 1)
INTERSECT
(SELECT unique1 AS random
  FROM onek ORDER BY random() LIMIT 1)
INTERSECT
(SELECT unique1 AS random
  FROM onek ORDER BY random() LIMIT 1);
RECEIVED ERROR: expected row count 0 but received 1

Footnotes

  1. These are tests that we're marking as Successful, however they do not match the expected output in some way. This is due to small differences, such as different wording on the error messages, or the column names being incorrect while the data itself is correct.

@itoqa

itoqa Bot commented Aug 25, 2026

Copy link
Copy Markdown

Ito QA test results
Commit: 58f80fa: 8 test cases ran, 8 passed ✅.

Summary

Coverage focuses on distinct-value counting across grouped and ungrouped data, including duplicate and missing values, shared values between groups, equivalent UUID formats, type conversions, reordered inputs, derived queries, and fresh database sessions. These are primarily data-integrity and edge-case checks spanning direct queries, client integration, and expression handling, with healthy results throughout.

Safe to merge — the exercised counting and UUID-handling behaviors all passed, with no regressions, new failures, or previously flagged failures attributable to this PR. No merge-blocking issue was identified.

Tests run by Ito

View full run

Result Severity Type Description
General Each group returned the correct number of unique UUIDs, even when one UUID was shared across groups. Duplicate values were counted once, NULL values were ignored, and changing the input order did not change the results.
General Grouped queries counted each unique UUID, text value, and integer correctly, even when values came from direct columns or casts. Reordering the expressions and using a derived query gave the same results, and every count was reported as bigint.
General The database returned the correct number of unique UUIDs in two fresh client sessions, including direct values and values created through casts.
Count The database returned 3 distinct UUIDs, 3 distinct text values, and 3 distinct integers. The count result was reported as the expected bigint type.
Rev The database counted uppercase, braced, and lowercase spellings of the same UUID as one value and ignored the NULL row.
Rev A PostgreSQL client returned the correct distinct UUID count and the correct count for each group. The temporary table was removed and the client exited cleanly.
Uuid The database counted three unique UUID values from six rows, ignored duplicate rows, and excluded the NULL value.
Uuid The database counted the three unique non-empty UUID values correctly after direct casts, text casts, and brace-based expressions. Each required count was 3.

Tip

Reply with @itoqa to send us feedback on this test run.

@coffeegoddd

coffeegoddd commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

@fulghum DOLT

read_tests from_latency to_latency percent_change
covering_index_scan_postgres 2.43 2.43 0.0
groupby_scan_postgres 77.19 77.19 0.0
index_join_postgres 2.26 2.26 0.0
index_join_scan_postgres 1.58 1.58 0.0
index_scan_postgres 493.24 484.44 -1.78
oltp_point_select 0.37 0.37 0.0
oltp_read_only 6.43 6.43 0.0
select_random_points 0.72 0.72 0.0
select_random_ranges 1.03 1.01 -1.94
table_scan_postgres 484.44 475.79 -1.79
types_table_scan_postgres 1235.62 1213.57 -1.78
write_tests from_latency to_latency percent_change
oltp_delete_insert_postgres 6.67 6.67 0.0
oltp_insert 3.36 3.36 0.0
oltp_read_write 13.46 13.46 0.0
oltp_update_index 3.55 3.55 0.0
oltp_update_non_index 3.25 3.25 0.0
oltp_write_only 7.04 7.04 0.0
types_delete_insert_postgres 7.17 7.17 0.0

@zachmu zachmu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@fulghum
fulghum force-pushed the fulghum/doltgres-3099-count-distinct-uuid branch from 58f80fa to 237f32d Compare August 25, 2026 21:23
@itoqa

itoqa Bot commented Aug 25, 2026

Copy link
Copy Markdown

Ito QA test results

History reset (rebase or force-push detected). Starting test narrative over.

Commit: 237f32d: 9 test cases ran, 3 passed ✅, 6 additional findings ⚠️.

Summary

The run covers standard distinct-value counting, empty and NULL-only inputs, grouped results, equivalent value representations, and client-facing result handling for UUID data. Edge cases and regression-oriented checks are exercised, with the change itself appearing behaviorally neutral while exposing existing failures in non-empty UUID aggregate paths.

Safe to merge — the PR introduces no regressions and none of the observed failures are attributable to its changes. Existing medium-severity UUID aggregate and result-conversion defects remain flag-for-later issues rather than merge blockers for this coverage-only change.

Tests run by Ito

View full run

Result Severity Type Description
Uuid Counting distinct UUIDs after selecting only NULL values returns 0, as expected.
Uuid Counting distinct values from no matching rows returns 0 and the query completes successfully.
Uuid Verified acceptable by independent adversarial review: the reported expectation does not match what the code actually promises. Review notes: The finding assumes the checked-out engine still sends UUID distinct keys through an incompatible text conversion and needs an unimplemented aggregate fix, but the source exposes canonical UUID serialization through the ExtendedType contract and pins the GMS revision that the PR identifies as consuming that representation for hashing. The PR diff adds only regression coverage, so it does not intro…
⚠️ Medium severity General The comparison query failed with an internal conversion error instead of returning three for both UUID expressions. The brace-wrapped UUID expression failed in the same way.
⚠️ Medium severity General The grouped distinct-count query fails with an incompatible conversion from UUID to text for UUID 00000000-0000-0000-0000-000000000002. No group rows are returned, so the test cannot verify partition-local distinct state or the expected relationship to the global count.
⚠️ Medium severity Rev The test was found, but counting distinct UUID values returned a server error instead of the expected result.
⚠️ Medium severity Rev The grouped query did not return the expected count rows. It returned SQLSTATE XX000 with an incompatible conversion error for a UUID value while preparing the PostgreSQL protocol response.
⚠️ Medium severity Rev The table setup and all five inserts succeed, but the distinct-count queries fail during result conversion. The expected behavior is one count for equivalent UUID representations, a second count for the different UUID, and normalized UUID output without a conversion error.
⚠️ Medium severity Uuid The database returned an error while counting the distinct UUID rows, so the expected count of 3 and the bigint result type could not be returned. The NULL-only and empty-input checks pass, which narrows the failure to non-empty UUID aggregate results.
Additional Findings Details

These findings are unrelated to the current changes but were observed during testing.

🟡 Equivalent UUID values cannot be counted together
  • Severity: Medium Medium severity
  • Description: The comparison query failed with an internal conversion error instead of returning three for both UUID expressions. The brace-wrapped UUID expression failed in the same way.
  • Impact: Users cannot count distinct UUID values when those values come from text or cast expressions, and the query returns an internal error instead of a result. Queries that use the UUID column directly continue to work, but rewriting the query may not be practical in every application.
  • Steps to Reproduce:
    1. Create a table with a UUID column and a text column containing the same three UUID values, including duplicates and NULL rows.
    2. Run SELECT COUNT(DISTINCT (u::text)::uuid), COUNT(DISTINCT uuid_text::uuid) FROM uuid_distinct;.
    3. Run SELECT COUNT(DISTINCT ('{' || uuid_text || '}')::uuid) FROM uuid_distinct;.
    4. Check that each expression returns three without an execution error.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: testing/go/count_distinct_test.go:59-66 defines the exact cross-representation contract: COUNT(DISTINCT (u::text)::uuid), COUNT(DISTINCT uuid_text::uuid), and COUNT(DISTINCT ('{' || uuid_text || '}')::uuid) must each return 3. The recorded focused TestCountDistinctUuid execution instead returns SQLSTATE XX000, incompatible conversion to SQL type: '00000000-0000-0000-0000-000000000002'->text, for the cast-based aggregates. This is not explained by missing fixture data: the setup inserts the same canonical values into both u and uuid_text at testing/go/count_distinct_test.go:27-36, and the direct UUID control COUNT(DISTINCT u) returns 3 with type bigint. The local UUID implementation represents UUIDs as a fixed 16-byte uuid.UUID value (server/types/uuid.go:24-59, especially serializeTypeUuid at lines 62-66 and deserializeTypeUuid at lines 68-75), so the failing conversion is in the aggregate/type-coercion integration rather than an invalid UUID literal. The resolved go-mysql-server dependency at go.mod:12 supplies that aggregate implementation; the local PR diff adds no production fix for it. The smallest practical fix is to correct the aggregate's distinct-key normalization/conversion so UUID values and UUID values produced by text casts are normalized to a comparable canonical key before duplicate elimination, while preserving NULL exclusion and bigint COUNT results.
Evidence Package
🟡 Grouped UUID counts fail before returning results
  • Severity: Medium Medium severity
  • Description: The grouped distinct-count query fails with an incompatible conversion from UUID to text for UUID 00000000-0000-0000-0000-000000000002. No group rows are returned, so the test cannot verify partition-local distinct state or the expected relationship to the global count.
  • Impact: Queries that count distinct UUID values fail with a server error instead of returning results. This blocks grouped reports that rely on those counts, but does not show data loss or corruption.
  • Steps to Reproduce:
    1. Create a table with a text group column and a UUID column.
    2. Insert duplicate UUIDs within groups a and b, include UUID 2 in both groups, and include NULL UUID rows.
    3. Run SELECT g, COUNT(DISTINCT u) FROM uuid_distinct GROUP BY g ORDER BY g.
    4. Observe the SQLSTATE XX000 error instead of rows a=2 and b=2.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: The added regression fixture at testing/go/count_distinct_test.go:23-80 defines the expected grouped result at lines 52-57: group a and group b should each return two distinct non-NULL UUIDs. The UUID production type at server/types/uuid.go:24-59 is an extended Doltgres type with a 16-byte SerializeValue implementation at lines 62-66 and UUID deserialization at lines 68-75. Its SerializedCompare path in server/types/type.go:994-1025 falls back to deserializing values and comparing them for non-string types, while the output path in server/types/type.go:700-712 delegates formatting to the UUID output function. The focused run and the minimal SQL reproduction both fail with SQLSTATE XX000 while processing COUNT(DISTINCT u), including the grouped form, which is consistent with the aggregate's extended-type state being converted through an incompatible text representation before results are emitted. The NULL-only and empty-input controls pass, so the failure is specific to non-NULL UUID values rather than fixture creation or COUNT semantics in general. The smallest practical fix is in the aggregate/extended-type conversion path supplied by the go-mysql-server dependency: preserve UUID's canonical serialized bytes or convert them through the UUID type's output/serialization contract when hashing and materializing DISTINCT state, then add this grouped query as a regression case in that implementation layer.
Evidence Package
🟡 UUID count test fails after it is discovered
  • Severity: Medium Medium severity
  • Description: The test was found, but counting distinct UUID values returned a server error instead of the expected result.
  • Impact: Users running distinct counts on UUID data may receive a server error instead of a count. Other query types may still work, and there is no evidence of data loss or exposure.
  • Steps to Reproduce:
    1. Run go test ./testing/go -run ^TestCountDistinctUuid$ -count=1 in the repository's test environment.
    2. Let the test create uuid_distinct with a UUID column and insert the duplicate and NULL UUID fixture rows.
    3. Observe that TestCountDistinctUuid/COUNT_DISTINCT_uuid is listed, then fails before the expected count results are returned with an incompatible UUID-to-text conversion error.
    4. Run go test ./testing/go -count=1 and observe that the complete testing/go package also ends with FAIL.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: testing/go/count_distinct_test.go:23-80 defines TestCountDistinctUuid and supplies a valid fixture with UUID values, duplicates, and NULLs. Its first assertion at lines 40-42 requests SELECT COUNT(DISTINCT u) and expects 3; later assertions cover NULL-only input, empty input, grouping, UUID casts, mixed controls, and pg_typeof(...), so this is not a registration-only check. The focused output lists TestCountDistinctUuid and its subtest, proving Go test discovery and execution succeeded before the assertion failed. The runtime error is SQLSTATE XX000, incompatible conversion from the UUID value 00000000-0000-0000-0000-000000000002 to text, and the complete testing/go package also fails. In the production path, server/types/uuid.go:24-60 registers UUID as a native Doltgres type with serialization and deserialization functions, while lines 62-75 serialize UUID values as 16-byte data and reconstruct uuid.UUID values. server/functions/uuid.go:38-65 defines uuid_in and uuid_out; uuid_out at lines 57-65 explicitly converts a uuid.UUID to its canonical string. Together these paths confirm that the input is a native UUID value and that a valid aggregate result is reaching a UUID/text conversion boundary, rather than failing because the test name, package, fixture setup, or discovery is invalid. The PR context shows one changed file, the added testing/go/count_distinct_test.go; no production UUID, result-conversion, aggregate, go.mod, or dependency file is in the PR diff. The smallest practical fix is in the existing aggregate/result conversion path or its resolved go-mysql-server implementation: preserve the COUNT result as its numeric type and apply UUID output conversion only to UUID-typed values, rather than attempting to convert the UUID aggregate value to text. That fix is outside this PR's changed code and should be made in the responsible engine dependency or server conversion layer.
Evidence Package
🟡 Grouped distinct counts fail for UUID values
  • Severity: Medium Medium severity
  • Description: The grouped query did not return the expected count rows. It returned SQLSTATE XX000 with an incompatible conversion error for a UUID value while preparing the PostgreSQL protocol response.
  • Impact: Clients cannot retrieve grouped distinct counts when the counted column contains UUID values. Queries that use this valid pattern fail instead of returning the expected results.
  • Steps to Reproduce:
    1. Connect a PostgreSQL client to the local server using the test connection settings.
    2. Create a table with a text group column and a UUID column, then insert the duplicate and NULL UUID rows from testing/go/count_distinct_test.go.
    3. Run SELECT g, COUNT(DISTINCT u) FROM uuid_distinct GROUP BY g ORDER BY g;.
    4. Check the client response. The expected rows are a=2 and b=2, but the server returns a UUID-to-text conversion error instead.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: The regression test in testing/go/count_distinct_test.go:23-80 defines a valid fixture with six rows, duplicate UUIDs, and NULLs. Its grouped assertion at lines 52-57 expects two rows, {"a", 2} and {"b", 2}; the recorded protocol run successfully created the table and inserted all six rows before failing on the aggregate response, so this is not a setup or query-syntax problem. The production UUID type in server/types/uuid.go:24-60 uses uuid.UUID as its native runtime value and assigns uuid_out as the PostgreSQL output function. server/types/uuid.go:62-75 serializes and deserializes UUIDs as their 16-byte representation, while server/functions/uuid.go:57-65 converts a UUID to canonical text only through uuid_out. The observed error shows that the aggregate result path instead attempts a generic UUID-to-text conversion and rejects the native uuid.UUID value before encoding the row for the client. The smallest practical fix is in the resolved go-mysql-server aggregate/result-conversion path: preserve the COUNT result as int64 and serialize UUID-backed values through the registered UUID output/serialization behavior rather than requiring a generic string conversion. This repository PR does not contain that implementation change, so the fix is not a targeted change to the added test file.
Evidence Package
🟡 UUID distinct counts return a conversion error
  • Severity: Medium Medium severity
  • Description: The table setup and all five inserts succeed, but the distinct-count queries fail during result conversion. The expected behavior is one count for equivalent UUID representations, a second count for the different UUID, and normalized UUID output without a conversion error.
  • Impact: Users cannot count distinct UUID values with standard database queries or receive the expected UUID result. This can block reports and data checks that rely on distinct UUID counts, but the evidence is limited to this UUID query path.
  • Steps to Reproduce:
    1. Connect to the local PostgreSQL-compatible server with a PostgreSQL client.
    2. Create a table with a UUID column and insert the same UUID in canonical, uppercase, compact, and braced forms plus one different UUID.
    3. Run COUNT(DISTINCT uuid) and COUNT(DISTINCT uuid::text).
    4. Observe that the server returns the UUID-to-text conversion error instead of the expected distinct counts.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: The source confirms that UUID values are a native application value, not ordinary strings: server/types/uuid.go:24-41 registers the UUID type and server/types/uuid.go:64-75 serializes and deserializes it as a uuid.UUID. The comparison implementation in server/types/type.go:429-431 compares UUID byte values, so duplicate elimination can distinguish UUID identities. However, server/types/type.go:493-600 only accepts values already typed as uuid.UUID when converting to the UUID type, while server/functions/uuid.go:57-65 exposes uuid_out as the explicit canonical string formatter. The observed uuid->text error means the aggregate result reaches a conversion boundary that does not use the UUID output function or otherwise handle uuid.UUID as text. The practical fix is to correct that aggregate/result conversion boundary, or its resolved go-mysql-server implementation, so UUID aggregate values are converted through the UUID output representation before PostgreSQL text results are encoded; the existing UUID comparison and normalization logic should remain unchanged.
Evidence Package
🟡 Counting unique UUIDs returns a server error
  • Severity: Medium Medium severity
  • Description: The database returned an error while counting the distinct UUID rows, so the expected count of 3 and the bigint result type could not be returned. The NULL-only and empty-input checks pass, which narrows the failure to non-empty UUID aggregate results.
  • Impact: Users cannot get distinct counts for non-empty UUID columns because the query fails instead of returning a count.
  • Steps to Reproduce:
    1. Run the focused Go test with go test -count=1 -run ^TestCountDistinctUuid$ ./... from testing/go.
    2. Create the uuid_distinct table and insert the fixture rows containing three distinct non-NULL UUIDs, duplicates, and NULLs.
    3. Run SELECT COUNT(DISTINCT u) FROM uuid_distinct;.
    4. Observe SQLSTATE XX000 with an incompatible conversion from a UUID value to text instead of the expected count of 3.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: The added regression fixture at testing/go/count_distinct_test.go:23-77 creates a UUID column with duplicate values 000...001 and 000...002, a third value 000...003, and NULL rows. Its primary assertion at lines 40-42 expects COUNT(DISTINCT u) to return 3; the focused run instead returns SQLSTATE XX000: incompatible conversion of 00000000-0000-0000-0000-000000000002 to text. The same error affects the grouped assertion at lines 52-57, the cast-equivalent UUID assertions at lines 59-67, the mixed control query at lines 69-73, and pg_typeof at lines 75-77, while lines 44-49 pass for NULL-only and empty input. In the unchanged production type definition, server/types/uuid.go:24-59 registers UUID as an extended type and serializes it as its 16-byte canonical representation through serializeTypeUuid and deserializeTypeUuid. The output path in server/types/type.go:743-756 invokes the UUID output function and requires a string result. The failure therefore points to the engine/dependency path that feeds a UUID aggregate result into text conversion or result decoding when the aggregate has non-empty values; it is not explained by fixture setup or the browser probe. The smallest practical fix is to correct UUID value handling in that aggregate/result-conversion path, preserving the UUID's canonical representation until the normal UUID output function formats it, then retain this regression test to verify the count and bigint type.
Evidence Package

Tip

Reply with @itoqa to send us feedback on this test run.

@fulghum
fulghum force-pushed the fulghum/doltgres-3099-count-distinct-uuid branch from 237f32d to a9266a9 Compare August 25, 2026 23:15
@itoqa

itoqa Bot commented Aug 25, 2026

Copy link
Copy Markdown

Ito QA test results

History reset (rebase or force-push detected). Starting test narrative over.

Commit: a9266a9: 9 test cases ran, 9 passed ✅.

Summary

Coverage focuses on database business logic for distinct UUID counting, including grouping, duplicate handling, null and empty inputs, UUID format conversions, bound values, repeated queries, and result types. Both targeted regression checks and the broader test suite exercised normal behavior plus edge cases around state isolation and query execution.

Safe to merge — all exercised behaviors passed, including focused regression checks and the full package suite, with no PR-attributable regressions or new failures. Overall merge risk is low.

Tests run by Ito

View full run

Result Severity Type Description
General Both groups returned the correct independent counts for UUIDs, converted UUIDs, integers, and brace-form UUIDs. Null values were excluded and the count type stayed bigint in both query orders.
General Each group returned two unique values after the UUID conversion, matching the direct count. The shared value did not change either group's result, including when the groups were checked separately on the same connection.
General The database returned three distinct UUIDs for populated rows, zero for NULL-only and empty results, and three again when the populated query was repeated.
Count The database counted each different UUID once, ignored NULL values, kept counts separate for groups, and returned the expected bigint result type.
Count The database returned a distinct count of 3 for direct UUID values and for each tested UUID cast. Duplicate values were counted once and NULL values were left out.
Count The database counted the three different UUID values as three distinct values while still collapsing repeated rows.
Rev Canonical, braced, and uppercase forms of the same UUID were counted as one value. The query returned 2 after one different UUID was added.
Rev Queries using bound UUID values returned the expected distinct counts for both UUIDs, a NULL value, and a repeated UUID. Each execution completed without a protocol or connection error.
Rev The new UUID counting test passed, and the full Go test suite passed as well. The test server started, all SQL checks completed, and no cleanup or unrelated test errors occurred.

Tip

Reply with @itoqa to send us feedback on this test run.

@fulghum
fulghum merged commit 9f49781 into main Aug 25, 2026
24 checks passed
@fulghum
fulghum deleted the fulghum/doltgres-3099-count-distinct-uuid branch August 25, 2026 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants