[UUID 5/8] UUID aggregation, group-by and distinct

[xiangfu0]

Rebased onto master now that #18872 has merged — this PR was stacked on it and carried its 13 files. It is now just the aggregation / group-by / distinct work: 19 files, +967 −77 (was 31 files, +1212).

What

UUID support across the aggregation and grouping paths:

• Group key generation — NoDictionarySingleColumnGroupKeyGenerator and NoDictionaryMultiColumnGroupKeyGenerator handle UUID, backed by a new UuidToIdMap registered in ValueToIdMapFactory. Group keys use UuidKey (two primitive longs) rather than ByteArray.
• Distinct-count family — DistinctCountBitmap, DistinctCountHLL, DistinctCountHLLPlus, DistinctCountCPCSketch, DistinctCountThetaSketch, DistinctCountULL and IntegerTupleSketch.
• DISTINCT — BytesDistinctTable formats result rows through ColumnDataType#convertAndFormat instead of hard-coding toHexString(), so a UUID column returns canonical UUIDs while BYTES still returns hex.
• ANY_VALUE and the shared AggregationFunctionUtils plumbing.

A bug this PR would have introduced, and its fix

The original version added case UUID to GroupByDataTableReducer#getConvertedKey as a fall-through to BYTES, returning the raw byte[].

That breaks against #18872, now on master: the other reduce path converts group keys with ColumnDataType#convert, and UUID is the one type whose converted form is not its stored bytes — it yields a java.util.UUID. PredicateRowMatcher casts directly on that, so a byte[] produces ClassCastException: class [B cannot be cast to class java.util.UUID on GROUP BY <uuidCol> ... HAVING <uuidCol> = ....

Fixed by delegating rather than duplicating the knowledge, so the two paths cannot diverge again:

case UUID:
  return columnDataType.convert(dataTable.getBytes(rowId, colId));

Covered by an integration test, not a unit test. The unit-level BaseQueriesTest harness reduces two data tables and so cannot reach this code — which is exactly why the path was uncovered. The new UuidAggregationTest goes through a real broker reduce. I checked the test is actually worth something by reverting the fix: testGroupByUuidColumnWithHaving and testGroupByUuidColumnWithHavingReturningFinalResult both fail with the ClassCastException, and both pass with it.

Review follow-up: UuidToIdMap now casts directly

UuidToIdMap called UuidKey.fromObject(value), which accepts five input types. Both callers already key on UuidKey (via UuidKey.fromBytes), so exactly one type is ever passed and the rest was an instanceof chain per row in the group-by loop. It now casts directly, matching the sibling maps (DoubleToIdMap casts to double) and keeping the input type deterministic — the same property asked for on the equivalent PredicateRowMatcher branch in #18872.

Distinct-count hashes the stored bytes

An earlier revision rendered each UUID as its 36-char canonical string so that DISTINCTCOUNTHLL(uuidCol) would equal DISTINCTCOUNTHLL(CAST(uuidCol AS STRING)). No other logical type provides that guarantee — the scan path switches on the stored type, so TIMESTAMP hashes its raw millis and BOOLEAN its int, and neither matches a CAST to STRING. It also cost a String allocation per row in the aggregation loop.

UUID now hashes its stored 16 bytes, which is what "the stored type is the contract" means for a logical type. I verified byte[] is content-hashed rather than identity-hashed by both HyperLogLog (clearspring MurmurHash) and UltraLogLogUtils.OBJECT_FUNNEL (putBytes) before relying on it.

A minimal guard is still needed at each site: unlike LONG or INT, the stored BYTES type is not a scalar case in this function family — the scan path has no case BYTES at all, and the dictionary path reads BYTES as serialized sketch state. The guard exists only to avoid that misreading:

• AggregationFunctionUtils — the three duplicated UUID blocks are gone. The BYTES guard now excludes UUID so it falls through to the scalar path, which offers dictionary.get(i) (the stored byte[]), exactly as the scan path does. That file went from +55/-9 to +27/-12.
• DistinctCountBitmap hashes Arrays.hashCode(bytes).
• DistinctCountThetaSketch is the one exception: DataType.BYTES there means "serialized sketch" with no scalar-bytes mode, so it must surface a String. It now uses the stored hex rendering, matching every other String-typed UUID boundary (PredicateUtils#getStoredValue, the Bloom filter key).

testUuidDistinctCountHllMatchesStringDistinctCountHll asserted the invariant being dropped; it is replaced by a test pinning the new behaviour so the canonical-string rendering is not quietly reintroduced.

Verification

Full pinot-core reactor on this branch:

Pinot local segment implementations ... SUCCESS [10:29 min]   5136 tests, 0 failures, 0 errors
Pinot Core .......................... SUCCESS [19:43 min]   6894 tests, 0 failures, 0 errors, 1 skipped
BUILD SUCCESS

The per-module counts are the evidence, not the exit code: the run used -Dmaven.test.failure.ignore=true (needed because pinot-core is skipped entirely if an upstream module fails), so BUILD SUCCESS alone would not prove anything. There are zero <<< FAILURE lines in the log.

Integration: UuidAggregationTest + UuidBloomFilterTest — 7 tests, 0 failures.

Part of the #18140 UUID split. Depends on parts 1, 2 and 4, all merged.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 25.86207% with 129 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.63%. Comparing base (3ffb614) to head (aa76511).

Files with missing lines	Patch %	Lines
.../function/DistinctCountULLAggregationFunction.java	0.00%	26 Missing ⚠️
...ion/DistinctCountCPCSketchAggregationFunction.java	0.00%	24 Missing ⚠️
...nction/DistinctCountBitmapAggregationFunction.java	24.00%	16 Missing and 3 partials ⚠️
...n/DistinctCountThetaSketchAggregationFunction.java	20.83%	18 Missing and 1 partial ⚠️
...ction/DistinctCountHLLPlusAggregationFunction.java	25.00%	15 Missing and 3 partials ⚠️
.../function/DistinctCountHLLAggregationFunction.java	50.00%	10 Missing and 2 partials ⚠️
...aggregation/function/AggregationFunctionUtils.java	53.33%	5 Missing and 2 partials ⚠️
.../core/query/distinct/table/BytesDistinctTable.java	72.72%	3 Missing ⚠️
...not/core/query/reduce/GroupByDataTableReducer.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             master   #18873    +/-   ##
==========================================
  Coverage     66.63%   66.63%            
  Complexity     1423     1423            
==========================================
  Files          3443     3443            
  Lines        218663   218798   +135     
  Branches      34801    34847    +46     
==========================================
+ Hits         145705   145802    +97     
- Misses        61230    61265    +35     
- Partials      11728    11731     +3     

Flag	Coverage Δ
integration	100.00% <ø> (ø)
integration1	100.00% <ø> (ø)
integration2	0.00% <ø> (ø)
java-25	66.63% <25.86%> (+<0.01%)	⬆️
lane-a	100.00% <ø> (ø)
lane-b	0.00% <ø> (ø)
temurin	66.63% <25.86%> (+<0.01%)	⬆️
unittests	66.63% <25.86%> (+<0.01%)	⬆️
unittests1	57.29% <25.86%> (-0.01%)	⬇️
unittests2	38.88% <0.00%> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

Pull request overview

Adds end-to-end support for Pinot’s logical UUID type across aggregation/group-by and DISTINCT-style result paths, aligning behavior between stored BYTES (16-byte) representation and canonical UUID semantics in query results and sketch-based distinct-count functions.

Changes:

• Extend no-dictionary group key generation to treat UUID as a first-class logical type using UuidKey/UuidToIdMap (instead of generic ByteArray) for faster hashing/equality.
• Fix broker reduce group-key conversion for UUID (GroupByDataTableReducer#getConvertedKey) to return the converted java.util.UUID form instead of raw bytes.
• Update DISTINCT and distinct-count aggregations (HLL/HLLPlus/ULL/CPC/Theta/Bitmap) plus ANY_VALUE/utility paths to render/hash UUIDs consistently (canonical UUID strings), with new unit + integration coverage.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/custom/UuidAggregationTest.java	Integration coverage for UUID GROUP BY, HAVING reduce branches, DISTINCT formatting, and distinct-count behaviors.
pinot-core/src/test/java/org/apache/pinot/core/query/distinct/table/BytesDistinctTableTest.java	Verifies DISTINCT result formatting differs correctly for UUID vs BYTES.
pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionaryGroupKeyGeneratorTest.java	Extends group key generator tests to include UUID (and additional logical types).
pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLAggregationFunctionTest.java	Adds regression + contract tests for UUID hashing behavior in HLL distinct count.
pinot-core/src/main/java/org/apache/pinot/core/query/reduce/GroupByDataTableReducer.java	Ensures UUID group keys are converted via ColumnDataType#convert to avoid reducer-path divergence.
pinot-core/src/main/java/org/apache/pinot/core/query/distinct/table/BytesDistinctTable.java	Formats DISTINCT output via ColumnDataType#convertAndFormat rather than hard-coded hex.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/utils/ValueToIdMapFactory.java	Registers UUID-specific on-the-fly dictionary map.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/utils/UuidToIdMap.java	Adds UUID-specialized ValueToIdMap using UuidKey keys and ByteArray storage.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionarySingleColumnGroupKeyGenerator.java	Preserves UUID logical type for group-key dispatch and uses UuidKey in hot paths.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionaryMultiColumnGroupKeyGenerator.java	Preserves UUID logical type per column and routes raw bytes through UuidKey for on-the-fly dictionaries.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/IntegerTupleSketchAggregationFunction.java	Rejects raw UUID inputs with a clearer error instead of sketch-deserialization failures.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountULLAggregationFunction.java	Hashes canonical UUID strings for UUID-typed columns to match STRING semantics.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountThetaSketchAggregationFunction.java	Converts UUID bytes to canonical strings before updating theta sketches to avoid treating them as serialized sketches.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLPlusAggregationFunction.java	Offers canonical UUID strings for UUID columns rather than deserializing raw bytes.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLAggregationFunction.java	Offers canonical UUID strings for UUID columns rather than deserializing raw bytes.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountCPCSketchAggregationFunction.java	Updates CPC sketch with canonical UUID strings for UUID-typed columns.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountBitmapAggregationFunction.java	Adds UUID handling by hashing canonical UUID strings into the bitmap path.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AnyValueAggregationFunction.java	Ensures UUID columns return ColumnDataType.UUID for canonical rendering.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionUtils.java	Handles UUID dictionary entries for distinct-count sketch results by offering canonical UUID strings instead of treating dictionary BYTES as serialized sketches.

Suppressed comments (1)

pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/custom/UuidAggregationTest.java:148

• This comment block contains lines over the 120-character Checkstyle LineLength limit (config/checkstyle.xml). Please reflow the comment to avoid checkstyle:check failures.

  /// The `serverReturnFinalResult` variant. This is the option that gates the single-data-table branch in
  /// `GroupByDataTableReducer#reduceAndSetResults` (`isServerReturnFinalResult() && dataTables.size() == 1`), so it
  /// covers a different reduce branch from the test above. Verified by reverting the `getConvertedKey` fix: both
  /// this and the plain `GROUP BY ... HAVING` test fail with
  /// `ClassCastException: class [B cannot be cast to class java.util.UUID`, and both pass with it.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.