From 83dd3fe44e9f6f417e42d900b0a2599fc2d59b95 Mon Sep 17 00:00:00 2001 From: Xiang Fu Date: Tue, 7 Jul 2026 14:27:43 -0700 Subject: [PATCH 1/8] [UUID 5/8] UUID aggregation, group-by and distinct Part 5/8 of splitting apache/pinot#18140 (logical UUID type). Rebased onto latest master; stacked on uuid-split/04-sse-predicates-cast. Downstream references use the UuidKey class merged in #18869. --- .../function/AggregationFunctionUtils.java | 55 +++++++++-- .../function/AnyValueAggregationFunction.java | 7 ++ ...istinctCountBitmapAggregationFunction.java | 47 ++++++++- ...inctCountCPCSketchAggregationFunction.java | 53 ++++++++-- .../DistinctCountHLLAggregationFunction.java | 48 +++++++++- ...stinctCountHLLPlusAggregationFunction.java | 45 ++++++++- ...ctCountThetaSketchAggregationFunction.java | 42 +++++++- .../DistinctCountULLAggregationFunction.java | 51 +++++++++- ...IntegerTupleSketchAggregationFunction.java | 25 ++++- ...ictionaryMultiColumnGroupKeyGenerator.java | 68 ++++++++++--- ...ctionarySingleColumnGroupKeyGenerator.java | 91 +++++++++++++++--- .../groupby/utils/UuidToIdMap.java | 59 ++++++++++++ .../groupby/utils/ValueToIdMapFactory.java | 2 + .../distinct/table/BytesDistinctTable.java | 16 +++- .../query/reduce/GroupByDataTableReducer.java | 1 + ...stinctCountHLLAggregationFunctionTest.java | 96 +++++++++++++++++++ .../NoDictionaryGroupKeyGeneratorTest.java | 48 ++++++++-- .../table/BytesDistinctTableTest.java | 70 ++++++++++++++ 18 files changed, 747 insertions(+), 77 deletions(-) create mode 100644 pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/utils/UuidToIdMap.java create mode 100644 pinot-core/src/test/java/org/apache/pinot/core/query/distinct/table/BytesDistinctTableTest.java diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionUtils.java b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionUtils.java index 527b7817621c..c5edbd429485 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionUtils.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionUtils.java @@ -74,6 +74,7 @@ import org.apache.pinot.spi.data.FieldSpec; import org.apache.pinot.spi.query.QueryThreadContext; import org.apache.pinot.spi.utils.ByteArray; +import org.apache.pinot.spi.utils.UuidUtils; /// The `AggregationFunctionUtils` class provides utility methods for aggregation function. @@ -610,23 +611,23 @@ public static Object getAggregationResult(AggregationFunction aggregationFunctio break; case DISTINCTCOUNTHLL: case DISTINCTCOUNTHLLMV: - result = getDistinctCountHLLResult(Objects.requireNonNull(dataSource.getDictionary()), + result = getDistinctCountHLLResult(dataSource, (DistinctCountHLLAggregationFunction) aggregationFunction, explainPlanName); break; case DISTINCTCOUNTRAWHLL: case DISTINCTCOUNTRAWHLLMV: - result = getDistinctCountHLLResult(Objects.requireNonNull(dataSource.getDictionary()), + result = getDistinctCountHLLResult(dataSource, ((DistinctCountRawHLLAggregationFunction) aggregationFunction).getDistinctCountHLLAggregationFunction(), explainPlanName); break; case DISTINCTCOUNTHLLPLUS: case DISTINCTCOUNTHLLPLUSMV: - result = getDistinctCountHLLPlusResult(Objects.requireNonNull(dataSource.getDictionary()), + result = getDistinctCountHLLPlusResult(dataSource, (DistinctCountHLLPlusAggregationFunction) aggregationFunction, explainPlanName); break; case DISTINCTCOUNTRAWHLLPLUS: case DISTINCTCOUNTRAWHLLPLUSMV: - result = getDistinctCountHLLPlusResult(Objects.requireNonNull(dataSource.getDictionary()), + result = getDistinctCountHLLPlusResult(dataSource, ((DistinctCountRawHLLPlusAggregationFunction) aggregationFunction) .getDistinctCountHLLPlusAggregationFunction(), explainPlanName); break; @@ -642,7 +643,7 @@ public static Object getAggregationResult(AggregationFunction aggregationFunctio (DistinctCountSmartHLLPlusAggregationFunction) aggregationFunction, explainPlanName); break; case DISTINCTCOUNTULL: - result = getDistinctCountULLResult(Objects.requireNonNull(dataSource.getDictionary()), + result = getDistinctCountULLResult(dataSource, (DistinctCountULLAggregationFunction) aggregationFunction, explainPlanName); break; case DISTINCTCOUNTSMARTULL: @@ -650,7 +651,7 @@ public static Object getAggregationResult(AggregationFunction aggregationFunctio (DistinctCountSmartULLAggregationFunction) aggregationFunction, explainPlanName); break; case DISTINCTCOUNTRAWULL: - result = getDistinctCountULLResult(Objects.requireNonNull(dataSource.getDictionary()), + result = getDistinctCountULLResult(dataSource, (DistinctCountULLAggregationFunction) aggregationFunction, explainPlanName); break; default: @@ -799,8 +800,20 @@ private static HyperLogLogPlus getDistinctValueHLLPlus(Dictionary dictionary, in return hllPlus; } - private static HyperLogLog getDistinctCountHLLResult(Dictionary dictionary, + private static HyperLogLog getDistinctCountHLLResult(DataSource dataSource, DistinctCountHLLAggregationFunction function, String explainPlanName) { + Dictionary dictionary = Objects.requireNonNull(dataSource.getDictionary()); + // UUID dictionary entries are logical scalar values, not serialized HyperLogLogs. Offer their canonical string + // representation to match the scan-based path and DISTINCTCOUNTHLL(CAST(uuidColumn AS STRING)). + if (dataSource.getDataSourceMetadata().getDataType() == FieldSpec.DataType.UUID) { + HyperLogLog hll = new HyperLogLog(function.getLog2m()); + int length = dictionary.length(); + for (int i = 0; i < length; i++) { + QueryThreadContext.checkTerminationAndSampleUsagePeriodically(i, explainPlanName); + hll.offer(UuidUtils.toString(dictionary.getBytesValue(i))); + } + return hll; + } if (dictionary.getValueType() == FieldSpec.DataType.BYTES) { // Treat BYTES value as serialized HyperLogLog try { @@ -820,8 +833,20 @@ private static HyperLogLog getDistinctCountHLLResult(Dictionary dictionary, } } - private static HyperLogLogPlus getDistinctCountHLLPlusResult(Dictionary dictionary, + private static HyperLogLogPlus getDistinctCountHLLPlusResult(DataSource dataSource, DistinctCountHLLPlusAggregationFunction function, String explainPlanName) { + Dictionary dictionary = Objects.requireNonNull(dataSource.getDictionary()); + // UUID dictionary entries are logical scalar values, not serialized HyperLogLogPluses. Offer their canonical + // string representation to match the scan-based path and DISTINCTCOUNTHLLPLUS(CAST(uuidColumn AS STRING)). + if (dataSource.getDataSourceMetadata().getDataType() == FieldSpec.DataType.UUID) { + HyperLogLogPlus hllPlus = new HyperLogLogPlus(function.getP(), function.getSp()); + int length = dictionary.length(); + for (int i = 0; i < length; i++) { + QueryThreadContext.checkTerminationAndSampleUsagePeriodically(i, explainPlanName); + hllPlus.offer(UuidUtils.toString(dictionary.getBytesValue(i))); + } + return hllPlus; + } if (dictionary.getValueType() == FieldSpec.DataType.BYTES) { // Treat BYTES value as serialized HyperLogLogPlus try { @@ -861,8 +886,20 @@ private static Object getDistinctCountSmartHLLPlusResult(Dictionary dictionary, } } - private static UltraLogLog getDistinctCountULLResult(Dictionary dictionary, + private static UltraLogLog getDistinctCountULLResult(DataSource dataSource, DistinctCountULLAggregationFunction function, String explainPlanName) { + Dictionary dictionary = Objects.requireNonNull(dataSource.getDictionary()); + // UUID dictionary entries are logical scalar values, not serialized UltraLogLogs. Hash their canonical string + // representation to match the scan-based path and DISTINCTCOUNTULL(CAST(uuidColumn AS STRING)). + if (dataSource.getDataSourceMetadata().getDataType() == FieldSpec.DataType.UUID) { + UltraLogLog ull = UltraLogLog.create(function.getP()); + int length = dictionary.length(); + for (int i = 0; i < length; i++) { + QueryThreadContext.checkTerminationAndSampleUsagePeriodically(i, explainPlanName); + UltraLogLogUtils.hashObject(UuidUtils.toString(dictionary.getBytesValue(i))).ifPresent(ull::add); + } + return ull; + } if (dictionary.getValueType() == FieldSpec.DataType.BYTES) { // Treat BYTES value as serialized UltraLogLog and merge try { diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AnyValueAggregationFunction.java b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AnyValueAggregationFunction.java index a649fe6d7a17..7a3ba85337ca 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AnyValueAggregationFunction.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AnyValueAggregationFunction.java @@ -327,6 +327,13 @@ private void ensureResultType(BlockValSet bvs) { if (_resultType != null) { return; } + // Inspect the logical type first so a UUID column reports ColumnDataType.UUID (and the broker renders canonical + // RFC-4122 strings) rather than collapsing to BYTES (which would render hex). All other dispatch keys off the + // stored type, matching the BYTES/STRING storage convention. + if (bvs.getValueType() == FieldSpec.DataType.UUID) { + _resultType = ColumnDataType.UUID; + return; + } switch (bvs.getValueType().getStoredType()) { case INT: _resultType = ColumnDataType.INT; diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountBitmapAggregationFunction.java b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountBitmapAggregationFunction.java index 4fe96b819dbb..04a3bc0d9b4d 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountBitmapAggregationFunction.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountBitmapAggregationFunction.java @@ -34,6 +34,7 @@ import org.apache.pinot.segment.spi.AggregationFunctionType; import org.apache.pinot.segment.spi.index.reader.Dictionary; import org.apache.pinot.spi.data.FieldSpec.DataType; +import org.apache.pinot.spi.utils.UuidUtils; import org.roaringbitmap.PeekableIntIterator; import org.roaringbitmap.RoaringBitmap; @@ -71,8 +72,22 @@ public void aggregate(int length, AggregationResultHolder aggregationResultHolde Map blockValSetMap) { BlockValSet blockValSet = blockValSetMap.get(_expression); + DataType dataType = blockValSet.getValueType(); + DataType storedType = dataType.getStoredType(); + + // UUID values are logical scalars (stored as 16-byte BYTES) — not serialized RoaringBitmap state. Add the + // hashCode of the canonical UUID string so DISTINCTCOUNTBITMAP(uuidCol) matches + // DISTINCTCOUNTBITMAP(CAST(uuidCol AS STRING)). + if (dataType == DataType.UUID) { + byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); + RoaringBitmap bitmap = getValueBitmap(aggregationResultHolder); + for (int i = 0; i < length; i++) { + bitmap.add(UuidUtils.toString(uuidBytesValues[i]).hashCode()); + } + return; + } + // Treat BYTES value as serialized RoaringBitmap - DataType storedType = blockValSet.getValueType().getStoredType(); if (storedType == DataType.BYTES) { byte[][] bytesValues = blockValSet.getBytesValuesSV(); RoaringBitmap valueBitmap = aggregationResultHolder.getResult(); @@ -209,8 +224,20 @@ public void aggregateGroupBySV(int length, int[] groupKeyArray, GroupByResultHol Map blockValSetMap) { BlockValSet blockValSet = blockValSetMap.get(_expression); + DataType dataType = blockValSet.getValueType(); + DataType storedType = dataType.getStoredType(); + + // UUID columns: add hashCode of canonical UUID string (see aggregate() for rationale). + if (dataType == DataType.UUID) { + byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); + for (int i = 0; i < length; i++) { + getValueBitmap(groupByResultHolder, groupKeyArray[i]) + .add(UuidUtils.toString(uuidBytesValues[i]).hashCode()); + } + return; + } + // Treat BYTES value as serialized RoaringBitmap - DataType storedType = blockValSet.getValueType().getStoredType(); if (storedType == DataType.BYTES) { byte[][] bytesValues = blockValSet.getBytesValuesSV(); for (int i = 0; i < length; i++) { @@ -350,8 +377,22 @@ public void aggregateGroupByMV(int length, int[][] groupKeysArray, GroupByResult Map blockValSetMap) { BlockValSet blockValSet = blockValSetMap.get(_expression); + DataType dataType = blockValSet.getValueType(); + DataType storedType = dataType.getStoredType(); + + // UUID columns: add hashCode of canonical UUID string (see aggregate() for rationale). + if (dataType == DataType.UUID) { + byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); + for (int i = 0; i < length; i++) { + int hash = UuidUtils.toString(uuidBytesValues[i]).hashCode(); + for (int groupKey : groupKeysArray[i]) { + getValueBitmap(groupByResultHolder, groupKey).add(hash); + } + } + return; + } + // Treat BYTES value as serialized RoaringBitmap - DataType storedType = blockValSet.getValueType().getStoredType(); if (storedType == DataType.BYTES) { byte[][] bytesValues = blockValSet.getBytesValuesSV(); for (int i = 0; i < length; i++) { diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountCPCSketchAggregationFunction.java b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountCPCSketchAggregationFunction.java index ec273c066ab1..7111f211e509 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountCPCSketchAggregationFunction.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountCPCSketchAggregationFunction.java @@ -41,6 +41,7 @@ import org.apache.pinot.spi.data.FieldSpec; import org.apache.pinot.spi.data.FieldSpec.DataType; import org.apache.pinot.spi.utils.CommonConstants; +import org.apache.pinot.spi.utils.UuidUtils; import org.roaringbitmap.PeekableIntIterator; import org.roaringbitmap.RoaringBitmap; @@ -135,8 +136,24 @@ public void aggregate(int length, AggregationResultHolder aggregationResultHolde Map blockValSetMap) { BlockValSet blockValSet = blockValSetMap.get(_expression); + FieldSpec.DataType dataType = blockValSet.getValueType(); + FieldSpec.DataType storedType = dataType.getStoredType(); + + // UUID values are logical scalars (stored as 16-byte BYTES) — not serialized CPC Sketch state. Update + // the sketch with the canonical UUID string so DISTINCTCOUNTCPC(uuidCol) matches + // DISTINCTCOUNTCPC(CAST(uuidCol AS STRING)). + if (dataType == DataType.UUID) { + byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); + // Leave the updated CpcSketch in the holder; extractAggregationResult converts it to an accumulator. + // Calling getAccumulator here would read the holder slot already occupied by the sketch and fail. + CpcSketch cpcSketch = getCpcSketch(aggregationResultHolder); + for (int i = 0; i < length; i++) { + cpcSketch.update(UuidUtils.toString(uuidBytesValues[i])); + } + return; + } + // Treat BYTES value as serialized CPC Sketch - FieldSpec.DataType storedType = blockValSet.getValueType().getStoredType(); if (storedType == DataType.BYTES) { byte[][] bytesValues = blockValSet.getBytesValuesSV(); try { @@ -197,8 +214,9 @@ public void aggregate(int length, AggregationResultHolder aggregationResultHolde default: throw new IllegalStateException("Illegal data type for DISTINCT_COUNT_CPC aggregation function: " + storedType); } - CpcSketchAccumulator cpcSketchAccumulator = getAccumulator(aggregationResultHolder); - cpcSketchAccumulator.apply(cpcSketch); + // The updated CpcSketch already lives in the holder (getCpcSketch stored it); extractAggregationResult + // converts it to a CpcSketchAccumulator. Reading the holder as an accumulator here would + // ClassCastException — the holder slot contains the sketch, not an accumulator. } @Override @@ -206,8 +224,19 @@ public void aggregateGroupBySV(int length, int[] groupKeyArray, GroupByResultHol Map blockValSetMap) { BlockValSet blockValSet = blockValSetMap.get(_expression); + DataType dataType = blockValSet.getValueType(); + DataType storedType = dataType.getStoredType(); + + // UUID columns: update with canonical UUID strings converted from raw bytes (see aggregate() for rationale). + if (dataType == DataType.UUID) { + byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); + for (int i = 0; i < length; i++) { + getCpcSketch(groupByResultHolder, groupKeyArray[i]).update(UuidUtils.toString(uuidBytesValues[i])); + } + return; + } + // Treat BYTES value as serialized CPC Sketch - DataType storedType = blockValSet.getValueType().getStoredType(); if (storedType == FieldSpec.DataType.BYTES) { byte[][] bytesValues = blockValSet.getBytesValuesSV(); try { @@ -277,10 +306,22 @@ public void aggregateGroupByMV(int length, int[][] groupKeysArray, GroupByResult Map blockValSetMap) { BlockValSet blockValSet = blockValSetMap.get(_expression); - // Treat BYTES value as serialized CPC Sketch - DataType storedType = blockValSet.getValueType().getStoredType(); + DataType dataType = blockValSet.getValueType(); + DataType storedType = dataType.getStoredType(); boolean singleValue = blockValSet.isSingleValue(); + // UUID columns: update with canonical UUID strings converted from raw bytes (see aggregate() for rationale). + if (dataType == DataType.UUID && singleValue) { + byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); + for (int i = 0; i < length; i++) { + String canonical = UuidUtils.toString(uuidBytesValues[i]); + for (int groupKey : groupKeysArray[i]) { + getCpcSketch(groupByResultHolder, groupKey).update(canonical); + } + } + return; + } + if (singleValue && storedType == DataType.BYTES) { byte[][] bytesValues = blockValSet.getBytesValuesSV(); try { diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLAggregationFunction.java b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLAggregationFunction.java index 07bfb6bd41af..8a74f972f7f3 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLAggregationFunction.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLAggregationFunction.java @@ -38,6 +38,7 @@ import org.apache.pinot.segment.spi.index.reader.Dictionary; import org.apache.pinot.spi.data.FieldSpec.DataType; import org.apache.pinot.spi.utils.CommonConstants; +import org.apache.pinot.spi.utils.UuidUtils; import org.roaringbitmap.RoaringBitmap; @@ -81,8 +82,24 @@ public void aggregate(int length, AggregationResultHolder aggregationResultHolde Map blockValSetMap) { BlockValSet blockValSet = blockValSetMap.get(_expression); + DataType dataType = blockValSet.getValueType(); + DataType storedType = dataType.getStoredType(); + + // UUID columns are stored as 16-byte BYTES, but a UUID value is a logical scalar — not a serialized + // HyperLogLog. Offer the canonical UUID string so the result matches DISTINCTCOUNTHLL on a STRING column + // holding the same logical UUIDs. NOTE: fetch raw bytes and convert explicitly — for identifier expressions + // the BlockValSet is a ProjectionBlockValSet whose getStringValuesSV() renders stored BYTES as bare hex, + // not the canonical RFC-4122 form. + if (dataType == DataType.UUID) { + byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); + HyperLogLog hyperLogLog = getHyperLogLog(aggregationResultHolder); + for (int i = 0; i < length; i++) { + hyperLogLog.offer(UuidUtils.toString(uuidBytesValues[i])); + } + return; + } + // Treat BYTES value as serialized HyperLogLog - DataType storedType = blockValSet.getValueType().getStoredType(); if (storedType == DataType.BYTES) { byte[][] bytesValues = blockValSet.getBytesValuesSV(); try { @@ -232,8 +249,19 @@ public void aggregateGroupBySV(int length, int[] groupKeyArray, GroupByResultHol Map blockValSetMap) { BlockValSet blockValSet = blockValSetMap.get(_expression); + DataType dataType = blockValSet.getValueType(); + DataType storedType = dataType.getStoredType(); + + // UUID columns: offer canonical UUID strings converted from raw bytes (see aggregate() for rationale). + if (dataType == DataType.UUID) { + byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); + for (int i = 0; i < length; i++) { + getHyperLogLog(groupByResultHolder, groupKeyArray[i]).offer(UuidUtils.toString(uuidBytesValues[i])); + } + return; + } + // Treat BYTES value as serialized HyperLogLog - DataType storedType = blockValSet.getValueType().getStoredType(); if (storedType == DataType.BYTES) { byte[][] bytesValues = blockValSet.getBytesValuesSV(); try { @@ -381,8 +409,22 @@ public void aggregateGroupByMV(int length, int[][] groupKeysArray, GroupByResult Map blockValSetMap) { BlockValSet blockValSet = blockValSetMap.get(_expression); + DataType dataType = blockValSet.getValueType(); + DataType storedType = dataType.getStoredType(); + + // UUID columns: offer canonical UUID strings converted from raw bytes (see aggregate() for rationale). + if (dataType == DataType.UUID) { + byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); + for (int i = 0; i < length; i++) { + String canonical = UuidUtils.toString(uuidBytesValues[i]); + for (int groupKey : groupKeysArray[i]) { + getHyperLogLog(groupByResultHolder, groupKey).offer(canonical); + } + } + return; + } + // Treat BYTES value as serialized HyperLogLog - DataType storedType = blockValSet.getValueType().getStoredType(); if (storedType == DataType.BYTES) { byte[][] bytesValues = blockValSet.getBytesValuesSV(); try { diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLPlusAggregationFunction.java b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLPlusAggregationFunction.java index fd337f433a34..1c2762670c61 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLPlusAggregationFunction.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLPlusAggregationFunction.java @@ -37,6 +37,7 @@ import org.apache.pinot.segment.spi.index.reader.Dictionary; import org.apache.pinot.spi.data.FieldSpec.DataType; import org.apache.pinot.spi.utils.CommonConstants; +import org.apache.pinot.spi.utils.UuidUtils; import org.roaringbitmap.PeekableIntIterator; import org.roaringbitmap.RoaringBitmap; @@ -93,8 +94,21 @@ public void aggregate(int length, AggregationResultHolder aggregationResultHolde Map blockValSetMap) { BlockValSet blockValSet = blockValSetMap.get(_expression); + DataType dataType = blockValSet.getValueType(); + DataType storedType = dataType.getStoredType(); + + // UUID values are logical scalars (stored as 16-byte BYTES) — not serialized HyperLogLogPlus state. Offer the + // canonical UUID string so DISTINCTCOUNTHLLPLUS(uuidCol) matches DISTINCTCOUNTHLLPLUS(CAST(uuidCol AS STRING)). + if (dataType == DataType.UUID) { + byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); + HyperLogLogPlus hyperLogLogPlus = getHyperLogLogPlus(aggregationResultHolder); + for (int i = 0; i < length; i++) { + hyperLogLogPlus.offer(UuidUtils.toString(uuidBytesValues[i])); + } + return; + } + // Treat BYTES value as serialized HyperLogLogPlus - DataType storedType = blockValSet.getValueType().getStoredType(); if (storedType == DataType.BYTES) { byte[][] bytesValues = blockValSet.getBytesValuesSV(); try { @@ -237,8 +251,19 @@ public void aggregateGroupBySV(int length, int[] groupKeyArray, GroupByResultHol Map blockValSetMap) { BlockValSet blockValSet = blockValSetMap.get(_expression); + DataType dataType = blockValSet.getValueType(); + DataType storedType = dataType.getStoredType(); + + // UUID columns: offer canonical UUID strings converted from raw bytes (see aggregate() for rationale). + if (dataType == DataType.UUID) { + byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); + for (int i = 0; i < length; i++) { + getHyperLogLogPlus(groupByResultHolder, groupKeyArray[i]).offer(UuidUtils.toString(uuidBytesValues[i])); + } + return; + } + // Treat BYTES value as serialized HyperLogLogPlus - DataType storedType = blockValSet.getValueType().getStoredType(); if (storedType == DataType.BYTES) { byte[][] bytesValues = blockValSet.getBytesValuesSV(); try { @@ -385,8 +410,22 @@ public void aggregateGroupByMV(int length, int[][] groupKeysArray, GroupByResult Map blockValSetMap) { BlockValSet blockValSet = blockValSetMap.get(_expression); + DataType dataType = blockValSet.getValueType(); + DataType storedType = dataType.getStoredType(); + + // UUID columns: offer canonical UUID strings converted from raw bytes (see aggregate() for rationale). + if (dataType == DataType.UUID) { + byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); + for (int i = 0; i < length; i++) { + String canonical = UuidUtils.toString(uuidBytesValues[i]); + for (int groupKey : groupKeysArray[i]) { + getHyperLogLogPlus(groupByResultHolder, groupKey).offer(canonical); + } + } + return; + } + // Treat BYTES value as serialized HyperLogLogPlus - DataType storedType = blockValSet.getValueType().getStoredType(); if (storedType == DataType.BYTES) { byte[][] bytesValues = blockValSet.getBytesValuesSV(); try { diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountThetaSketchAggregationFunction.java b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountThetaSketchAggregationFunction.java index a2859fe471ec..67e9c2996ef9 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountThetaSketchAggregationFunction.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountThetaSketchAggregationFunction.java @@ -56,6 +56,7 @@ import org.apache.pinot.segment.spi.Constants; import org.apache.pinot.spi.data.FieldSpec.DataType; import org.apache.pinot.spi.utils.CommonConstants; +import org.apache.pinot.spi.utils.UuidUtils; import org.apache.pinot.sql.parsers.CalciteSqlParser; @@ -189,7 +190,7 @@ public void aggregate(int length, AggregationResultHolder aggregationResultHolde boolean[] singleValues = new boolean[numExpressions]; DataType[] valueTypes = new DataType[numExpressions]; Object[] valueArrays = new Object[numExpressions]; - extractValues(blockValSetMap, singleValues, valueTypes, valueArrays); + extractValues(length, blockValSetMap, singleValues, valueTypes, valueArrays); int numFilters = _filterEvaluators.size(); // Main expression is always index 0 @@ -438,7 +439,7 @@ public void aggregateGroupBySV(int length, int[] groupKeyArray, GroupByResultHol boolean[] singleValues = new boolean[numExpressions]; DataType[] valueTypes = new DataType[numExpressions]; Object[] valueArrays = new Object[numExpressions]; - extractValues(blockValSetMap, singleValues, valueTypes, valueArrays); + extractValues(length, blockValSetMap, singleValues, valueTypes, valueArrays); int numFilters = _filterEvaluators.size(); // Main expression is always index 0 @@ -662,7 +663,7 @@ public void aggregateGroupByMV(int length, int[][] groupKeysArray, GroupByResult boolean[] singleValues = new boolean[numExpressions]; DataType[] valueTypes = new DataType[numExpressions]; Object[] valueArrays = new Object[numExpressions]; - extractValues(blockValSetMap, singleValues, valueTypes, valueArrays); + extractValues(length, blockValSetMap, singleValues, valueTypes, valueArrays); int numFilters = _filterEvaluators.size(); // Main expression is always index 0 @@ -1226,14 +1227,45 @@ private static int extractSketchId(String identifier) { } /// Extracts values from the BlockValSet map. - private void extractValues(Map blockValSetMap, boolean[] singleValues, + private void extractValues(int length, Map blockValSetMap, boolean[] singleValues, DataType[] valueTypes, Object[] valueArrays) { int numExpressions = _inputExpressions.size(); for (int i = 0; i < numExpressions; i++) { BlockValSet blockValSet = blockValSetMap.get(_inputExpressions.get(i)); boolean singleValue = blockValSet.isSingleValue(); - DataType storedType = blockValSet.getValueType().getStoredType(); + DataType dataType = blockValSet.getValueType(); + DataType storedType = dataType.getStoredType(); singleValues[i] = singleValue; + // UUID columns are stored as 16-byte BYTES but a UUID value is a logical scalar, not a pre-serialized + // theta sketch. Surface UUID as STRING (canonical UUID form) so the downstream update-sketch path + // matches DISTINCTCOUNTTHETASKETCH(CAST(uuidCol AS STRING)). Without this branch, the function would + // take the serialized-sketch path below and Sketch.wrap would fail on raw 16-byte UUID content. + // NOTE: fetch raw bytes and convert explicitly — for identifier expressions the BlockValSet is a + // ProjectionBlockValSet whose getStringValuesSV() renders stored BYTES as bare hex, not canonical form. + if (dataType == DataType.UUID) { + valueTypes[i] = DataType.STRING; + if (singleValue) { + byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); + String[] canonicalValues = new String[length]; + for (int j = 0; j < length; j++) { + canonicalValues[j] = UuidUtils.toString(uuidBytesValues[j]); + } + valueArrays[i] = canonicalValues; + } else { + byte[][][] uuidBytesValuesMV = blockValSet.getBytesValuesMV(); + String[][] canonicalValuesMV = new String[length][]; + for (int j = 0; j < length; j++) { + byte[][] row = uuidBytesValuesMV[j]; + String[] canonicalRow = new String[row.length]; + for (int k = 0; k < row.length; k++) { + canonicalRow[k] = UuidUtils.toString(row[k]); + } + canonicalValuesMV[j] = canonicalRow; + } + valueArrays[i] = canonicalValuesMV; + } + continue; + } valueTypes[i] = storedType; if (singleValue) { switch (storedType) { diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountULLAggregationFunction.java b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountULLAggregationFunction.java index 5309a9713af3..cad535aa2aea 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountULLAggregationFunction.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountULLAggregationFunction.java @@ -38,6 +38,7 @@ import org.apache.pinot.segment.spi.index.reader.Dictionary; import org.apache.pinot.spi.data.FieldSpec.DataType; import org.apache.pinot.spi.utils.CommonConstants; +import org.apache.pinot.spi.utils.UuidUtils; import org.roaringbitmap.PeekableIntIterator; import org.roaringbitmap.RoaringBitmap; @@ -82,8 +83,21 @@ public void aggregate(int length, AggregationResultHolder aggregationResultHolde Map blockValSetMap) { BlockValSet blockValSet = blockValSetMap.get(_expression); - // Treat BYTES value as serialized HyperLogLogPlus - DataType storedType = blockValSet.getValueType().getStoredType(); + DataType dataType = blockValSet.getValueType(); + DataType storedType = dataType.getStoredType(); + + // UUID values are logical scalars (stored as 16-byte BYTES) — not serialized UltraLogLog state. Hash the + // canonical UUID string so DISTINCTCOUNTULL(uuidCol) matches DISTINCTCOUNTULL(CAST(uuidCol AS STRING)). + if (dataType == DataType.UUID) { + byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); + UltraLogLog ull = getULL(aggregationResultHolder); + for (int i = 0; i < length; i++) { + UltraLogLogUtils.hashObject(UuidUtils.toString(uuidBytesValues[i])).ifPresent(ull::add); + } + return; + } + + // Treat BYTES value as serialized UltraLogLog if (storedType == DataType.BYTES) { byte[][] bytesValues = blockValSet.getBytesValuesSV(); try { @@ -155,8 +169,20 @@ public void aggregateGroupBySV(int length, int[] groupKeyArray, GroupByResultHol Map blockValSetMap) { BlockValSet blockValSet = blockValSetMap.get(_expression); + DataType dataType = blockValSet.getValueType(); + DataType storedType = dataType.getStoredType(); + + // UUID columns: hash canonical UUID strings converted from raw bytes (see aggregate() for rationale). + if (dataType == DataType.UUID) { + byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); + for (int i = 0; i < length; i++) { + UltraLogLog ull = getULL(groupByResultHolder, groupKeyArray[i]); + UltraLogLogUtils.hashObject(UuidUtils.toString(uuidBytesValues[i])).ifPresent(ull::add); + } + return; + } + // Treat BYTES value as serialized UltraLogLogs - DataType storedType = blockValSet.getValueType().getStoredType(); if (storedType == DataType.BYTES) { byte[][] bytesValues = blockValSet.getBytesValuesSV(); try { @@ -234,8 +260,23 @@ public void aggregateGroupByMV(int length, int[][] groupKeysArray, GroupByResult Map blockValSetMap) { BlockValSet blockValSet = blockValSetMap.get(_expression); - // Treat BYTES value as serialized HyperLogLogPlus - DataType storedType = blockValSet.getValueType().getStoredType(); + DataType dataType = blockValSet.getValueType(); + DataType storedType = dataType.getStoredType(); + + // UUID columns: hash canonical UUID strings converted from raw bytes (see aggregate() for rationale). + if (dataType == DataType.UUID) { + byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); + for (int i = 0; i < length; i++) { + String canonical = UuidUtils.toString(uuidBytesValues[i]); + for (int groupKey : groupKeysArray[i]) { + UltraLogLog ull = getULL(groupByResultHolder, groupKey); + UltraLogLogUtils.hashObject(canonical).ifPresent(ull::add); + } + } + return; + } + + // Treat BYTES value as serialized UltraLogLogs if (storedType == DataType.BYTES) { byte[][] bytesValues = blockValSet.getBytesValuesSV(); try { diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/IntegerTupleSketchAggregationFunction.java b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/IntegerTupleSketchAggregationFunction.java index 83ae908cfa80..b42979201962 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/IntegerTupleSketchAggregationFunction.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/IntegerTupleSketchAggregationFunction.java @@ -151,8 +151,15 @@ public void aggregate(int length, AggregationResultHolder aggregationResultHolde Map blockValSetMap) { BlockValSet blockValSet = blockValSetMap.get(_expression); + FieldSpec.DataType dataType = blockValSet.getValueType(); + FieldSpec.DataType storedType = dataType.getStoredType(); + // UUID columns are stored as BYTES but contain raw 16-byte UUID values, not serialized tuple sketches. + // Surface a clear error rather than letting the deserialize step fail with a confusing sketch-format message. + if (dataType == FieldSpec.DataType.UUID) { + throw new IllegalStateException(getType() + " does not accept raw UUID values; pre-serialize the column as " + + "Integer Tuple Sketches first"); + } // Treat BYTES value as serialized Integer Tuple Sketch - FieldSpec.DataType storedType = blockValSet.getValueType().getStoredType(); if (storedType == FieldSpec.DataType.BYTES) { byte[][] bytesValues = blockValSet.getBytesValuesSV(); try { @@ -175,8 +182,12 @@ public void aggregateGroupBySV(int length, int[] groupKeyArray, GroupByResultHol BlockValSet blockValSet = blockValSetMap.get(_expression); - // Treat BYTES value as serialized Integer Tuple Sketch - FieldSpec.DataType storedType = blockValSet.getValueType().getStoredType(); + FieldSpec.DataType dataType = blockValSet.getValueType(); + FieldSpec.DataType storedType = dataType.getStoredType(); + if (dataType == FieldSpec.DataType.UUID) { + throw new IllegalStateException(getType() + " does not accept raw UUID values; pre-serialize the column as " + + "Integer Tuple Sketches first"); + } if (storedType == FieldSpec.DataType.BYTES) { byte[][] bytesValues = blockValSet.getBytesValuesSV(); @@ -201,8 +212,12 @@ public void aggregateGroupByMV(int length, int[][] groupKeysArray, GroupByResult Map blockValSetMap) { BlockValSet blockValSet = blockValSetMap.get(_expression); - // Treat BYTES value as serialized Integer Tuple Sketch - FieldSpec.DataType storedType = blockValSet.getValueType().getStoredType(); + FieldSpec.DataType dataType = blockValSet.getValueType(); + FieldSpec.DataType storedType = dataType.getStoredType(); + if (dataType == FieldSpec.DataType.UUID) { + throw new IllegalStateException(getType() + " does not accept raw UUID values; pre-serialize the column as " + + "Integer Tuple Sketches first"); + } boolean singleValue = blockValSet.isSingleValue(); if (singleValue && storedType == FieldSpec.DataType.BYTES) { diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionaryMultiColumnGroupKeyGenerator.java b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionaryMultiColumnGroupKeyGenerator.java index a10de9f47a58..f6d3784ebf96 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionaryMultiColumnGroupKeyGenerator.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionaryMultiColumnGroupKeyGenerator.java @@ -36,6 +36,7 @@ import org.apache.pinot.spi.data.FieldSpec.DataType; import org.apache.pinot.spi.utils.ByteArray; import org.apache.pinot.spi.utils.FixedIntArray; +import org.apache.pinot.spi.utils.UuidKey; import org.roaringbitmap.RoaringBitmap; @@ -51,7 +52,11 @@ public class NoDictionaryMultiColumnGroupKeyGenerator implements GroupKeyGenerat private final ExpressionContext[] _groupByExpressions; private final int _numGroupByExpressions; - private final DataType[] _storedTypes; + /// Per-column group-key dispatch type: stored type of each column, except UUID is preserved as + /// [DataType#UUID] so the on-the-fly dictionary keys on [org.apache.pinot.spi.utils.UuidKey] + /// instead of [org.apache.pinot.spi.utils.ByteArray]. For every other logical type this equals + /// `logicalType.getStoredType()`. + private final DataType[] _dataTypes; private final Dictionary[] _dictionaries; private final ValueToIdMap[] _onTheFlyDictionaries; private final Object2IntOpenHashMap _groupKeyMap; @@ -65,7 +70,7 @@ public NoDictionaryMultiColumnGroupKeyGenerator(BaseProjectOperator projectOp Map groupByExpressionSizesFromPredicates) { _groupByExpressions = groupByExpressions; _numGroupByExpressions = groupByExpressions.length; - _storedTypes = new DataType[_numGroupByExpressions]; + _dataTypes = new DataType[_numGroupByExpressions]; _dictionaries = new Dictionary[_numGroupByExpressions]; _onTheFlyDictionaries = new ValueToIdMap[_numGroupByExpressions]; _isSingleValueExpressions = new boolean[_numGroupByExpressions]; @@ -75,7 +80,9 @@ public NoDictionaryMultiColumnGroupKeyGenerator(BaseProjectOperator projectOp for (int i = 0; i < _numGroupByExpressions; i++) { ExpressionContext groupByExpression = groupByExpressions[i]; ColumnContext columnContext = projectOperator.getResultColumnContext(groupByExpression); - _storedTypes[i] = columnContext.getDataType().getStoredType(); + DataType logicalType = columnContext.getDataType(); + // Normalize to stored type, but preserve UUID so it uses UuidKey instead of ByteArray. + _dataTypes[i] = logicalType == DataType.UUID ? DataType.UUID : logicalType.getStoredType(); // Take the dict-id path only when the forward index is dict-encoded. A column with EncodingType.RAW + // dictionaryIndex exposes a Dictionary but BlockValSet#getDictionaryIdsSV throws on its RAW forward // index — fall back to an on-the-fly dictionary on raw values for that case. @@ -84,7 +91,7 @@ public NoDictionaryMultiColumnGroupKeyGenerator(BaseProjectOperator projectOp if (dictionary != null) { _dictionaries[i] = dictionary; } else { - _onTheFlyDictionaries[i] = ValueToIdMapFactory.get(_storedTypes[i]); + _onTheFlyDictionaries[i] = ValueToIdMapFactory.get(_dataTypes[i]); } if (canOptimizeGroupByUpperBound) { Integer size = groupByExpressionSizesFromPredicates.get(groupByExpression); @@ -121,7 +128,7 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[] groupKeys) { if (_dictionaries[i] != null) { values[i] = blockValSet.getDictionaryIdsSV(); } else { - switch (_storedTypes[i]) { + switch (_dataTypes[i]) { case INT: values[i] = blockValSet.getIntValuesSV(); break; @@ -141,10 +148,11 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[] groupKeys) { values[i] = blockValSet.getStringValuesSV(); break; case BYTES: + case UUID: values[i] = blockValSet.getBytesValuesSV(); break; default: - throw new IllegalArgumentException("Illegal data type for no-dictionary key generator: " + _storedTypes[i]); + throw new IllegalArgumentException("Illegal data type for no-dictionary key generator: " + _dataTypes[i]); } } } @@ -176,7 +184,7 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[] groupKeys) { } else if (columnValues instanceof double[]) { keyValue = onTheFlyDictionary.put(((double[]) columnValues)[row]); } else if (columnValues instanceof byte[][]) { - keyValue = onTheFlyDictionary.put(new ByteArray(((byte[][]) columnValues)[row])); + keyValue = putBytesValue(onTheFlyDictionary, (byte[][]) columnValues, row, _dataTypes[col]); } else { keyValue = onTheFlyDictionary.put(((Object[]) columnValues)[row]); } @@ -200,7 +208,7 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[] groupKeys) { } else if (columnValues instanceof double[]) { keyValue = onTheFlyDictionary.getId(((double[]) columnValues)[row]); } else if (columnValues instanceof byte[][]) { - keyValue = onTheFlyDictionary.getId(new ByteArray(((byte[][]) columnValues)[row])); + keyValue = getBytesValueId(onTheFlyDictionary, (byte[][]) columnValues, row, _dataTypes[col]); } else { keyValue = onTheFlyDictionary.getId(((Object[]) columnValues)[row]); } @@ -242,7 +250,7 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[] groupKeys) { } else if (columnValues instanceof double[]) { keyValue = onTheFlyDictionary.put(((double[]) columnValues)[row]); } else if (columnValues instanceof byte[][]) { - keyValue = onTheFlyDictionary.put(new ByteArray(((byte[][]) columnValues)[row])); + keyValue = putBytesValue(onTheFlyDictionary, (byte[][]) columnValues, row, _dataTypes[col]); } else { keyValue = onTheFlyDictionary.put(((Object[]) columnValues)[row]); } @@ -263,7 +271,7 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[] groupKeys) { } else if (columnValues instanceof double[]) { keyValue = onTheFlyDictionary.getId(((double[]) columnValues)[row]); } else if (columnValues instanceof byte[][]) { - keyValue = onTheFlyDictionary.getId(new ByteArray(((byte[][]) columnValues)[row])); + keyValue = getBytesValueId(onTheFlyDictionary, (byte[][]) columnValues, row, _dataTypes[col]); } else { keyValue = onTheFlyDictionary.getId(((Object[]) columnValues)[row]); } @@ -310,7 +318,7 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[][] groupKeys) { } else { ValueToIdMap onTheFlyDictionary = _onTheFlyDictionaries[i]; if (_isSingleValueExpressions[i]) { - switch (_storedTypes[i]) { + switch (_dataTypes[i]) { case INT: int[] intValues = blockValSet.getIntValuesSV(); for (int j = 0; j < numDocs; j++) { @@ -341,6 +349,12 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[][] groupKeys) { keys[j][i] = new int[]{onTheFlyDictionary.put(stringValues[j])}; } break; + case UUID: + byte[][] uuidValues = blockValSet.getBytesValuesSV(); + for (int j = 0; j < numDocs; j++) { + keys[j][i] = new int[]{onTheFlyDictionary.put(UuidKey.fromBytes(uuidValues[j]))}; + } + break; case BYTES: byte[][] bytesValues = blockValSet.getBytesValuesSV(); for (int j = 0; j < numDocs; j++) { @@ -349,10 +363,10 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[][] groupKeys) { break; default: throw new IllegalArgumentException( - "Illegal data type for no-dictionary key generator: " + _storedTypes[i]); + "Illegal data type for no-dictionary key generator: " + _dataTypes[i]); } } else { - switch (_storedTypes[i]) { + switch (_dataTypes[i]) { case INT: int[][] intValues = blockValSet.getIntValuesMV(); for (int j = 0; j < numDocs; j++) { @@ -408,9 +422,20 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[][] groupKeys) { keys[j][i] = mvKeys; } break; + case UUID: + byte[][][] uuidValues = blockValSet.getBytesValuesMV(); + for (int j = 0; j < numDocs; j++) { + int mvSize = uuidValues[j].length; + int[] mvKeys = new int[mvSize]; + for (int k = 0; k < mvSize; k++) { + mvKeys[k] = onTheFlyDictionary.put(UuidKey.fromBytes(uuidValues[j][k])); + } + keys[j][i] = mvKeys; + } + break; default: throw new IllegalArgumentException( - "Illegal data type for no-dictionary key generator: " + _storedTypes[i]); + "Illegal data type for no-dictionary key generator: " + _dataTypes[i]); } } } @@ -516,4 +541,19 @@ private Object[] buildKeysFromIds(FixedIntArray keyList) { } return keys; } + + private static int putBytesValue(ValueToIdMap onTheFlyDictionary, byte[][] columnValues, int row, DataType dataType) { + if (dataType == DataType.UUID) { + return onTheFlyDictionary.put(UuidKey.fromBytes(columnValues[row])); + } + return onTheFlyDictionary.put(new ByteArray(columnValues[row])); + } + + private static int getBytesValueId(ValueToIdMap onTheFlyDictionary, byte[][] columnValues, int row, + DataType dataType) { + if (dataType == DataType.UUID) { + return onTheFlyDictionary.getId(UuidKey.fromBytes(columnValues[row])); + } + return onTheFlyDictionary.getId(new ByteArray(columnValues[row])); + } } diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionarySingleColumnGroupKeyGenerator.java b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionarySingleColumnGroupKeyGenerator.java index d6b0bfcce935..a0cc335c3d93 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionarySingleColumnGroupKeyGenerator.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionarySingleColumnGroupKeyGenerator.java @@ -41,6 +41,7 @@ import org.apache.pinot.core.operator.blocks.ValueBlock; import org.apache.pinot.spi.data.FieldSpec.DataType; import org.apache.pinot.spi.utils.ByteArray; +import org.apache.pinot.spi.utils.UuidKey; import org.roaringbitmap.RoaringBitmap; @@ -49,7 +50,11 @@ @SuppressWarnings({"rawtypes", "unchecked"}) public class NoDictionarySingleColumnGroupKeyGenerator implements GroupKeyGenerator { private final ExpressionContext _groupByExpression; - private final DataType _storedType; + /// Group-key dispatch type: stored type of the column, except UUID is preserved as [DataType#UUID] so the + /// group-key map keys on [org.apache.pinot.spi.utils.UuidKey] (two primitive longs) instead of + /// [org.apache.pinot.spi.utils.ByteArray]. For every other logical type this equals + /// `logicalType.getStoredType()`. + private final DataType _dataType; private final Map _groupKeyMap; private final int _globalGroupIdUpperBound; // TODO(nhejazi): Most of the logic between _nullHandlingEnabled=true/false is not sharable, so consider making a @@ -66,8 +71,10 @@ public NoDictionarySingleColumnGroupKeyGenerator(BaseProjectOperator projectO @Nullable Map groupByExpressionSizesFromPredicates) { _groupByExpression = groupByExpression; ColumnContext columnContext = projectOperator.getResultColumnContext(groupByExpression); - _storedType = columnContext.getDataType().getStoredType(); - _groupKeyMap = createGroupKeyMap(_storedType); + DataType logicalType = columnContext.getDataType(); + // Normalize to stored type, but preserve UUID so it uses UuidKey instead of ByteArray + _dataType = logicalType == DataType.UUID ? DataType.UUID : logicalType.getStoredType(); + _groupKeyMap = createGroupKeyMap(_dataType); if (groupByExpressionSizesFromPredicates != null) { Integer size = groupByExpressionSizesFromPredicates.get(groupByExpression); _globalGroupIdUpperBound = size != null ? Math.min(size, numGroupsLimit) : numGroupsLimit; @@ -95,7 +102,7 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[] groupKeys) { } int numDocs = valueBlock.getNumDocs(); - switch (_storedType) { + switch (_dataType) { case INT: int[] intValues = blockValSet.getIntValuesSV(); for (int i = 0; i < numDocs; i++) { @@ -132,6 +139,12 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[] groupKeys) { groupKeys[i] = getKeyForValue(stringValues[i]); } break; + case UUID: + byte[][] uuidValues = blockValSet.getBytesValuesSV(); + for (int i = 0; i < numDocs; i++) { + groupKeys[i] = getKeyForValue(UuidKey.fromBytes(uuidValues[i])); + } + break; case BYTES: byte[][] bytesValues = blockValSet.getBytesValuesSV(); for (int i = 0; i < numDocs; i++) { @@ -139,7 +152,7 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[] groupKeys) { } break; default: - throw new IllegalArgumentException("Illegal data type for no-dictionary key generator: " + _storedType); + throw new IllegalArgumentException("Illegal data type for no-dictionary key generator: " + _dataType); } } @@ -149,7 +162,7 @@ public void generateKeysForBlockNullHandlingEnabled(ValueBlock valueBlock, int[] BlockValSet blockValSet = valueBlock.getBlockValueSet(_groupByExpression); int numDocs = valueBlock.getNumDocs(); - switch (_storedType) { + switch (_dataType) { case INT: int[] intValues = blockValSet.getIntValuesSV(); if (nullBitmap.getCardinality() < numDocs) { @@ -210,6 +223,16 @@ public void generateKeysForBlockNullHandlingEnabled(ValueBlock valueBlock, int[] Arrays.fill(groupKeys, 0, numDocs, getKeyForValue((String) null)); } break; + case UUID: + byte[][] uuidValues = blockValSet.getBytesValuesSV(); + if (nullBitmap.getCardinality() < numDocs) { + for (int i = 0; i < numDocs; i++) { + groupKeys[i] = getKeyForValue(nullBitmap.contains(i) ? null : UuidKey.fromBytes(uuidValues[i])); + } + } else if (numDocs > 0) { + Arrays.fill(groupKeys, 0, numDocs, getKeyForValue((UuidKey) null)); + } + break; case BYTES: byte[][] bytesValues = blockValSet.getBytesValuesSV(); if (nullBitmap.getCardinality() < numDocs) { @@ -221,7 +244,7 @@ public void generateKeysForBlockNullHandlingEnabled(ValueBlock valueBlock, int[] } break; default: - throw new IllegalArgumentException("Illegal data type for no-dictionary key generator: " + _storedType); + throw new IllegalArgumentException("Illegal data type for no-dictionary key generator: " + _dataType); } } @@ -256,6 +279,10 @@ private Map createGroupKeyMap(DataType keyType) { Object2IntOpenHashMap stringMap = new Object2IntOpenHashMap<>(); stringMap.defaultReturnValue(INVALID_ID); return stringMap; + case UUID: + Object2IntOpenHashMap uuidMap = new Object2IntOpenHashMap<>(); + uuidMap.defaultReturnValue(INVALID_ID); + return uuidMap; case BYTES: Object2IntOpenHashMap bytesMap = new Object2IntOpenHashMap<>(); bytesMap.defaultReturnValue(INVALID_ID); @@ -271,7 +298,7 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[][] groupKeys) { BlockValSet blockValSet = valueBlock.getBlockValueSet(_groupByExpression); if (_isSingleValueExpression) { - switch (_storedType) { + switch (_dataType) { case INT: int[] intValues = blockValSet.getIntValuesSV(); for (int i = 0; i < numDocs; i++) { @@ -302,6 +329,12 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[][] groupKeys) { groupKeys[i] = new int[]{getKeyForValue(stringValues[i])}; } break; + case UUID: + byte[][] uuidValues = blockValSet.getBytesValuesSV(); + for (int i = 0; i < numDocs; i++) { + groupKeys[i] = new int[]{getKeyForValue(UuidKey.fromBytes(uuidValues[i]))}; + } + break; case BYTES: byte[][] byteValues = blockValSet.getBytesValuesSV(); for (int i = 0; i < numDocs; i++) { @@ -309,10 +342,10 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[][] groupKeys) { } break; default: - throw new IllegalArgumentException("Illegal data type for no-dictionary key generator: " + _storedType); + throw new IllegalArgumentException("Illegal data type for no-dictionary key generator: " + _dataType); } } else { - switch (_storedType) { + switch (_dataType) { case INT: int[][] intValues = blockValSet.getIntValuesMV(); for (int i = 0; i < numDocs; i++) { @@ -368,8 +401,19 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[][] groupKeys) { groupKeys[i] = mvKeys; } break; + case UUID: + byte[][][] uuidValues = blockValSet.getBytesValuesMV(); + for (int i = 0; i < numDocs; i++) { + int mvSize = uuidValues[i].length; + int[] mvKeys = new int[mvSize]; + for (int j = 0; j < mvSize; j++) { + mvKeys[j] = getKeyForValue(UuidKey.fromBytes(uuidValues[i][j])); + } + groupKeys[i] = mvKeys; + } + break; default: - throw new IllegalArgumentException("Illegal data type for no-dictionary key generator: " + _storedType); + throw new IllegalArgumentException("Illegal data type for no-dictionary key generator: " + _dataType); } } } @@ -381,7 +425,7 @@ public int getCurrentGroupKeyUpperBound() { @Override public Iterator getGroupKeys() { - switch (_storedType) { + switch (_dataType) { case INT: return new IntGroupKeyIterator((Int2IntOpenHashMap) _groupKeyMap, _groupIdForNullValue); case LONG: @@ -393,7 +437,8 @@ public Iterator getGroupKeys() { case BIG_DECIMAL: case STRING: case BYTES: - return new ObjectGroupKeyIterator((Object2IntOpenHashMap) _groupKeyMap); + case UUID: + return new ObjectGroupKeyIterator((Object2IntOpenHashMap) _groupKeyMap, _dataType); default: throw new IllegalStateException(); } @@ -485,6 +530,16 @@ private int getKeyForValue(ByteArray value) { return groupId; } + private int getKeyForValue(UuidKey value) { + Object2IntMap map = (Object2IntMap) _groupKeyMap; + int groupId = map.getInt(value); + if (groupId == INVALID_ID && _numGroups < _globalGroupIdUpperBound) { + groupId = _numGroups++; + map.put(value, groupId); + } + return groupId; + } + private static class IntGroupKeyIterator implements Iterator { final Iterator _iterator; final GroupKey _groupKey; @@ -633,10 +688,12 @@ public void remove() { private static class ObjectGroupKeyIterator implements Iterator { final ObjectIterator _iterator; final GroupKey _groupKey; + final DataType _dataType; - ObjectGroupKeyIterator(Object2IntOpenHashMap objectMap) { + ObjectGroupKeyIterator(Object2IntOpenHashMap objectMap, DataType dataType) { _iterator = objectMap.object2IntEntrySet().fastIterator(); _groupKey = new GroupKey(); + _dataType = dataType; } @Override @@ -648,7 +705,11 @@ public boolean hasNext() { public GroupKey next() { Object2IntMap.Entry entry = _iterator.next(); _groupKey._groupId = entry.getIntValue(); - _groupKey._keys = new Object[]{entry.getKey()}; + Object key = entry.getKey(); + if (_dataType == DataType.UUID && key != null) { + key = ((UuidKey) key).toByteArray(); + } + _groupKey._keys = new Object[]{key}; return _groupKey; } diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/utils/UuidToIdMap.java b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/utils/UuidToIdMap.java new file mode 100644 index 000000000000..845ad21c37a9 --- /dev/null +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/utils/UuidToIdMap.java @@ -0,0 +1,59 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.core.query.aggregation.groupby.utils; + +import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap; +import java.util.ArrayList; +import org.apache.pinot.spi.utils.ByteArray; +import org.apache.pinot.spi.utils.UuidKey; + + +/// Implementation of [ValueToIdMap] for Pinot's logical UUID type. +public class UuidToIdMap implements ValueToIdMap { + private final Object2IntOpenHashMap _valueToIdMap; + private final ArrayList _idToValueMap; + + public UuidToIdMap() { + _valueToIdMap = new Object2IntOpenHashMap<>(); + _valueToIdMap.defaultReturnValue(INVALID_KEY); + _idToValueMap = new ArrayList<>(); + } + + @Override + public int put(Object value) { + UuidKey uuidKey = UuidKey.fromObject(value); + int id = _valueToIdMap.getInt(uuidKey); + if (id == INVALID_KEY) { + id = _valueToIdMap.size(); + _valueToIdMap.put(uuidKey, id); + _idToValueMap.add(uuidKey.toByteArray()); + } + return id; + } + + @Override + public int getId(Object value) { + return _valueToIdMap.getInt(UuidKey.fromObject(value)); + } + + @Override + public Object get(int id) { + return _idToValueMap.get(id); + } +} diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/utils/ValueToIdMapFactory.java b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/utils/ValueToIdMapFactory.java index 4ce47caa196b..a15f9bf9a263 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/utils/ValueToIdMapFactory.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/utils/ValueToIdMapFactory.java @@ -36,6 +36,8 @@ public static ValueToIdMap get(DataType dataType) { return new FloatToIdMap(); case DOUBLE: return new DoubleToIdMap(); + case UUID: + return new UuidToIdMap(); default: return new ObjectToIdMap(); } diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/distinct/table/BytesDistinctTable.java b/pinot-core/src/main/java/org/apache/pinot/core/query/distinct/table/BytesDistinctTable.java index 386648b64f5b..42649c9bd28a 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/distinct/table/BytesDistinctTable.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/distinct/table/BytesDistinctTable.java @@ -287,12 +287,13 @@ private ResultTable toResultTableWithOrderBy() { rows = new ArrayList<>(numValues); addRows(sortedValues, numValues, rows); } + formatRows(rows); return new ResultTable(_dataSchema, rows); } private static void addRows(ByteArray[] values, int length, List rows) { for (int i = 0; i < length; i++) { - rows.add(new Object[]{values[i].toHexString()}); + rows.add(new Object[]{values[i]}); } } @@ -308,12 +309,23 @@ private ResultTable toResultTableWithoutOrderBy() { rows = new ArrayList<>(numValues); addRows(_valueSet, rows); } + formatRows(rows); return new ResultTable(_dataSchema, rows); } private static void addRows(HashSet values, List rows) { for (ByteArray value : values) { - rows.add(new Object[]{value.toHexString()}); + rows.add(new Object[]{value}); + } + } + + private void formatRows(List rows) { + DataSchema.ColumnDataType columnDataType = _dataSchema.getColumnDataType(0); + for (Object[] row : rows) { + Object value = row[0]; + if (value != null) { + row[0] = columnDataType.convertAndFormat(value); + } } } } diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/reduce/GroupByDataTableReducer.java b/pinot-core/src/main/java/org/apache/pinot/core/query/reduce/GroupByDataTableReducer.java index c1add8ca7082..b0abf23afb08 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/reduce/GroupByDataTableReducer.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/reduce/GroupByDataTableReducer.java @@ -530,6 +530,7 @@ private Object getConvertedKey(DataTable dataTable, ColumnDataType columnDataTyp case JSON: return dataTable.getString(rowId, colId); case BYTES: + case UUID: return dataTable.getBytes(rowId, colId).getBytes(); default: throw new IllegalStateException("Illegal column data type in group key: " + columnDataType); diff --git a/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLAggregationFunctionTest.java b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLAggregationFunctionTest.java index 2520115affa0..71feab4aa979 100644 --- a/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLAggregationFunctionTest.java +++ b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLAggregationFunctionTest.java @@ -20,13 +20,19 @@ import com.clearspring.analytics.stream.cardinality.HyperLogLog; import java.util.BitSet; +import java.util.HashMap; import java.util.List; import java.util.Map; import org.apache.pinot.common.request.Literal; import org.apache.pinot.common.request.context.ExpressionContext; +import org.apache.pinot.common.request.context.RequestContextUtils; +import org.apache.pinot.core.common.BlockValSet; +import org.apache.pinot.core.query.aggregation.AggregationResultHolder; import org.apache.pinot.core.query.aggregation.ObjectAggregationResultHolder; import org.apache.pinot.segment.spi.Constants; import org.apache.pinot.segment.spi.index.reader.Dictionary; +import org.apache.pinot.spi.data.FieldSpec.DataType; +import org.apache.pinot.spi.utils.UuidUtils; import org.testng.Assert; import org.testng.annotations.Test; @@ -55,6 +61,96 @@ public void testCanUseStarTreeDefaultLog2m() { Assert.assertFalse(function.canUseStarTree(Map.of(Constants.HLL_LOG2M_KEY, "16"))); } + /// Regression: UUID columns have storedType=BYTES, but a UUID value is a logical scalar, not a serialized + /// HyperLogLog. The aggregator must route UUID columns through the same content-hash path as STRING (offering + /// canonical UUID strings) instead of trying to deserialize each 16-byte value as an HLL. + @Test + public void testAggregateOnUuidColumnOffersCanonicalStringsAndProducesExactDistinctCount() { + ExpressionContext expression = RequestContextUtils.getExpression("uuidCol"); + DistinctCountHLLAggregationFunction function = new DistinctCountHLLAggregationFunction(List.of(expression)); + + // Three distinct UUIDs across six rows; the same UUID repeats twice on rows 0/3, 1/4, 2/5. + String[] uuidStrings = new String[]{ + "550e8400-e29b-41d4-a716-446655440000", + "12345678-1234-1234-1234-1234567890ab", + "9c5e1f24-0b8e-4c9d-87f1-0aa64a3b9d12", + "550e8400-e29b-41d4-a716-446655440000", + "12345678-1234-1234-1234-1234567890ab", + "9c5e1f24-0b8e-4c9d-87f1-0aa64a3b9d12" + }; + + // Stub the BYTES fetch (raw 16-byte values) — the production path converts bytes to canonical strings + // itself because ProjectionBlockValSet.getStringValuesSV() would render stored BYTES as bare hex. + byte[][] uuidBytes = new byte[uuidStrings.length][]; + for (int i = 0; i < uuidStrings.length; i++) { + uuidBytes[i] = UuidUtils.toBytes(uuidStrings[i]); + } + BlockValSet uuidBlockValSet = mock(BlockValSet.class); + when(uuidBlockValSet.getValueType()).thenReturn(DataType.UUID); + when(uuidBlockValSet.getBytesValuesSV()).thenReturn(uuidBytes); + when(uuidBlockValSet.isSingleValue()).thenReturn(true); + when(uuidBlockValSet.getDictionary()).thenReturn(null); + + Map blockValSetMap = new HashMap<>(); + blockValSetMap.put(expression, uuidBlockValSet); + + AggregationResultHolder resultHolder = function.createAggregationResultHolder(); + function.aggregate(uuidStrings.length, resultHolder, blockValSetMap); + + Object intermediate = function.extractAggregationResult(resultHolder); + Assert.assertTrue(intermediate instanceof HyperLogLog, + "Intermediate result must be a HyperLogLog, not a dictionary bitmap"); + long cardinality = ((HyperLogLog) intermediate).cardinality(); + Assert.assertEquals(cardinality, 3L, + "HLL cardinality must equal the 3 distinct UUIDs; got " + cardinality); + } + + /// Cross-type consistency: DISTINCTCOUNTHLL(uuidCol) must produce the same HLL cardinality as + /// DISTINCTCOUNTHLL(stringRepresentationOfSameUuids). Locks in the design contract that UUID columns are + /// hashed as canonical UUID strings. + @Test + public void testUuidDistinctCountHllMatchesStringDistinctCountHll() { + String[] uuidStrings = new String[]{ + "550e8400-e29b-41d4-a716-446655440000", + "12345678-1234-1234-1234-1234567890ab", + "9c5e1f24-0b8e-4c9d-87f1-0aa64a3b9d12" + }; + + long uuidHllCardinality = computeHllCardinality(uuidStrings, DataType.UUID); + long stringHllCardinality = computeHllCardinality(uuidStrings, DataType.STRING); + + Assert.assertEquals(uuidHllCardinality, stringHllCardinality, + "DISTINCTCOUNTHLL(uuidCol) must match DISTINCTCOUNTHLL(CAST(uuidCol AS STRING))"); + } + + private long computeHllCardinality(String[] values, DataType valueType) { + ExpressionContext expression = RequestContextUtils.getExpression("col"); + DistinctCountHLLAggregationFunction function = new DistinctCountHLLAggregationFunction(List.of(expression)); + + BlockValSet blockValSet = mock(BlockValSet.class); + when(blockValSet.getValueType()).thenReturn(valueType); + if (valueType == DataType.UUID) { + // UUID path fetches raw bytes and converts to canonical form itself (projection string fetch returns hex) + byte[][] uuidBytes = new byte[values.length][]; + for (int i = 0; i < values.length; i++) { + uuidBytes[i] = UuidUtils.toBytes(values[i]); + } + when(blockValSet.getBytesValuesSV()).thenReturn(uuidBytes); + } else { + when(blockValSet.getStringValuesSV()).thenReturn(values); + } + when(blockValSet.isSingleValue()).thenReturn(true); + when(blockValSet.getDictionary()).thenReturn(null); + + Map blockValSetMap = new HashMap<>(); + blockValSetMap.put(expression, blockValSet); + + AggregationResultHolder resultHolder = function.createAggregationResultHolder(); + function.aggregate(values.length, resultHolder, blockValSetMap); + Object intermediate = function.extractAggregationResult(resultHolder); + return ((HyperLogLog) intermediate).cardinality(); + } + @Test public void testCanUseStarTreeCustomLog2m() { DistinctCountHLLAggregationFunction function = new DistinctCountHLLAggregationFunction( diff --git a/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionaryGroupKeyGeneratorTest.java b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionaryGroupKeyGeneratorTest.java index 2ed7bff83f1b..2192848feb3d 100644 --- a/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionaryGroupKeyGeneratorTest.java +++ b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionaryGroupKeyGeneratorTest.java @@ -27,6 +27,7 @@ import java.util.List; import java.util.Random; import java.util.Set; +import java.util.UUID; import org.apache.commons.io.FileUtils; import org.apache.commons.lang3.RandomStringUtils; import org.apache.commons.lang3.StringUtils; @@ -51,6 +52,7 @@ import org.apache.pinot.spi.utils.BytesUtils; import org.apache.pinot.spi.utils.CommonConstants.Server; import org.apache.pinot.spi.utils.ReadMode; +import org.apache.pinot.spi.utils.UuidUtils; import org.apache.pinot.spi.utils.builder.TableConfigBuilder; import org.testng.annotations.AfterClass; import org.testng.annotations.BeforeClass; @@ -75,12 +77,19 @@ public class NoDictionaryGroupKeyGeneratorTest { private static final String STRING_COLUMN = "stringColumn"; private static final String BYTES_COLUMN = "bytesColumn"; private static final String BYTES_DICT_COLUMN = "bytesDictColumn"; + private static final String UUID_COLUMN = "uuidColumn"; + private static final String BOOLEAN_COLUMN = "booleanColumn"; + private static final String TIMESTAMP_COLUMN = "timestampColumn"; + private static final String UUID_DICT_COLUMN = "uuidDictColumn"; private static final List COLUMNS = Arrays.asList(INT_COLUMN, LONG_COLUMN, FLOAT_COLUMN, DOUBLE_COLUMN, STRING_COLUMN, BYTES_COLUMN, - BYTES_DICT_COLUMN); + BYTES_DICT_COLUMN, UUID_COLUMN, BOOLEAN_COLUMN, TIMESTAMP_COLUMN, UUID_DICT_COLUMN); private static final int NUM_COLUMNS = COLUMNS.size(); + private static final Set UUID_COLUMNS = Set.of(UUID_COLUMN, UUID_DICT_COLUMN); private static final TableConfig TABLE_CONFIG = new TableConfigBuilder(TableType.OFFLINE).setTableName(RAW_TABLE_NAME) - .setNoDictionaryColumns(COLUMNS.subList(0, NUM_COLUMNS - 1)).build(); + .setNoDictionaryColumns( + List.of(INT_COLUMN, LONG_COLUMN, FLOAT_COLUMN, DOUBLE_COLUMN, STRING_COLUMN, BYTES_COLUMN, UUID_COLUMN, + BOOLEAN_COLUMN, TIMESTAMP_COLUMN)).build(); private static final Schema SCHEMA = new Schema.SchemaBuilder().addSingleValueDimension(INT_COLUMN, FieldSpec.DataType.INT) .addSingleValueDimension(LONG_COLUMN, FieldSpec.DataType.LONG) @@ -88,7 +97,11 @@ public class NoDictionaryGroupKeyGeneratorTest { .addSingleValueDimension(DOUBLE_COLUMN, FieldSpec.DataType.DOUBLE) .addSingleValueDimension(STRING_COLUMN, FieldSpec.DataType.STRING) .addSingleValueDimension(BYTES_COLUMN, FieldSpec.DataType.BYTES) - .addSingleValueDimension(BYTES_DICT_COLUMN, FieldSpec.DataType.BYTES).build(); + .addSingleValueDimension(BYTES_DICT_COLUMN, FieldSpec.DataType.BYTES) + .addSingleValueDimension(UUID_COLUMN, FieldSpec.DataType.UUID) + .addSingleValueDimension(BOOLEAN_COLUMN, FieldSpec.DataType.BOOLEAN) + .addSingleValueDimension(TIMESTAMP_COLUMN, FieldSpec.DataType.TIMESTAMP) + .addSingleValueDimension(UUID_DICT_COLUMN, FieldSpec.DataType.UUID).build(); private static final int NUM_RECORDS = 1000; private static final int NUM_UNIQUE_RECORDS = 100; @@ -129,6 +142,19 @@ public void setUp() record.putValue(BYTES_DICT_COLUMN, bytesValue); values[5] = BytesUtils.toHexString(bytesValue); values[6] = values[5]; + byte[] uuidBytes = UuidUtils.toBytes(new UUID(RANDOM.nextLong(), RANDOM.nextLong())); + record.putValue(UUID_COLUMN, uuidBytes); + values[7] = UuidUtils.toString(uuidBytes); + // BOOLEAN stored as INT (0/1) — exercises the logical→stored-type normalization fix + int boolIntValue = RANDOM.nextBoolean() ? 1 : 0; + record.putValue(BOOLEAN_COLUMN, boolIntValue); + values[8] = Integer.toString(boolIntValue); + // TIMESTAMP stored as LONG — exercises the logical→stored-type normalization fix + long timestampValue = Math.abs(RANDOM.nextLong()); + record.putValue(TIMESTAMP_COLUMN, timestampValue); + values[9] = Long.toString(timestampValue); + record.putValue(UUID_DICT_COLUMN, uuidBytes); + values[10] = values[7]; for (int j = 0; j < NUM_RECORDS / NUM_UNIQUE_RECORDS; j++) { records.add(record); } @@ -173,9 +199,12 @@ public void testMultiColumnGroupKeyGenerator() { testGroupKeyGenerator(new int[]{0, 1}); testGroupKeyGenerator(new int[]{2, 3}); testGroupKeyGenerator(new int[]{4, 5}); + testGroupKeyGenerator(new int[]{7, 10}); + testGroupKeyGenerator(new int[]{8, 9}); testGroupKeyGenerator(new int[]{1, 2, 3}); testGroupKeyGenerator(new int[]{4, 5, 0}); - testGroupKeyGenerator(new int[]{5, 4, 3, 2, 1, 0}); + testGroupKeyGenerator(new int[]{7, 5, 4}); + testGroupKeyGenerator(new int[]{7, 5, 4, 3, 2, 1, 0}); } /// Tests multi-column group key generator when at least one column as dictionary, and others don't. @@ -212,7 +241,7 @@ private void testGroupKeyGenerator(int[] groupByColumnIndexes) { Iterator groupKeys = groupKeyGenerator.getGroupKeys(); while (groupKeys.hasNext()) { GroupKeyGenerator.GroupKey groupKey = groupKeys.next(); - assertTrue(expectedGroupKeys.contains(getActualGroupKey(groupKey._keys))); + assertTrue(expectedGroupKeys.contains(getActualGroupKey(groupKey._keys, groupByColumnIndexes))); } } @@ -234,13 +263,18 @@ private Set getExpectedGroupKeys(int[] groupByColumnIndexes) { return groupKeys; } - private String getActualGroupKey(Object[] groupKeys) { + private String getActualGroupKey(Object[] groupKeys, int[] groupByColumnIndexes) { StringBuilder stringBuilder = new StringBuilder(); for (int i = 0; i < groupKeys.length; i++) { if (i > 0) { stringBuilder.append(GroupKeyGenerator.DELIMITER); } - stringBuilder.append(groupKeys[i]); + int columnIndex = groupByColumnIndexes[i]; + if (UUID_COLUMNS.contains(COLUMNS.get(columnIndex))) { + stringBuilder.append(UuidUtils.toString(((org.apache.pinot.spi.utils.ByteArray) groupKeys[i]).getBytes())); + } else { + stringBuilder.append(groupKeys[i]); + } } return stringBuilder.toString(); } diff --git a/pinot-core/src/test/java/org/apache/pinot/core/query/distinct/table/BytesDistinctTableTest.java b/pinot-core/src/test/java/org/apache/pinot/core/query/distinct/table/BytesDistinctTableTest.java new file mode 100644 index 000000000000..6ca1c2298cb3 --- /dev/null +++ b/pinot-core/src/test/java/org/apache/pinot/core/query/distinct/table/BytesDistinctTableTest.java @@ -0,0 +1,70 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.core.query.distinct.table; + +import org.apache.pinot.common.request.context.ExpressionContext; +import org.apache.pinot.common.request.context.OrderByExpressionContext; +import org.apache.pinot.common.response.broker.ResultTable; +import org.apache.pinot.common.utils.DataSchema; +import org.apache.pinot.common.utils.DataSchema.ColumnDataType; +import org.apache.pinot.spi.utils.ByteArray; +import org.apache.pinot.spi.utils.BytesUtils; +import org.apache.pinot.spi.utils.UuidUtils; +import org.testng.annotations.Test; + +import static org.testng.Assert.assertEquals; + + +/// Tests for [BytesDistinctTable]. +public class BytesDistinctTableTest { + private static final String UUID_COLUMN = "uuidCol"; + private static final String UUID_VALUE_1 = "550e8400-e29b-41d4-a716-446655440000"; + private static final String UUID_VALUE_2 = "550e8400-e29b-41d4-a716-446655440001"; + + @Test + public void testToResultTableFormatsUuidAndBytesWithoutOrderBy() { + BytesDistinctTable uuidTable = new BytesDistinctTable( + new DataSchema(new String[]{UUID_COLUMN}, new ColumnDataType[]{ColumnDataType.UUID}), 10, false, null); + uuidTable.addUnbounded(new ByteArray(UuidUtils.toBytes(UUID_VALUE_1))); + + ResultTable uuidResultTable = uuidTable.toResultTable(); + assertEquals(uuidResultTable.getRows().get(0)[0], UUID_VALUE_1); + + byte[] bytesValue = new byte[]{0x01, 0x23, 0x45}; + BytesDistinctTable bytesTable = new BytesDistinctTable( + new DataSchema(new String[]{"bytesCol"}, new ColumnDataType[]{ColumnDataType.BYTES}), 10, false, null); + bytesTable.addUnbounded(new ByteArray(bytesValue)); + + ResultTable bytesResultTable = bytesTable.toResultTable(); + assertEquals(bytesResultTable.getRows().get(0)[0], BytesUtils.toHexString(bytesValue)); + } + + @Test + public void testToResultTableFormatsUuidWithOrderBy() { + BytesDistinctTable uuidTable = new BytesDistinctTable( + new DataSchema(new String[]{UUID_COLUMN}, new ColumnDataType[]{ColumnDataType.UUID}), 10, false, + new OrderByExpressionContext(ExpressionContext.forIdentifier(UUID_COLUMN), true)); + uuidTable.addUnbounded(new ByteArray(UuidUtils.toBytes(UUID_VALUE_2))); + uuidTable.addUnbounded(new ByteArray(UuidUtils.toBytes(UUID_VALUE_1))); + + ResultTable resultTable = uuidTable.toResultTable(); + assertEquals(resultTable.getRows().get(0)[0], UUID_VALUE_1); + assertEquals(resultTable.getRows().get(1)[0], UUID_VALUE_2); + } +} From 0903746c71a6fea805260d12907c243331babbe7 Mon Sep 17 00:00:00 2001 From: Xiang Fu Date: Sat, 8 Aug 2026 15:24:01 -0700 Subject: [PATCH 2/8] Return the converted UUID form for group keys in the DataTable reduce path getConvertedKey had `case UUID` falling through to BYTES, returning the raw byte[]. The other reduce path converts group keys via ColumnDataType#convert, and UUID is the one type whose converted form is not its stored bytes -- it yields a java.util.UUID. PredicateRowMatcher casts that directly (see #18872), so the byte[] made GROUP BY ... HAVING over a UUID column fail with "ClassCastException: class [B cannot be cast to class java.util.UUID". Delegating to columnDataType.convert(...) keeps the two paths identical by construction rather than by duplicated knowledge. Covered by a new UuidAggregationTest integration test rather than a unit test. The unit-level BaseQueriesTest harness cannot reach this code, which is why it was uncovered; a query-level test goes through the real broker reduce. Verified by reverting the fix: testGroupByUuidColumnWithHaving and testGroupByUuidColumnWithHavingReturningFinalResult both fail with the ClassCastException and both pass with it. The test also covers GROUP BY key rendering, DISTINCT (BytesDistinctTable no longer hard-codes hex) and DISTINCTCOUNT / DISTINCTCOUNTHLL / DISTINCTCOUNTBITMAP over a UUID column. --- .../query/reduce/GroupByDataTableReducer.java | 8 +- .../tests/custom/UuidAggregationTest.java | 209 ++++++++++++++++++ 2 files changed, 216 insertions(+), 1 deletion(-) create mode 100644 pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/custom/UuidAggregationTest.java diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/reduce/GroupByDataTableReducer.java b/pinot-core/src/main/java/org/apache/pinot/core/query/reduce/GroupByDataTableReducer.java index b0abf23afb08..be757ecd9770 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/reduce/GroupByDataTableReducer.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/reduce/GroupByDataTableReducer.java @@ -530,8 +530,14 @@ private Object getConvertedKey(DataTable dataTable, ColumnDataType columnDataTyp case JSON: return dataTable.getString(rowId, colId); case BYTES: - case UUID: return dataTable.getBytes(rowId, colId).getBytes(); + case UUID: + // Deliberately delegated to ColumnDataType#convert rather than falling through to BYTES. The other reduce + // path (reduceWithIndexedTable) converts group keys with exactly that method, 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 returning the raw byte[] here makes GROUP BY ... HAVING on a UUID column throw + // ClassCastException. Delegating keeps the two paths identical by construction. + return columnDataType.convert(dataTable.getBytes(rowId, colId)); default: throw new IllegalStateException("Illegal column data type in group key: " + columnDataType); } diff --git a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/custom/UuidAggregationTest.java b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/custom/UuidAggregationTest.java new file mode 100644 index 000000000000..016ed27e3606 --- /dev/null +++ b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/custom/UuidAggregationTest.java @@ -0,0 +1,209 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.integration.tests.custom; + +import com.fasterxml.jackson.databind.JsonNode; +import java.io.File; +import java.util.ArrayList; +import java.util.List; +import org.apache.avro.file.DataFileWriter; +import org.apache.avro.generic.GenericData; +import org.apache.pinot.spi.config.table.TableConfig; +import org.apache.pinot.spi.config.table.TableType; +import org.apache.pinot.spi.data.FieldSpec.DataType; +import org.apache.pinot.spi.data.Schema; +import org.apache.pinot.spi.utils.builder.TableConfigBuilder; +import org.testng.annotations.Test; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertTrue; + + +/// End-to-end coverage for aggregating, grouping and de-duplicating a UUID column. +/// +/// These run through a real broker reduce, which is the point: the group-key conversion in +/// `GroupByDataTableReducer#getConvertedKey` is only reachable when the broker reduces a *single* data table, and the +/// unit-level `BaseQueriesTest` harness always reduces two. A `case UUID` there that returns the stored `byte[]` +/// instead of the converted `java.util.UUID` makes `GROUP BY ... HAVING` over a UUID column fail with +/// `ClassCastException: class [B cannot be cast to class java.util.UUID`, and only a query-level test catches it. +@Test(suiteName = "CustomClusterIntegrationTest") +public class UuidAggregationTest extends CustomDataQueryClusterIntegrationTest { + private static final String TABLE_NAME = "UuidAggregationTest"; + private static final String UUID_COLUMN = "uuidColumn"; + private static final String UUID_0 = "550e8400-e29b-41d4-a716-446655440000"; + private static final String UUID_0_HEX = "550e8400e29b41d4a716446655440000"; + private static final String UUID_1 = "550e8400-e29b-41d4-a716-446655440001"; + private static final String UUID_2 = "550e8400-e29b-41d4-a716-446655440002"; + + /// `UUID_0` appears twice so grouping and distinct are distinguishable from a plain row count. + private static final List ROWS = List.of(UUID_0, UUID_0, UUID_1, UUID_2); + private static final int NUM_DISTINCT = 3; + + @Override + public String getTableName() { + return TABLE_NAME; + } + + @Override + protected long getCountStarResult() { + return ROWS.size(); + } + + @Override + public int getNumAvroFiles() { + return 1; + } + + @Override + public TableConfig createOfflineTableConfig() { + return new TableConfigBuilder(TableType.OFFLINE).setTableName(getTableName()).build(); + } + + @Override + public Schema createSchema() { + return new Schema.SchemaBuilder().setSchemaName(getTableName()) + .addSingleValueDimension(UUID_COLUMN, DataType.UUID) + .build(); + } + + @Override + public List createAvroFiles() + throws Exception { + org.apache.avro.Schema avroSchema = org.apache.avro.Schema.createRecord("uuidRecord", null, null, false); + avroSchema.setFields(List.of(new org.apache.avro.Schema.Field(UUID_COLUMN, + org.apache.avro.Schema.create(org.apache.avro.Schema.Type.STRING), null, null))); + + try (AvroFilesAndWriters avroFilesAndWriters = createAvroFilesAndWriters(avroSchema)) { + DataFileWriter writer = avroFilesAndWriters.getWriters().get(0); + for (String uuid : ROWS) { + GenericData.Record record = new GenericData.Record(avroSchema); + record.put(UUID_COLUMN, uuid); + writer.append(record); + } + return avroFilesAndWriters.getAvroFiles(); + } + } + + @Test + public void testGroupByUuidColumn() + throws Exception { + setUseMultiStageQueryEngine(false); + JsonNode rows = query( + String.format("SELECT %s, COUNT(*) FROM %s GROUP BY %s ORDER BY %s", UUID_COLUMN, getTableName(), UUID_COLUMN, + UUID_COLUMN)); + assertEquals(rows.size(), NUM_DISTINCT, rows.toPrettyString()); + + // Group keys must come back as canonical UUIDs, not hex and not a byte-array rendering. + List keys = new ArrayList<>(); + for (JsonNode row : rows) { + keys.add(row.get(0).asText()); + } + assertEquals(keys, List.of(UUID_0, UUID_1, UUID_2), rows.toPrettyString()); + assertEquals(rows.get(0).get(1).asLong(), 2, rows.toPrettyString()); + assertEquals(rows.get(1).get(1).asLong(), 1, rows.toPrettyString()); + } + + /// The regression that motivated this class: `GROUP BY` a UUID column with a `HAVING` predicate on that same + /// column runs the group key through `getConvertedKey` and then straight into `PredicateRowMatcher`, which casts + /// to `java.util.UUID`. + @Test + public void testGroupByUuidColumnWithHaving() + throws Exception { + setUseMultiStageQueryEngine(false); + JsonNode rows = query( + String.format("SELECT %s, COUNT(*) FROM %s GROUP BY %s HAVING %s = '%s'", UUID_COLUMN, getTableName(), + UUID_COLUMN, UUID_COLUMN, UUID_0_HEX)); + assertEquals(rows.size(), 1, rows.toPrettyString()); + assertEquals(rows.get(0).get(0).asText(), UUID_0, rows.toPrettyString()); + assertEquals(rows.get(0).get(1).asLong(), 2, rows.toPrettyString()); + + // Same thing via an explicit CAST of the canonical form. + rows = query(String.format("SELECT %s, COUNT(*) FROM %s GROUP BY %s HAVING %s = CAST('%s' AS UUID)", UUID_COLUMN, + getTableName(), UUID_COLUMN, UUID_COLUMN, UUID_1)); + assertEquals(rows.size(), 1, rows.toPrettyString()); + assertEquals(rows.get(0).get(0).asText(), UUID_1, rows.toPrettyString()); + assertEquals(rows.get(0).get(1).asLong(), 1, rows.toPrettyString()); + } + + /// 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. + @Test + public void testGroupByUuidColumnWithHavingReturningFinalResult() + throws Exception { + setUseMultiStageQueryEngine(false); + JsonNode rows = query( + String.format("SELECT %s, COUNT(*) FROM %s GROUP BY %s HAVING %s = '%s' OPTION(serverReturnFinalResult=true)", + UUID_COLUMN, getTableName(), UUID_COLUMN, UUID_COLUMN, UUID_0_HEX)); + assertEquals(rows.size(), 1, rows.toPrettyString()); + assertEquals(rows.get(0).get(0).asText(), UUID_0, rows.toPrettyString()); + assertEquals(rows.get(0).get(1).asLong(), 2, rows.toPrettyString()); + } + + /// Group keys must also render canonically on the `getConvertedKey` path, not as hex. + @Test + public void testGroupByUuidColumnReturningFinalResult() + throws Exception { + setUseMultiStageQueryEngine(false); + JsonNode rows = query( + String.format("SELECT %s, COUNT(*) FROM %s GROUP BY %s ORDER BY %s OPTION(serverReturnFinalResult=true)", + UUID_COLUMN, getTableName(), UUID_COLUMN, UUID_COLUMN)); + assertEquals(rows.size(), NUM_DISTINCT, rows.toPrettyString()); + List keys = new ArrayList<>(); + for (JsonNode row : rows) { + keys.add(row.get(0).asText()); + } + assertEquals(keys, List.of(UUID_0, UUID_1, UUID_2), rows.toPrettyString()); + } + + @Test + public void testDistinctOnUuidColumn() + throws Exception { + setUseMultiStageQueryEngine(false); + JsonNode rows = + query(String.format("SELECT DISTINCT %s FROM %s ORDER BY %s", UUID_COLUMN, getTableName(), UUID_COLUMN)); + assertEquals(rows.size(), NUM_DISTINCT, rows.toPrettyString()); + + // BytesDistinctTable used to hard-code toHexString(); a UUID column must render canonically. + List values = new ArrayList<>(); + for (JsonNode row : rows) { + values.add(row.get(0).asText()); + } + assertEquals(values, List.of(UUID_0, UUID_1, UUID_2), rows.toPrettyString()); + } + + @Test + public void testDistinctCountOnUuidColumn() + throws Exception { + setUseMultiStageQueryEngine(false); + for (String function : List.of("DISTINCTCOUNT", "DISTINCTCOUNTHLL", "DISTINCTCOUNTBITMAP")) { + JsonNode rows = query(String.format("SELECT %s(%s) FROM %s", function, UUID_COLUMN, getTableName())); + assertEquals(rows.get(0).get(0).asLong(), NUM_DISTINCT, function + ": " + rows.toPrettyString()); + } + } + + private JsonNode query(String sql) + throws Exception { + JsonNode response = postQuery(sql); + assertTrue(response.path("exceptions").isEmpty(), sql + " -> " + response.toPrettyString()); + return response.path("resultTable").path("rows"); + } +} From fd6df4872e42edc196511954f89c936caab378ab Mon Sep 17 00:00:00 2001 From: Xiang Fu Date: Sat, 8 Aug 2026 16:40:41 -0700 Subject: [PATCH 3/8] Cast directly to UuidKey in UuidToIdMap Both callers key on UuidKey already (NoDictionary{Single,Multi}ColumnGroupKey Generator, via UuidKey.fromBytes), so UuidKey.fromObject accepted five input types where exactly one is ever passed, and ran an instanceof chain per row in the group-by loop. Casting directly matches the sibling maps -- DoubleToIdMap casts to double -- and keeps the input type deterministic, which is what was asked for on the equivalent PredicateRowMatcher branch in #18872. --- .../query/aggregation/groupby/utils/UuidToIdMap.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/utils/UuidToIdMap.java b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/utils/UuidToIdMap.java index 845ad21c37a9..0ee338e97013 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/utils/UuidToIdMap.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/utils/UuidToIdMap.java @@ -35,9 +35,14 @@ public UuidToIdMap() { _idToValueMap = new ArrayList<>(); } + /// Both callers -- [org.apache.pinot.core.query.aggregation.groupby.NoDictionaryMultiColumnGroupKeyGenerator] and + /// [org.apache.pinot.core.query.aggregation.groupby.NoDictionarySingleColumnGroupKeyGenerator] -- key on + /// [UuidKey] already, so this casts directly rather than going through `UuidKey#fromObject`. That matches the + /// sibling maps (e.g. [DoubleToIdMap] casts to `double`) and keeps the per-row `instanceof` chain out of the + /// group-by loop. @Override public int put(Object value) { - UuidKey uuidKey = UuidKey.fromObject(value); + UuidKey uuidKey = (UuidKey) value; int id = _valueToIdMap.getInt(uuidKey); if (id == INVALID_KEY) { id = _valueToIdMap.size(); @@ -49,7 +54,7 @@ public int put(Object value) { @Override public int getId(Object value) { - return _valueToIdMap.getInt(UuidKey.fromObject(value)); + return _valueToIdMap.getInt((UuidKey) value); } @Override From 1977e2c2890275d79b9732d43c5b641f003ba401 Mon Sep 17 00:00:00 2001 From: Xiang Fu Date: Sun, 9 Aug 2026 02:06:00 -0700 Subject: [PATCH 4/8] Hash UUID's stored bytes in distinct-count aggregations, not a canonical string The previous version 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 offers its raw millis and BOOLEAN its int, and neither matches a CAST to STRING. The UUID rendering was inventing a cross-type equivalence at the cost of a String allocation per row in the aggregation loop. UUID now hashes its stored 16 bytes. Verified byte[] is content-hashed rather than identity-hashed by both HyperLogLog (clearspring MurmurHash) and UltraLogLogUtils.OBJECT_FUNNEL (putBytes). A minimal guard is still needed at each site, because unlike LONG or INT the stored BYTES type is not a scalar case in this family: the scan path has no `case BYTES`, and the dictionary path reads BYTES as serialized sketch state. - AggregationFunctionUtils: the three 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. - DistinctCountBitmap hashes Arrays.hashCode(bytes). - DistinctCountThetaSketch cannot take scalar bytes (BYTES there means "serialized sketch"), so it surfaces the stored hex rendering instead. Replaces testUuidDistinctCountHllMatchesStringDistinctCountHll, which asserted the invariant being dropped, with one pinning the new behaviour. --- .../function/AggregationFunctionUtils.java | 52 ++++++------------- ...istinctCountBitmapAggregationFunction.java | 8 +-- ...inctCountCPCSketchAggregationFunction.java | 7 ++- .../DistinctCountHLLAggregationFunction.java | 7 ++- ...stinctCountHLLPlusAggregationFunction.java | 7 ++- ...ctCountThetaSketchAggregationFunction.java | 28 +++++----- .../DistinctCountULLAggregationFunction.java | 7 ++- ...stinctCountHLLAggregationFunctionTest.java | 30 ++++++++--- 8 files changed, 67 insertions(+), 79 deletions(-) diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionUtils.java b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionUtils.java index c5edbd429485..6908ef1f19d8 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionUtils.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionUtils.java @@ -74,7 +74,6 @@ import org.apache.pinot.spi.data.FieldSpec; import org.apache.pinot.spi.query.QueryThreadContext; import org.apache.pinot.spi.utils.ByteArray; -import org.apache.pinot.spi.utils.UuidUtils; /// The `AggregationFunctionUtils` class provides utility methods for aggregation function. @@ -803,18 +802,11 @@ private static HyperLogLogPlus getDistinctValueHLLPlus(Dictionary dictionary, in private static HyperLogLog getDistinctCountHLLResult(DataSource dataSource, DistinctCountHLLAggregationFunction function, String explainPlanName) { Dictionary dictionary = Objects.requireNonNull(dataSource.getDictionary()); - // UUID dictionary entries are logical scalar values, not serialized HyperLogLogs. Offer their canonical string - // representation to match the scan-based path and DISTINCTCOUNTHLL(CAST(uuidColumn AS STRING)). - if (dataSource.getDataSourceMetadata().getDataType() == FieldSpec.DataType.UUID) { - HyperLogLog hll = new HyperLogLog(function.getLog2m()); - int length = dictionary.length(); - for (int i = 0; i < length; i++) { - QueryThreadContext.checkTerminationAndSampleUsagePeriodically(i, explainPlanName); - hll.offer(UuidUtils.toString(dictionary.getBytesValue(i))); - } - return hll; - } - if (dictionary.getValueType() == FieldSpec.DataType.BYTES) { + // A UUID column's dictionary reports BYTES (it is a plain BytesDictionary), but its entries are logical + // scalars, not serialized sketch state. Excluding it here lets it fall through to the scalar path + // below, which offers dictionary.get(i) -- the stored byte[] -- exactly as the scan path does. + if (dataSource.getDataSourceMetadata().getDataType() != FieldSpec.DataType.UUID + && dictionary.getValueType() == FieldSpec.DataType.BYTES) { // Treat BYTES value as serialized HyperLogLog try { QueryThreadContext.checkTerminationAndSampleUsage(explainPlanName); @@ -836,18 +828,11 @@ private static HyperLogLog getDistinctCountHLLResult(DataSource dataSource, private static HyperLogLogPlus getDistinctCountHLLPlusResult(DataSource dataSource, DistinctCountHLLPlusAggregationFunction function, String explainPlanName) { Dictionary dictionary = Objects.requireNonNull(dataSource.getDictionary()); - // UUID dictionary entries are logical scalar values, not serialized HyperLogLogPluses. Offer their canonical - // string representation to match the scan-based path and DISTINCTCOUNTHLLPLUS(CAST(uuidColumn AS STRING)). - if (dataSource.getDataSourceMetadata().getDataType() == FieldSpec.DataType.UUID) { - HyperLogLogPlus hllPlus = new HyperLogLogPlus(function.getP(), function.getSp()); - int length = dictionary.length(); - for (int i = 0; i < length; i++) { - QueryThreadContext.checkTerminationAndSampleUsagePeriodically(i, explainPlanName); - hllPlus.offer(UuidUtils.toString(dictionary.getBytesValue(i))); - } - return hllPlus; - } - if (dictionary.getValueType() == FieldSpec.DataType.BYTES) { + // A UUID column's dictionary reports BYTES (it is a plain BytesDictionary), but its entries are logical + // scalars, not serialized sketch state. Excluding it here lets it fall through to the scalar path + // below, which offers dictionary.get(i) -- the stored byte[] -- exactly as the scan path does. + if (dataSource.getDataSourceMetadata().getDataType() != FieldSpec.DataType.UUID + && dictionary.getValueType() == FieldSpec.DataType.BYTES) { // Treat BYTES value as serialized HyperLogLogPlus try { QueryThreadContext.checkTerminationAndSampleUsage(explainPlanName); @@ -889,18 +874,11 @@ private static Object getDistinctCountSmartHLLPlusResult(Dictionary dictionary, private static UltraLogLog getDistinctCountULLResult(DataSource dataSource, DistinctCountULLAggregationFunction function, String explainPlanName) { Dictionary dictionary = Objects.requireNonNull(dataSource.getDictionary()); - // UUID dictionary entries are logical scalar values, not serialized UltraLogLogs. Hash their canonical string - // representation to match the scan-based path and DISTINCTCOUNTULL(CAST(uuidColumn AS STRING)). - if (dataSource.getDataSourceMetadata().getDataType() == FieldSpec.DataType.UUID) { - UltraLogLog ull = UltraLogLog.create(function.getP()); - int length = dictionary.length(); - for (int i = 0; i < length; i++) { - QueryThreadContext.checkTerminationAndSampleUsagePeriodically(i, explainPlanName); - UltraLogLogUtils.hashObject(UuidUtils.toString(dictionary.getBytesValue(i))).ifPresent(ull::add); - } - return ull; - } - if (dictionary.getValueType() == FieldSpec.DataType.BYTES) { + // A UUID column's dictionary reports BYTES (it is a plain BytesDictionary), but its entries are logical + // scalars, not serialized sketch state. Excluding it here lets it fall through to the scalar path + // below, which offers dictionary.get(i) -- the stored byte[] -- exactly as the scan path does. + if (dataSource.getDataSourceMetadata().getDataType() != FieldSpec.DataType.UUID + && dictionary.getValueType() == FieldSpec.DataType.BYTES) { // Treat BYTES value as serialized UltraLogLog and merge try { QueryThreadContext.checkTerminationAndSampleUsage(explainPlanName); diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountBitmapAggregationFunction.java b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountBitmapAggregationFunction.java index 04a3bc0d9b4d..03c10883f36a 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountBitmapAggregationFunction.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountBitmapAggregationFunction.java @@ -18,6 +18,7 @@ */ package org.apache.pinot.core.query.aggregation.function; +import java.util.Arrays; import java.util.List; import java.util.Map; import javax.annotation.Nullable; @@ -34,7 +35,6 @@ import org.apache.pinot.segment.spi.AggregationFunctionType; import org.apache.pinot.segment.spi.index.reader.Dictionary; import org.apache.pinot.spi.data.FieldSpec.DataType; -import org.apache.pinot.spi.utils.UuidUtils; import org.roaringbitmap.PeekableIntIterator; import org.roaringbitmap.RoaringBitmap; @@ -82,7 +82,7 @@ public void aggregate(int length, AggregationResultHolder aggregationResultHolde byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); RoaringBitmap bitmap = getValueBitmap(aggregationResultHolder); for (int i = 0; i < length; i++) { - bitmap.add(UuidUtils.toString(uuidBytesValues[i]).hashCode()); + bitmap.add(Arrays.hashCode(uuidBytesValues[i])); } return; } @@ -232,7 +232,7 @@ public void aggregateGroupBySV(int length, int[] groupKeyArray, GroupByResultHol byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); for (int i = 0; i < length; i++) { getValueBitmap(groupByResultHolder, groupKeyArray[i]) - .add(UuidUtils.toString(uuidBytesValues[i]).hashCode()); + .add(Arrays.hashCode(uuidBytesValues[i])); } return; } @@ -384,7 +384,7 @@ public void aggregateGroupByMV(int length, int[][] groupKeysArray, GroupByResult if (dataType == DataType.UUID) { byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); for (int i = 0; i < length; i++) { - int hash = UuidUtils.toString(uuidBytesValues[i]).hashCode(); + int hash = Arrays.hashCode(uuidBytesValues[i]); for (int groupKey : groupKeysArray[i]) { getValueBitmap(groupByResultHolder, groupKey).add(hash); } diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountCPCSketchAggregationFunction.java b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountCPCSketchAggregationFunction.java index 7111f211e509..96e286aae1e9 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountCPCSketchAggregationFunction.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountCPCSketchAggregationFunction.java @@ -41,7 +41,6 @@ import org.apache.pinot.spi.data.FieldSpec; import org.apache.pinot.spi.data.FieldSpec.DataType; import org.apache.pinot.spi.utils.CommonConstants; -import org.apache.pinot.spi.utils.UuidUtils; import org.roaringbitmap.PeekableIntIterator; import org.roaringbitmap.RoaringBitmap; @@ -148,7 +147,7 @@ public void aggregate(int length, AggregationResultHolder aggregationResultHolde // Calling getAccumulator here would read the holder slot already occupied by the sketch and fail. CpcSketch cpcSketch = getCpcSketch(aggregationResultHolder); for (int i = 0; i < length; i++) { - cpcSketch.update(UuidUtils.toString(uuidBytesValues[i])); + cpcSketch.update(uuidBytesValues[i]); } return; } @@ -231,7 +230,7 @@ public void aggregateGroupBySV(int length, int[] groupKeyArray, GroupByResultHol if (dataType == DataType.UUID) { byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); for (int i = 0; i < length; i++) { - getCpcSketch(groupByResultHolder, groupKeyArray[i]).update(UuidUtils.toString(uuidBytesValues[i])); + getCpcSketch(groupByResultHolder, groupKeyArray[i]).update(uuidBytesValues[i]); } return; } @@ -314,7 +313,7 @@ public void aggregateGroupByMV(int length, int[][] groupKeysArray, GroupByResult if (dataType == DataType.UUID && singleValue) { byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); for (int i = 0; i < length; i++) { - String canonical = UuidUtils.toString(uuidBytesValues[i]); + byte[] canonical = uuidBytesValues[i]; for (int groupKey : groupKeysArray[i]) { getCpcSketch(groupByResultHolder, groupKey).update(canonical); } diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLAggregationFunction.java b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLAggregationFunction.java index 8a74f972f7f3..8a837029e977 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLAggregationFunction.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLAggregationFunction.java @@ -38,7 +38,6 @@ import org.apache.pinot.segment.spi.index.reader.Dictionary; import org.apache.pinot.spi.data.FieldSpec.DataType; import org.apache.pinot.spi.utils.CommonConstants; -import org.apache.pinot.spi.utils.UuidUtils; import org.roaringbitmap.RoaringBitmap; @@ -94,7 +93,7 @@ public void aggregate(int length, AggregationResultHolder aggregationResultHolde byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); HyperLogLog hyperLogLog = getHyperLogLog(aggregationResultHolder); for (int i = 0; i < length; i++) { - hyperLogLog.offer(UuidUtils.toString(uuidBytesValues[i])); + hyperLogLog.offer(uuidBytesValues[i]); } return; } @@ -256,7 +255,7 @@ public void aggregateGroupBySV(int length, int[] groupKeyArray, GroupByResultHol if (dataType == DataType.UUID) { byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); for (int i = 0; i < length; i++) { - getHyperLogLog(groupByResultHolder, groupKeyArray[i]).offer(UuidUtils.toString(uuidBytesValues[i])); + getHyperLogLog(groupByResultHolder, groupKeyArray[i]).offer(uuidBytesValues[i]); } return; } @@ -416,7 +415,7 @@ public void aggregateGroupByMV(int length, int[][] groupKeysArray, GroupByResult if (dataType == DataType.UUID) { byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); for (int i = 0; i < length; i++) { - String canonical = UuidUtils.toString(uuidBytesValues[i]); + byte[] canonical = uuidBytesValues[i]; for (int groupKey : groupKeysArray[i]) { getHyperLogLog(groupByResultHolder, groupKey).offer(canonical); } diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLPlusAggregationFunction.java b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLPlusAggregationFunction.java index 1c2762670c61..452f33412099 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLPlusAggregationFunction.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLPlusAggregationFunction.java @@ -37,7 +37,6 @@ import org.apache.pinot.segment.spi.index.reader.Dictionary; import org.apache.pinot.spi.data.FieldSpec.DataType; import org.apache.pinot.spi.utils.CommonConstants; -import org.apache.pinot.spi.utils.UuidUtils; import org.roaringbitmap.PeekableIntIterator; import org.roaringbitmap.RoaringBitmap; @@ -103,7 +102,7 @@ public void aggregate(int length, AggregationResultHolder aggregationResultHolde byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); HyperLogLogPlus hyperLogLogPlus = getHyperLogLogPlus(aggregationResultHolder); for (int i = 0; i < length; i++) { - hyperLogLogPlus.offer(UuidUtils.toString(uuidBytesValues[i])); + hyperLogLogPlus.offer(uuidBytesValues[i]); } return; } @@ -258,7 +257,7 @@ public void aggregateGroupBySV(int length, int[] groupKeyArray, GroupByResultHol if (dataType == DataType.UUID) { byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); for (int i = 0; i < length; i++) { - getHyperLogLogPlus(groupByResultHolder, groupKeyArray[i]).offer(UuidUtils.toString(uuidBytesValues[i])); + getHyperLogLogPlus(groupByResultHolder, groupKeyArray[i]).offer(uuidBytesValues[i]); } return; } @@ -417,7 +416,7 @@ public void aggregateGroupByMV(int length, int[][] groupKeysArray, GroupByResult if (dataType == DataType.UUID) { byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); for (int i = 0; i < length; i++) { - String canonical = UuidUtils.toString(uuidBytesValues[i]); + byte[] canonical = uuidBytesValues[i]; for (int groupKey : groupKeysArray[i]) { getHyperLogLogPlus(groupByResultHolder, groupKey).offer(canonical); } diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountThetaSketchAggregationFunction.java b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountThetaSketchAggregationFunction.java index 67e9c2996ef9..263cf8af0091 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountThetaSketchAggregationFunction.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountThetaSketchAggregationFunction.java @@ -55,8 +55,8 @@ import org.apache.pinot.segment.spi.AggregationFunctionType; import org.apache.pinot.segment.spi.Constants; import org.apache.pinot.spi.data.FieldSpec.DataType; +import org.apache.pinot.spi.utils.BytesUtils; import org.apache.pinot.spi.utils.CommonConstants; -import org.apache.pinot.spi.utils.UuidUtils; import org.apache.pinot.sql.parsers.CalciteSqlParser; @@ -1237,32 +1237,32 @@ private void extractValues(int length, Map block DataType storedType = dataType.getStoredType(); singleValues[i] = singleValue; // UUID columns are stored as 16-byte BYTES but a UUID value is a logical scalar, not a pre-serialized - // theta sketch. Surface UUID as STRING (canonical UUID form) so the downstream update-sketch path - // matches DISTINCTCOUNTTHETASKETCH(CAST(uuidCol AS STRING)). Without this branch, the function would - // take the serialized-sketch path below and Sketch.wrap would fail on raw 16-byte UUID content. - // NOTE: fetch raw bytes and convert explicitly — for identifier expressions the BlockValSet is a - // ProjectionBlockValSet whose getStringValuesSV() renders stored BYTES as bare hex, not canonical form. + // theta sketch: without this branch the function takes the serialized-sketch path below and Sketch.wrap + // fails on raw 16-byte UUID content. Unlike the other distinct-count functions this one cannot consume the + // stored bytes directly -- DataType.BYTES here means "serialized sketch", with no scalar-bytes mode -- so + // the stored value is surfaced as its hex rendering, the same form used at every other String-typed UUID + // boundary (see PredicateUtils#getStoredValue and the Bloom filter key). if (dataType == DataType.UUID) { valueTypes[i] = DataType.STRING; if (singleValue) { byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); - String[] canonicalValues = new String[length]; + String[] hexValues = new String[length]; for (int j = 0; j < length; j++) { - canonicalValues[j] = UuidUtils.toString(uuidBytesValues[j]); + hexValues[j] = BytesUtils.toHexString(uuidBytesValues[j]); } - valueArrays[i] = canonicalValues; + valueArrays[i] = hexValues; } else { byte[][][] uuidBytesValuesMV = blockValSet.getBytesValuesMV(); - String[][] canonicalValuesMV = new String[length][]; + String[][] hexValuesMV = new String[length][]; for (int j = 0; j < length; j++) { byte[][] row = uuidBytesValuesMV[j]; - String[] canonicalRow = new String[row.length]; + String[] hexRow = new String[row.length]; for (int k = 0; k < row.length; k++) { - canonicalRow[k] = UuidUtils.toString(row[k]); + hexRow[k] = BytesUtils.toHexString(row[k]); } - canonicalValuesMV[j] = canonicalRow; + hexValuesMV[j] = hexRow; } - valueArrays[i] = canonicalValuesMV; + valueArrays[i] = hexValuesMV; } continue; } diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountULLAggregationFunction.java b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountULLAggregationFunction.java index cad535aa2aea..3a9319e00c7c 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountULLAggregationFunction.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountULLAggregationFunction.java @@ -38,7 +38,6 @@ import org.apache.pinot.segment.spi.index.reader.Dictionary; import org.apache.pinot.spi.data.FieldSpec.DataType; import org.apache.pinot.spi.utils.CommonConstants; -import org.apache.pinot.spi.utils.UuidUtils; import org.roaringbitmap.PeekableIntIterator; import org.roaringbitmap.RoaringBitmap; @@ -92,7 +91,7 @@ public void aggregate(int length, AggregationResultHolder aggregationResultHolde byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); UltraLogLog ull = getULL(aggregationResultHolder); for (int i = 0; i < length; i++) { - UltraLogLogUtils.hashObject(UuidUtils.toString(uuidBytesValues[i])).ifPresent(ull::add); + UltraLogLogUtils.hashObject(uuidBytesValues[i]).ifPresent(ull::add); } return; } @@ -177,7 +176,7 @@ public void aggregateGroupBySV(int length, int[] groupKeyArray, GroupByResultHol byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); for (int i = 0; i < length; i++) { UltraLogLog ull = getULL(groupByResultHolder, groupKeyArray[i]); - UltraLogLogUtils.hashObject(UuidUtils.toString(uuidBytesValues[i])).ifPresent(ull::add); + UltraLogLogUtils.hashObject(uuidBytesValues[i]).ifPresent(ull::add); } return; } @@ -267,7 +266,7 @@ public void aggregateGroupByMV(int length, int[][] groupKeysArray, GroupByResult if (dataType == DataType.UUID) { byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); for (int i = 0; i < length; i++) { - String canonical = UuidUtils.toString(uuidBytesValues[i]); + byte[] canonical = uuidBytesValues[i]; for (int groupKey : groupKeysArray[i]) { UltraLogLog ull = getULL(groupByResultHolder, groupKey); UltraLogLogUtils.hashObject(canonical).ifPresent(ull::add); diff --git a/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLAggregationFunctionTest.java b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLAggregationFunctionTest.java index 71feab4aa979..3049fae37358 100644 --- a/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLAggregationFunctionTest.java +++ b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLAggregationFunctionTest.java @@ -19,6 +19,7 @@ package org.apache.pinot.core.query.aggregation.function; import com.clearspring.analytics.stream.cardinality.HyperLogLog; +import java.util.Arrays; import java.util.BitSet; import java.util.HashMap; import java.util.List; @@ -32,6 +33,7 @@ import org.apache.pinot.segment.spi.Constants; import org.apache.pinot.segment.spi.index.reader.Dictionary; import org.apache.pinot.spi.data.FieldSpec.DataType; +import org.apache.pinot.spi.utils.CommonConstants; import org.apache.pinot.spi.utils.UuidUtils; import org.testng.Assert; import org.testng.annotations.Test; @@ -105,22 +107,34 @@ public void testAggregateOnUuidColumnOffersCanonicalStringsAndProducesExactDisti "HLL cardinality must equal the 3 distinct UUIDs; got " + cardinality); } - /// Cross-type consistency: DISTINCTCOUNTHLL(uuidCol) must produce the same HLL cardinality as - /// DISTINCTCOUNTHLL(stringRepresentationOfSameUuids). Locks in the design contract that UUID columns are - /// hashed as canonical UUID strings. + /// UUID columns hash their **stored bytes**, exactly as TIMESTAMP hashes its stored millis rather than a + /// formatted string. Consequence: DISTINCTCOUNTHLL(uuidCol) does NOT equal + /// DISTINCTCOUNTHLL(CAST(uuidCol AS STRING)) -- and neither does it for TIMESTAMP, so this is the consistent + /// behaviour for a logical type, not a gap. Pinned here so nobody "fixes" it back into a canonical-string + /// rendering, which would reintroduce a per-row String allocation in the aggregation loop. @Test - public void testUuidDistinctCountHllMatchesStringDistinctCountHll() { + public void testUuidDistinctCountHllHashesStoredBytesNotCanonicalString() + throws java.io.IOException { String[] uuidStrings = new String[]{ "550e8400-e29b-41d4-a716-446655440000", "12345678-1234-1234-1234-1234567890ab", "9c5e1f24-0b8e-4c9d-87f1-0aa64a3b9d12" }; - long uuidHllCardinality = computeHllCardinality(uuidStrings, DataType.UUID); - long stringHllCardinality = computeHllCardinality(uuidStrings, DataType.STRING); + // Cardinality is still exact for a small distinct set... + Assert.assertEquals(computeHllCardinality(uuidStrings, DataType.UUID), 3L); - Assert.assertEquals(uuidHllCardinality, stringHllCardinality, - "DISTINCTCOUNTHLL(uuidCol) must match DISTINCTCOUNTHLL(CAST(uuidCol AS STRING))"); + // ...but the sketch is built over the 16 stored bytes, so a HyperLogLog fed the canonical strings differs. + HyperLogLog fromCanonicalStrings = new HyperLogLog(CommonConstants.Helix.DEFAULT_HYPERLOGLOG_LOG2M); + for (String uuid : uuidStrings) { + fromCanonicalStrings.offer(uuid); + } + HyperLogLog fromStoredBytes = new HyperLogLog(CommonConstants.Helix.DEFAULT_HYPERLOGLOG_LOG2M); + for (String uuid : uuidStrings) { + fromStoredBytes.offer(UuidUtils.toBytes(uuid)); + } + Assert.assertFalse(Arrays.equals(fromCanonicalStrings.getBytes(), fromStoredBytes.getBytes()), + "stored-bytes and canonical-string sketches are expected to differ"); } private long computeHllCardinality(String[] values, DataType valueType) { From 1a5b302124e74d20675610892346b67ce86ffcf4 Mon Sep 17 00:00:00 2001 From: Xiang Fu Date: Sun, 9 Aug 2026 03:47:45 -0700 Subject: [PATCH 5/8] Drop UUID special-casing that the stored type already covers Applying the rule that if TIMESTAMP and BIG_DECIMAL need no special handling at a site, UUID does not either -- their stored types carry them, and UUID's should too. Removed: - NoDictionary{Single,Multi}ColumnGroupKeyGenerator, UuidToIdMap and its ValueToIdMapFactory entry. `case BYTES` is already supported there and is structurally identical to the UUID branch: same getBytesValuesSV(), same loop, same map, differing only in wrapping UuidKey vs ByteArray. Both yield ByteArray downstream, which is what ColumnDataType.UUID#convert consumes, so this was a pure key-representation optimization -- two primitive longs instead of a byte[] wrapper -- with no benchmark to justify 5 files and +237/-36. UuidAggregationTest#testGroupByUuidColumn still passes, confirming group keys render identically without it. - AnyValueAggregationFunction. Its switch is on getStoredType(), so TIMESTAMP already collapses to LONG (raw millis) and BOOLEAN to INT. UUID collapsing to BYTES is the consistent behaviour; the branch made it the odd one out. - IntegerTupleSketchAggregationFunction. The UUID branch only produced a friendlier error message; TIMESTAMP gets no such treatment when it is equally unusable there. Kept, because the same rule shows they are needed: - BytesDistinctTable: LongDistinctTable and BigDecimalDistinctTable already render by ColumnDataType, and MultiColumnDistinctTable already calls convertAndFormat. BytesDistinctTable hard-coding toHexString() was the outlier; this brings it in line rather than special-casing UUID. - The distinct-count guards and GroupByDataTableReducer, both verified necessary by probe -- BYTES means "serialized sketch" in one and the group key must convert in the other. 19 files / +958 -80 -> 12 files / +694 -39. --- .../function/AnyValueAggregationFunction.java | 7 -- ...IntegerTupleSketchAggregationFunction.java | 25 +---- ...ictionaryMultiColumnGroupKeyGenerator.java | 68 +++----------- ...ctionarySingleColumnGroupKeyGenerator.java | 91 +++---------------- .../groupby/utils/UuidToIdMap.java | 64 ------------- .../groupby/utils/ValueToIdMapFactory.java | 2 - .../NoDictionaryGroupKeyGeneratorTest.java | 48 ++-------- 7 files changed, 41 insertions(+), 264 deletions(-) delete mode 100644 pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/utils/UuidToIdMap.java diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AnyValueAggregationFunction.java b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AnyValueAggregationFunction.java index 7a3ba85337ca..a649fe6d7a17 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AnyValueAggregationFunction.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AnyValueAggregationFunction.java @@ -327,13 +327,6 @@ private void ensureResultType(BlockValSet bvs) { if (_resultType != null) { return; } - // Inspect the logical type first so a UUID column reports ColumnDataType.UUID (and the broker renders canonical - // RFC-4122 strings) rather than collapsing to BYTES (which would render hex). All other dispatch keys off the - // stored type, matching the BYTES/STRING storage convention. - if (bvs.getValueType() == FieldSpec.DataType.UUID) { - _resultType = ColumnDataType.UUID; - return; - } switch (bvs.getValueType().getStoredType()) { case INT: _resultType = ColumnDataType.INT; diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/IntegerTupleSketchAggregationFunction.java b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/IntegerTupleSketchAggregationFunction.java index b42979201962..83ae908cfa80 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/IntegerTupleSketchAggregationFunction.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/IntegerTupleSketchAggregationFunction.java @@ -151,15 +151,8 @@ public void aggregate(int length, AggregationResultHolder aggregationResultHolde Map blockValSetMap) { BlockValSet blockValSet = blockValSetMap.get(_expression); - FieldSpec.DataType dataType = blockValSet.getValueType(); - FieldSpec.DataType storedType = dataType.getStoredType(); - // UUID columns are stored as BYTES but contain raw 16-byte UUID values, not serialized tuple sketches. - // Surface a clear error rather than letting the deserialize step fail with a confusing sketch-format message. - if (dataType == FieldSpec.DataType.UUID) { - throw new IllegalStateException(getType() + " does not accept raw UUID values; pre-serialize the column as " - + "Integer Tuple Sketches first"); - } // Treat BYTES value as serialized Integer Tuple Sketch + FieldSpec.DataType storedType = blockValSet.getValueType().getStoredType(); if (storedType == FieldSpec.DataType.BYTES) { byte[][] bytesValues = blockValSet.getBytesValuesSV(); try { @@ -182,12 +175,8 @@ public void aggregateGroupBySV(int length, int[] groupKeyArray, GroupByResultHol BlockValSet blockValSet = blockValSetMap.get(_expression); - FieldSpec.DataType dataType = blockValSet.getValueType(); - FieldSpec.DataType storedType = dataType.getStoredType(); - if (dataType == FieldSpec.DataType.UUID) { - throw new IllegalStateException(getType() + " does not accept raw UUID values; pre-serialize the column as " - + "Integer Tuple Sketches first"); - } + // Treat BYTES value as serialized Integer Tuple Sketch + FieldSpec.DataType storedType = blockValSet.getValueType().getStoredType(); if (storedType == FieldSpec.DataType.BYTES) { byte[][] bytesValues = blockValSet.getBytesValuesSV(); @@ -212,12 +201,8 @@ public void aggregateGroupByMV(int length, int[][] groupKeysArray, GroupByResult Map blockValSetMap) { BlockValSet blockValSet = blockValSetMap.get(_expression); - FieldSpec.DataType dataType = blockValSet.getValueType(); - FieldSpec.DataType storedType = dataType.getStoredType(); - if (dataType == FieldSpec.DataType.UUID) { - throw new IllegalStateException(getType() + " does not accept raw UUID values; pre-serialize the column as " - + "Integer Tuple Sketches first"); - } + // Treat BYTES value as serialized Integer Tuple Sketch + FieldSpec.DataType storedType = blockValSet.getValueType().getStoredType(); boolean singleValue = blockValSet.isSingleValue(); if (singleValue && storedType == FieldSpec.DataType.BYTES) { diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionaryMultiColumnGroupKeyGenerator.java b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionaryMultiColumnGroupKeyGenerator.java index f6d3784ebf96..a10de9f47a58 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionaryMultiColumnGroupKeyGenerator.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionaryMultiColumnGroupKeyGenerator.java @@ -36,7 +36,6 @@ import org.apache.pinot.spi.data.FieldSpec.DataType; import org.apache.pinot.spi.utils.ByteArray; import org.apache.pinot.spi.utils.FixedIntArray; -import org.apache.pinot.spi.utils.UuidKey; import org.roaringbitmap.RoaringBitmap; @@ -52,11 +51,7 @@ public class NoDictionaryMultiColumnGroupKeyGenerator implements GroupKeyGenerat private final ExpressionContext[] _groupByExpressions; private final int _numGroupByExpressions; - /// Per-column group-key dispatch type: stored type of each column, except UUID is preserved as - /// [DataType#UUID] so the on-the-fly dictionary keys on [org.apache.pinot.spi.utils.UuidKey] - /// instead of [org.apache.pinot.spi.utils.ByteArray]. For every other logical type this equals - /// `logicalType.getStoredType()`. - private final DataType[] _dataTypes; + private final DataType[] _storedTypes; private final Dictionary[] _dictionaries; private final ValueToIdMap[] _onTheFlyDictionaries; private final Object2IntOpenHashMap _groupKeyMap; @@ -70,7 +65,7 @@ public NoDictionaryMultiColumnGroupKeyGenerator(BaseProjectOperator projectOp Map groupByExpressionSizesFromPredicates) { _groupByExpressions = groupByExpressions; _numGroupByExpressions = groupByExpressions.length; - _dataTypes = new DataType[_numGroupByExpressions]; + _storedTypes = new DataType[_numGroupByExpressions]; _dictionaries = new Dictionary[_numGroupByExpressions]; _onTheFlyDictionaries = new ValueToIdMap[_numGroupByExpressions]; _isSingleValueExpressions = new boolean[_numGroupByExpressions]; @@ -80,9 +75,7 @@ public NoDictionaryMultiColumnGroupKeyGenerator(BaseProjectOperator projectOp for (int i = 0; i < _numGroupByExpressions; i++) { ExpressionContext groupByExpression = groupByExpressions[i]; ColumnContext columnContext = projectOperator.getResultColumnContext(groupByExpression); - DataType logicalType = columnContext.getDataType(); - // Normalize to stored type, but preserve UUID so it uses UuidKey instead of ByteArray. - _dataTypes[i] = logicalType == DataType.UUID ? DataType.UUID : logicalType.getStoredType(); + _storedTypes[i] = columnContext.getDataType().getStoredType(); // Take the dict-id path only when the forward index is dict-encoded. A column with EncodingType.RAW + // dictionaryIndex exposes a Dictionary but BlockValSet#getDictionaryIdsSV throws on its RAW forward // index — fall back to an on-the-fly dictionary on raw values for that case. @@ -91,7 +84,7 @@ public NoDictionaryMultiColumnGroupKeyGenerator(BaseProjectOperator projectOp if (dictionary != null) { _dictionaries[i] = dictionary; } else { - _onTheFlyDictionaries[i] = ValueToIdMapFactory.get(_dataTypes[i]); + _onTheFlyDictionaries[i] = ValueToIdMapFactory.get(_storedTypes[i]); } if (canOptimizeGroupByUpperBound) { Integer size = groupByExpressionSizesFromPredicates.get(groupByExpression); @@ -128,7 +121,7 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[] groupKeys) { if (_dictionaries[i] != null) { values[i] = blockValSet.getDictionaryIdsSV(); } else { - switch (_dataTypes[i]) { + switch (_storedTypes[i]) { case INT: values[i] = blockValSet.getIntValuesSV(); break; @@ -148,11 +141,10 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[] groupKeys) { values[i] = blockValSet.getStringValuesSV(); break; case BYTES: - case UUID: values[i] = blockValSet.getBytesValuesSV(); break; default: - throw new IllegalArgumentException("Illegal data type for no-dictionary key generator: " + _dataTypes[i]); + throw new IllegalArgumentException("Illegal data type for no-dictionary key generator: " + _storedTypes[i]); } } } @@ -184,7 +176,7 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[] groupKeys) { } else if (columnValues instanceof double[]) { keyValue = onTheFlyDictionary.put(((double[]) columnValues)[row]); } else if (columnValues instanceof byte[][]) { - keyValue = putBytesValue(onTheFlyDictionary, (byte[][]) columnValues, row, _dataTypes[col]); + keyValue = onTheFlyDictionary.put(new ByteArray(((byte[][]) columnValues)[row])); } else { keyValue = onTheFlyDictionary.put(((Object[]) columnValues)[row]); } @@ -208,7 +200,7 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[] groupKeys) { } else if (columnValues instanceof double[]) { keyValue = onTheFlyDictionary.getId(((double[]) columnValues)[row]); } else if (columnValues instanceof byte[][]) { - keyValue = getBytesValueId(onTheFlyDictionary, (byte[][]) columnValues, row, _dataTypes[col]); + keyValue = onTheFlyDictionary.getId(new ByteArray(((byte[][]) columnValues)[row])); } else { keyValue = onTheFlyDictionary.getId(((Object[]) columnValues)[row]); } @@ -250,7 +242,7 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[] groupKeys) { } else if (columnValues instanceof double[]) { keyValue = onTheFlyDictionary.put(((double[]) columnValues)[row]); } else if (columnValues instanceof byte[][]) { - keyValue = putBytesValue(onTheFlyDictionary, (byte[][]) columnValues, row, _dataTypes[col]); + keyValue = onTheFlyDictionary.put(new ByteArray(((byte[][]) columnValues)[row])); } else { keyValue = onTheFlyDictionary.put(((Object[]) columnValues)[row]); } @@ -271,7 +263,7 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[] groupKeys) { } else if (columnValues instanceof double[]) { keyValue = onTheFlyDictionary.getId(((double[]) columnValues)[row]); } else if (columnValues instanceof byte[][]) { - keyValue = getBytesValueId(onTheFlyDictionary, (byte[][]) columnValues, row, _dataTypes[col]); + keyValue = onTheFlyDictionary.getId(new ByteArray(((byte[][]) columnValues)[row])); } else { keyValue = onTheFlyDictionary.getId(((Object[]) columnValues)[row]); } @@ -318,7 +310,7 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[][] groupKeys) { } else { ValueToIdMap onTheFlyDictionary = _onTheFlyDictionaries[i]; if (_isSingleValueExpressions[i]) { - switch (_dataTypes[i]) { + switch (_storedTypes[i]) { case INT: int[] intValues = blockValSet.getIntValuesSV(); for (int j = 0; j < numDocs; j++) { @@ -349,12 +341,6 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[][] groupKeys) { keys[j][i] = new int[]{onTheFlyDictionary.put(stringValues[j])}; } break; - case UUID: - byte[][] uuidValues = blockValSet.getBytesValuesSV(); - for (int j = 0; j < numDocs; j++) { - keys[j][i] = new int[]{onTheFlyDictionary.put(UuidKey.fromBytes(uuidValues[j]))}; - } - break; case BYTES: byte[][] bytesValues = blockValSet.getBytesValuesSV(); for (int j = 0; j < numDocs; j++) { @@ -363,10 +349,10 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[][] groupKeys) { break; default: throw new IllegalArgumentException( - "Illegal data type for no-dictionary key generator: " + _dataTypes[i]); + "Illegal data type for no-dictionary key generator: " + _storedTypes[i]); } } else { - switch (_dataTypes[i]) { + switch (_storedTypes[i]) { case INT: int[][] intValues = blockValSet.getIntValuesMV(); for (int j = 0; j < numDocs; j++) { @@ -422,20 +408,9 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[][] groupKeys) { keys[j][i] = mvKeys; } break; - case UUID: - byte[][][] uuidValues = blockValSet.getBytesValuesMV(); - for (int j = 0; j < numDocs; j++) { - int mvSize = uuidValues[j].length; - int[] mvKeys = new int[mvSize]; - for (int k = 0; k < mvSize; k++) { - mvKeys[k] = onTheFlyDictionary.put(UuidKey.fromBytes(uuidValues[j][k])); - } - keys[j][i] = mvKeys; - } - break; default: throw new IllegalArgumentException( - "Illegal data type for no-dictionary key generator: " + _dataTypes[i]); + "Illegal data type for no-dictionary key generator: " + _storedTypes[i]); } } } @@ -541,19 +516,4 @@ private Object[] buildKeysFromIds(FixedIntArray keyList) { } return keys; } - - private static int putBytesValue(ValueToIdMap onTheFlyDictionary, byte[][] columnValues, int row, DataType dataType) { - if (dataType == DataType.UUID) { - return onTheFlyDictionary.put(UuidKey.fromBytes(columnValues[row])); - } - return onTheFlyDictionary.put(new ByteArray(columnValues[row])); - } - - private static int getBytesValueId(ValueToIdMap onTheFlyDictionary, byte[][] columnValues, int row, - DataType dataType) { - if (dataType == DataType.UUID) { - return onTheFlyDictionary.getId(UuidKey.fromBytes(columnValues[row])); - } - return onTheFlyDictionary.getId(new ByteArray(columnValues[row])); - } } diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionarySingleColumnGroupKeyGenerator.java b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionarySingleColumnGroupKeyGenerator.java index a0cc335c3d93..d6b0bfcce935 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionarySingleColumnGroupKeyGenerator.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionarySingleColumnGroupKeyGenerator.java @@ -41,7 +41,6 @@ import org.apache.pinot.core.operator.blocks.ValueBlock; import org.apache.pinot.spi.data.FieldSpec.DataType; import org.apache.pinot.spi.utils.ByteArray; -import org.apache.pinot.spi.utils.UuidKey; import org.roaringbitmap.RoaringBitmap; @@ -50,11 +49,7 @@ @SuppressWarnings({"rawtypes", "unchecked"}) public class NoDictionarySingleColumnGroupKeyGenerator implements GroupKeyGenerator { private final ExpressionContext _groupByExpression; - /// Group-key dispatch type: stored type of the column, except UUID is preserved as [DataType#UUID] so the - /// group-key map keys on [org.apache.pinot.spi.utils.UuidKey] (two primitive longs) instead of - /// [org.apache.pinot.spi.utils.ByteArray]. For every other logical type this equals - /// `logicalType.getStoredType()`. - private final DataType _dataType; + private final DataType _storedType; private final Map _groupKeyMap; private final int _globalGroupIdUpperBound; // TODO(nhejazi): Most of the logic between _nullHandlingEnabled=true/false is not sharable, so consider making a @@ -71,10 +66,8 @@ public NoDictionarySingleColumnGroupKeyGenerator(BaseProjectOperator projectO @Nullable Map groupByExpressionSizesFromPredicates) { _groupByExpression = groupByExpression; ColumnContext columnContext = projectOperator.getResultColumnContext(groupByExpression); - DataType logicalType = columnContext.getDataType(); - // Normalize to stored type, but preserve UUID so it uses UuidKey instead of ByteArray - _dataType = logicalType == DataType.UUID ? DataType.UUID : logicalType.getStoredType(); - _groupKeyMap = createGroupKeyMap(_dataType); + _storedType = columnContext.getDataType().getStoredType(); + _groupKeyMap = createGroupKeyMap(_storedType); if (groupByExpressionSizesFromPredicates != null) { Integer size = groupByExpressionSizesFromPredicates.get(groupByExpression); _globalGroupIdUpperBound = size != null ? Math.min(size, numGroupsLimit) : numGroupsLimit; @@ -102,7 +95,7 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[] groupKeys) { } int numDocs = valueBlock.getNumDocs(); - switch (_dataType) { + switch (_storedType) { case INT: int[] intValues = blockValSet.getIntValuesSV(); for (int i = 0; i < numDocs; i++) { @@ -139,12 +132,6 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[] groupKeys) { groupKeys[i] = getKeyForValue(stringValues[i]); } break; - case UUID: - byte[][] uuidValues = blockValSet.getBytesValuesSV(); - for (int i = 0; i < numDocs; i++) { - groupKeys[i] = getKeyForValue(UuidKey.fromBytes(uuidValues[i])); - } - break; case BYTES: byte[][] bytesValues = blockValSet.getBytesValuesSV(); for (int i = 0; i < numDocs; i++) { @@ -152,7 +139,7 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[] groupKeys) { } break; default: - throw new IllegalArgumentException("Illegal data type for no-dictionary key generator: " + _dataType); + throw new IllegalArgumentException("Illegal data type for no-dictionary key generator: " + _storedType); } } @@ -162,7 +149,7 @@ public void generateKeysForBlockNullHandlingEnabled(ValueBlock valueBlock, int[] BlockValSet blockValSet = valueBlock.getBlockValueSet(_groupByExpression); int numDocs = valueBlock.getNumDocs(); - switch (_dataType) { + switch (_storedType) { case INT: int[] intValues = blockValSet.getIntValuesSV(); if (nullBitmap.getCardinality() < numDocs) { @@ -223,16 +210,6 @@ public void generateKeysForBlockNullHandlingEnabled(ValueBlock valueBlock, int[] Arrays.fill(groupKeys, 0, numDocs, getKeyForValue((String) null)); } break; - case UUID: - byte[][] uuidValues = blockValSet.getBytesValuesSV(); - if (nullBitmap.getCardinality() < numDocs) { - for (int i = 0; i < numDocs; i++) { - groupKeys[i] = getKeyForValue(nullBitmap.contains(i) ? null : UuidKey.fromBytes(uuidValues[i])); - } - } else if (numDocs > 0) { - Arrays.fill(groupKeys, 0, numDocs, getKeyForValue((UuidKey) null)); - } - break; case BYTES: byte[][] bytesValues = blockValSet.getBytesValuesSV(); if (nullBitmap.getCardinality() < numDocs) { @@ -244,7 +221,7 @@ public void generateKeysForBlockNullHandlingEnabled(ValueBlock valueBlock, int[] } break; default: - throw new IllegalArgumentException("Illegal data type for no-dictionary key generator: " + _dataType); + throw new IllegalArgumentException("Illegal data type for no-dictionary key generator: " + _storedType); } } @@ -279,10 +256,6 @@ private Map createGroupKeyMap(DataType keyType) { Object2IntOpenHashMap stringMap = new Object2IntOpenHashMap<>(); stringMap.defaultReturnValue(INVALID_ID); return stringMap; - case UUID: - Object2IntOpenHashMap uuidMap = new Object2IntOpenHashMap<>(); - uuidMap.defaultReturnValue(INVALID_ID); - return uuidMap; case BYTES: Object2IntOpenHashMap bytesMap = new Object2IntOpenHashMap<>(); bytesMap.defaultReturnValue(INVALID_ID); @@ -298,7 +271,7 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[][] groupKeys) { BlockValSet blockValSet = valueBlock.getBlockValueSet(_groupByExpression); if (_isSingleValueExpression) { - switch (_dataType) { + switch (_storedType) { case INT: int[] intValues = blockValSet.getIntValuesSV(); for (int i = 0; i < numDocs; i++) { @@ -329,12 +302,6 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[][] groupKeys) { groupKeys[i] = new int[]{getKeyForValue(stringValues[i])}; } break; - case UUID: - byte[][] uuidValues = blockValSet.getBytesValuesSV(); - for (int i = 0; i < numDocs; i++) { - groupKeys[i] = new int[]{getKeyForValue(UuidKey.fromBytes(uuidValues[i]))}; - } - break; case BYTES: byte[][] byteValues = blockValSet.getBytesValuesSV(); for (int i = 0; i < numDocs; i++) { @@ -342,10 +309,10 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[][] groupKeys) { } break; default: - throw new IllegalArgumentException("Illegal data type for no-dictionary key generator: " + _dataType); + throw new IllegalArgumentException("Illegal data type for no-dictionary key generator: " + _storedType); } } else { - switch (_dataType) { + switch (_storedType) { case INT: int[][] intValues = blockValSet.getIntValuesMV(); for (int i = 0; i < numDocs; i++) { @@ -401,19 +368,8 @@ public void generateKeysForBlock(ValueBlock valueBlock, int[][] groupKeys) { groupKeys[i] = mvKeys; } break; - case UUID: - byte[][][] uuidValues = blockValSet.getBytesValuesMV(); - for (int i = 0; i < numDocs; i++) { - int mvSize = uuidValues[i].length; - int[] mvKeys = new int[mvSize]; - for (int j = 0; j < mvSize; j++) { - mvKeys[j] = getKeyForValue(UuidKey.fromBytes(uuidValues[i][j])); - } - groupKeys[i] = mvKeys; - } - break; default: - throw new IllegalArgumentException("Illegal data type for no-dictionary key generator: " + _dataType); + throw new IllegalArgumentException("Illegal data type for no-dictionary key generator: " + _storedType); } } } @@ -425,7 +381,7 @@ public int getCurrentGroupKeyUpperBound() { @Override public Iterator getGroupKeys() { - switch (_dataType) { + switch (_storedType) { case INT: return new IntGroupKeyIterator((Int2IntOpenHashMap) _groupKeyMap, _groupIdForNullValue); case LONG: @@ -437,8 +393,7 @@ public Iterator getGroupKeys() { case BIG_DECIMAL: case STRING: case BYTES: - case UUID: - return new ObjectGroupKeyIterator((Object2IntOpenHashMap) _groupKeyMap, _dataType); + return new ObjectGroupKeyIterator((Object2IntOpenHashMap) _groupKeyMap); default: throw new IllegalStateException(); } @@ -530,16 +485,6 @@ private int getKeyForValue(ByteArray value) { return groupId; } - private int getKeyForValue(UuidKey value) { - Object2IntMap map = (Object2IntMap) _groupKeyMap; - int groupId = map.getInt(value); - if (groupId == INVALID_ID && _numGroups < _globalGroupIdUpperBound) { - groupId = _numGroups++; - map.put(value, groupId); - } - return groupId; - } - private static class IntGroupKeyIterator implements Iterator { final Iterator _iterator; final GroupKey _groupKey; @@ -688,12 +633,10 @@ public void remove() { private static class ObjectGroupKeyIterator implements Iterator { final ObjectIterator _iterator; final GroupKey _groupKey; - final DataType _dataType; - ObjectGroupKeyIterator(Object2IntOpenHashMap objectMap, DataType dataType) { + ObjectGroupKeyIterator(Object2IntOpenHashMap objectMap) { _iterator = objectMap.object2IntEntrySet().fastIterator(); _groupKey = new GroupKey(); - _dataType = dataType; } @Override @@ -705,11 +648,7 @@ public boolean hasNext() { public GroupKey next() { Object2IntMap.Entry entry = _iterator.next(); _groupKey._groupId = entry.getIntValue(); - Object key = entry.getKey(); - if (_dataType == DataType.UUID && key != null) { - key = ((UuidKey) key).toByteArray(); - } - _groupKey._keys = new Object[]{key}; + _groupKey._keys = new Object[]{entry.getKey()}; return _groupKey; } diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/utils/UuidToIdMap.java b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/utils/UuidToIdMap.java deleted file mode 100644 index 0ee338e97013..000000000000 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/utils/UuidToIdMap.java +++ /dev/null @@ -1,64 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.pinot.core.query.aggregation.groupby.utils; - -import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap; -import java.util.ArrayList; -import org.apache.pinot.spi.utils.ByteArray; -import org.apache.pinot.spi.utils.UuidKey; - - -/// Implementation of [ValueToIdMap] for Pinot's logical UUID type. -public class UuidToIdMap implements ValueToIdMap { - private final Object2IntOpenHashMap _valueToIdMap; - private final ArrayList _idToValueMap; - - public UuidToIdMap() { - _valueToIdMap = new Object2IntOpenHashMap<>(); - _valueToIdMap.defaultReturnValue(INVALID_KEY); - _idToValueMap = new ArrayList<>(); - } - - /// Both callers -- [org.apache.pinot.core.query.aggregation.groupby.NoDictionaryMultiColumnGroupKeyGenerator] and - /// [org.apache.pinot.core.query.aggregation.groupby.NoDictionarySingleColumnGroupKeyGenerator] -- key on - /// [UuidKey] already, so this casts directly rather than going through `UuidKey#fromObject`. That matches the - /// sibling maps (e.g. [DoubleToIdMap] casts to `double`) and keeps the per-row `instanceof` chain out of the - /// group-by loop. - @Override - public int put(Object value) { - UuidKey uuidKey = (UuidKey) value; - int id = _valueToIdMap.getInt(uuidKey); - if (id == INVALID_KEY) { - id = _valueToIdMap.size(); - _valueToIdMap.put(uuidKey, id); - _idToValueMap.add(uuidKey.toByteArray()); - } - return id; - } - - @Override - public int getId(Object value) { - return _valueToIdMap.getInt((UuidKey) value); - } - - @Override - public Object get(int id) { - return _idToValueMap.get(id); - } -} diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/utils/ValueToIdMapFactory.java b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/utils/ValueToIdMapFactory.java index a15f9bf9a263..4ce47caa196b 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/utils/ValueToIdMapFactory.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/utils/ValueToIdMapFactory.java @@ -36,8 +36,6 @@ public static ValueToIdMap get(DataType dataType) { return new FloatToIdMap(); case DOUBLE: return new DoubleToIdMap(); - case UUID: - return new UuidToIdMap(); default: return new ObjectToIdMap(); } diff --git a/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionaryGroupKeyGeneratorTest.java b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionaryGroupKeyGeneratorTest.java index 2192848feb3d..2ed7bff83f1b 100644 --- a/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionaryGroupKeyGeneratorTest.java +++ b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionaryGroupKeyGeneratorTest.java @@ -27,7 +27,6 @@ import java.util.List; import java.util.Random; import java.util.Set; -import java.util.UUID; import org.apache.commons.io.FileUtils; import org.apache.commons.lang3.RandomStringUtils; import org.apache.commons.lang3.StringUtils; @@ -52,7 +51,6 @@ import org.apache.pinot.spi.utils.BytesUtils; import org.apache.pinot.spi.utils.CommonConstants.Server; import org.apache.pinot.spi.utils.ReadMode; -import org.apache.pinot.spi.utils.UuidUtils; import org.apache.pinot.spi.utils.builder.TableConfigBuilder; import org.testng.annotations.AfterClass; import org.testng.annotations.BeforeClass; @@ -77,19 +75,12 @@ public class NoDictionaryGroupKeyGeneratorTest { private static final String STRING_COLUMN = "stringColumn"; private static final String BYTES_COLUMN = "bytesColumn"; private static final String BYTES_DICT_COLUMN = "bytesDictColumn"; - private static final String UUID_COLUMN = "uuidColumn"; - private static final String BOOLEAN_COLUMN = "booleanColumn"; - private static final String TIMESTAMP_COLUMN = "timestampColumn"; - private static final String UUID_DICT_COLUMN = "uuidDictColumn"; private static final List COLUMNS = Arrays.asList(INT_COLUMN, LONG_COLUMN, FLOAT_COLUMN, DOUBLE_COLUMN, STRING_COLUMN, BYTES_COLUMN, - BYTES_DICT_COLUMN, UUID_COLUMN, BOOLEAN_COLUMN, TIMESTAMP_COLUMN, UUID_DICT_COLUMN); + BYTES_DICT_COLUMN); private static final int NUM_COLUMNS = COLUMNS.size(); - private static final Set UUID_COLUMNS = Set.of(UUID_COLUMN, UUID_DICT_COLUMN); private static final TableConfig TABLE_CONFIG = new TableConfigBuilder(TableType.OFFLINE).setTableName(RAW_TABLE_NAME) - .setNoDictionaryColumns( - List.of(INT_COLUMN, LONG_COLUMN, FLOAT_COLUMN, DOUBLE_COLUMN, STRING_COLUMN, BYTES_COLUMN, UUID_COLUMN, - BOOLEAN_COLUMN, TIMESTAMP_COLUMN)).build(); + .setNoDictionaryColumns(COLUMNS.subList(0, NUM_COLUMNS - 1)).build(); private static final Schema SCHEMA = new Schema.SchemaBuilder().addSingleValueDimension(INT_COLUMN, FieldSpec.DataType.INT) .addSingleValueDimension(LONG_COLUMN, FieldSpec.DataType.LONG) @@ -97,11 +88,7 @@ public class NoDictionaryGroupKeyGeneratorTest { .addSingleValueDimension(DOUBLE_COLUMN, FieldSpec.DataType.DOUBLE) .addSingleValueDimension(STRING_COLUMN, FieldSpec.DataType.STRING) .addSingleValueDimension(BYTES_COLUMN, FieldSpec.DataType.BYTES) - .addSingleValueDimension(BYTES_DICT_COLUMN, FieldSpec.DataType.BYTES) - .addSingleValueDimension(UUID_COLUMN, FieldSpec.DataType.UUID) - .addSingleValueDimension(BOOLEAN_COLUMN, FieldSpec.DataType.BOOLEAN) - .addSingleValueDimension(TIMESTAMP_COLUMN, FieldSpec.DataType.TIMESTAMP) - .addSingleValueDimension(UUID_DICT_COLUMN, FieldSpec.DataType.UUID).build(); + .addSingleValueDimension(BYTES_DICT_COLUMN, FieldSpec.DataType.BYTES).build(); private static final int NUM_RECORDS = 1000; private static final int NUM_UNIQUE_RECORDS = 100; @@ -142,19 +129,6 @@ public void setUp() record.putValue(BYTES_DICT_COLUMN, bytesValue); values[5] = BytesUtils.toHexString(bytesValue); values[6] = values[5]; - byte[] uuidBytes = UuidUtils.toBytes(new UUID(RANDOM.nextLong(), RANDOM.nextLong())); - record.putValue(UUID_COLUMN, uuidBytes); - values[7] = UuidUtils.toString(uuidBytes); - // BOOLEAN stored as INT (0/1) — exercises the logical→stored-type normalization fix - int boolIntValue = RANDOM.nextBoolean() ? 1 : 0; - record.putValue(BOOLEAN_COLUMN, boolIntValue); - values[8] = Integer.toString(boolIntValue); - // TIMESTAMP stored as LONG — exercises the logical→stored-type normalization fix - long timestampValue = Math.abs(RANDOM.nextLong()); - record.putValue(TIMESTAMP_COLUMN, timestampValue); - values[9] = Long.toString(timestampValue); - record.putValue(UUID_DICT_COLUMN, uuidBytes); - values[10] = values[7]; for (int j = 0; j < NUM_RECORDS / NUM_UNIQUE_RECORDS; j++) { records.add(record); } @@ -199,12 +173,9 @@ public void testMultiColumnGroupKeyGenerator() { testGroupKeyGenerator(new int[]{0, 1}); testGroupKeyGenerator(new int[]{2, 3}); testGroupKeyGenerator(new int[]{4, 5}); - testGroupKeyGenerator(new int[]{7, 10}); - testGroupKeyGenerator(new int[]{8, 9}); testGroupKeyGenerator(new int[]{1, 2, 3}); testGroupKeyGenerator(new int[]{4, 5, 0}); - testGroupKeyGenerator(new int[]{7, 5, 4}); - testGroupKeyGenerator(new int[]{7, 5, 4, 3, 2, 1, 0}); + testGroupKeyGenerator(new int[]{5, 4, 3, 2, 1, 0}); } /// Tests multi-column group key generator when at least one column as dictionary, and others don't. @@ -241,7 +212,7 @@ private void testGroupKeyGenerator(int[] groupByColumnIndexes) { Iterator groupKeys = groupKeyGenerator.getGroupKeys(); while (groupKeys.hasNext()) { GroupKeyGenerator.GroupKey groupKey = groupKeys.next(); - assertTrue(expectedGroupKeys.contains(getActualGroupKey(groupKey._keys, groupByColumnIndexes))); + assertTrue(expectedGroupKeys.contains(getActualGroupKey(groupKey._keys))); } } @@ -263,18 +234,13 @@ private Set getExpectedGroupKeys(int[] groupByColumnIndexes) { return groupKeys; } - private String getActualGroupKey(Object[] groupKeys, int[] groupByColumnIndexes) { + private String getActualGroupKey(Object[] groupKeys) { StringBuilder stringBuilder = new StringBuilder(); for (int i = 0; i < groupKeys.length; i++) { if (i > 0) { stringBuilder.append(GroupKeyGenerator.DELIMITER); } - int columnIndex = groupByColumnIndexes[i]; - if (UUID_COLUMNS.contains(COLUMNS.get(columnIndex))) { - stringBuilder.append(UuidUtils.toString(((org.apache.pinot.spi.utils.ByteArray) groupKeys[i]).getBytes())); - } else { - stringBuilder.append(groupKeys[i]); - } + stringBuilder.append(groupKeys[i]); } return stringBuilder.toString(); } From 0e665739c62696a0bd3a5d6b29500508d70af419 Mon Sep 17 00:00:00 2001 From: Xiang Fu Date: Sun, 9 Aug 2026 13:09:33 -0700 Subject: [PATCH 6/8] Format DISTINCT rows in one pass in BytesDistinctTable The previous version added every row holding a ByteArray, then walked the whole list again in formatRows() to rewrite row[0]. Two traversals and two writes per row where master did one. The formatting now happens inline in addRows, with the ColumnDataType passed in -- the same shape LongDistinctTable already uses for its TIMESTAMP handling. Not an extra String allocation either way: ColumnDataType#convertAndFormat for BYTES is ((ByteArray) value).toHexString(), the exact call master made, and ByteArray#getBytes() returns the internal array without copying. --- .../distinct/table/BytesDistinctTable.java | 37 +++++++------------ 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/distinct/table/BytesDistinctTable.java b/pinot-core/src/main/java/org/apache/pinot/core/query/distinct/table/BytesDistinctTable.java index 42649c9bd28a..f41794fb7420 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/distinct/table/BytesDistinctTable.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/distinct/table/BytesDistinctTable.java @@ -32,6 +32,7 @@ import org.apache.pinot.common.request.context.OrderByExpressionContext; import org.apache.pinot.common.response.broker.ResultTable; import org.apache.pinot.common.utils.DataSchema; +import org.apache.pinot.common.utils.DataSchema.ColumnDataType; import org.apache.pinot.core.common.datatable.DataTableBuilder; import org.apache.pinot.core.common.datatable.DataTableBuilderFactory; import org.apache.pinot.spi.query.QueryThreadContext; @@ -263,69 +264,59 @@ private ResultTable toResultTableWithOrderBy() { } int numValues = sortedValues.length; assert numValues <= _limit; + ColumnDataType columnDataType = _dataSchema.getColumnDataType(0); List rows; if (_hasNull) { if (numValues == _limit) { rows = new ArrayList<>(_limit); if (_orderByExpression.isNullsLast()) { - addRows(sortedValues, numValues, rows); + addRows(columnDataType, sortedValues, numValues, rows); } else { rows.add(new Object[]{null}); - addRows(sortedValues, numValues - 1, rows); + addRows(columnDataType, sortedValues, numValues - 1, rows); } } else { rows = new ArrayList<>(numValues + 1); if (_orderByExpression.isNullsLast()) { - addRows(sortedValues, numValues, rows); + addRows(columnDataType, sortedValues, numValues, rows); rows.add(new Object[]{null}); } else { rows.add(new Object[]{null}); - addRows(sortedValues, numValues, rows); + addRows(columnDataType, sortedValues, numValues, rows); } } } else { rows = new ArrayList<>(numValues); - addRows(sortedValues, numValues, rows); + addRows(columnDataType, sortedValues, numValues, rows); } - formatRows(rows); return new ResultTable(_dataSchema, rows); } - private static void addRows(ByteArray[] values, int length, List rows) { + private static void addRows(ColumnDataType columnDataType, ByteArray[] values, int length, List rows) { for (int i = 0; i < length; i++) { - rows.add(new Object[]{values[i]}); + rows.add(new Object[]{columnDataType.convertAndFormat(values[i])}); } } private ResultTable toResultTableWithoutOrderBy() { int numValues = _valueSet.size(); assert numValues <= _limit; + ColumnDataType columnDataType = _dataSchema.getColumnDataType(0); List rows; if (_hasNull && numValues < _limit) { rows = new ArrayList<>(numValues + 1); - addRows(_valueSet, rows); + addRows(columnDataType, _valueSet, rows); rows.add(new Object[]{null}); } else { rows = new ArrayList<>(numValues); - addRows(_valueSet, rows); + addRows(columnDataType, _valueSet, rows); } - formatRows(rows); return new ResultTable(_dataSchema, rows); } - private static void addRows(HashSet values, List rows) { + private static void addRows(ColumnDataType columnDataType, HashSet values, List rows) { for (ByteArray value : values) { - rows.add(new Object[]{value}); - } - } - - private void formatRows(List rows) { - DataSchema.ColumnDataType columnDataType = _dataSchema.getColumnDataType(0); - for (Object[] row : rows) { - Object value = row[0]; - if (value != null) { - row[0] = columnDataType.convertAndFormat(value); - } + rows.add(new Object[]{columnDataType.convertAndFormat(value)}); } } } From a7d78920ecabdfa2100931b9ef71f4f8e3087bb9 Mon Sep 17 00:00:00 2001 From: Xiang Fu Date: Wed, 12 Aug 2026 17:57:26 -0700 Subject: [PATCH 7/8] Route UUID through the normal dispatch in DistinctCountBitmap Replaces the three `if (dataType == DataType.UUID)` early-returns with a narrowed guard on the serialized-bitmap branch: if (storedType == DataType.BYTES && dataType != DataType.UUID) UUID now falls through to aggregateSV/aggregateMV and the group-by variants like any other type, handled by a `case BYTES` in each switch. That fixes MV UUID, which the early-returns silently broke -- they called getBytesValuesSV() above the isSingleValue() dispatch, so an MV column took the SV accessor. Seven switches needed the case, not six: the dictionary path in convertToValueBitmap also throws on BYTES, which the integration test caught -- DISTINCTCOUNTBITMAP(uuidColumn) failed with "Illegal data type for DISTINCT_COUNT_BITMAP aggregation function: BYTES" until it was added. DistinctCountHLL, DistinctCountHLLPlus, DistinctCountCPCSketch and DistinctCountULL still use the early-return shape and need the same treatment. --- ...istinctCountBitmapAggregationFunction.java | 99 ++++++++++++------- 1 file changed, 62 insertions(+), 37 deletions(-) diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountBitmapAggregationFunction.java b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountBitmapAggregationFunction.java index 03c10883f36a..20cbc7999e68 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountBitmapAggregationFunction.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountBitmapAggregationFunction.java @@ -75,20 +75,8 @@ public void aggregate(int length, AggregationResultHolder aggregationResultHolde DataType dataType = blockValSet.getValueType(); DataType storedType = dataType.getStoredType(); - // UUID values are logical scalars (stored as 16-byte BYTES) — not serialized RoaringBitmap state. Add the - // hashCode of the canonical UUID string so DISTINCTCOUNTBITMAP(uuidCol) matches - // DISTINCTCOUNTBITMAP(CAST(uuidCol AS STRING)). - if (dataType == DataType.UUID) { - byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); - RoaringBitmap bitmap = getValueBitmap(aggregationResultHolder); - for (int i = 0; i < length; i++) { - bitmap.add(Arrays.hashCode(uuidBytesValues[i])); - } - return; - } - // Treat BYTES value as serialized RoaringBitmap - if (storedType == DataType.BYTES) { + if (storedType == DataType.BYTES && dataType != DataType.UUID) { byte[][] bytesValues = blockValSet.getBytesValuesSV(); RoaringBitmap valueBitmap = aggregationResultHolder.getResult(); if (valueBitmap != null) { @@ -153,6 +141,13 @@ protected void aggregateSV(int length, AggregationResultHolder aggregationResult valueBitmap.add(stringValues[i].hashCode()); } break; + // Reached only by UUID: a real BYTES column is serialized sketch state and is handled above. + case BYTES: + byte[][] uuidValues = blockValSet.getBytesValuesSV(); + for (int i = 0; i < length; i++) { + valueBitmap.add(Arrays.hashCode(uuidValues[i])); + } + break; default: throw new IllegalStateException( "Illegal data type for DISTINCT_COUNT_BITMAP aggregation function: " + storedType); @@ -213,6 +208,15 @@ protected void aggregateMV(int length, AggregationResultHolder aggregationResult } } break; + // Reached only by UUID: a real BYTES column is serialized sketch state and is handled above. + case BYTES: + byte[][][] uuidValues = blockValSet.getBytesValuesMV(); + for (int i = 0; i < length; i++) { + for (byte[] value : uuidValues[i]) { + valueBitmap.add(Arrays.hashCode(value)); + } + } + break; default: throw new IllegalStateException( "Illegal data type for DISTINCT_COUNT_BITMAP aggregation function: " + storedType); @@ -227,18 +231,8 @@ public void aggregateGroupBySV(int length, int[] groupKeyArray, GroupByResultHol DataType dataType = blockValSet.getValueType(); DataType storedType = dataType.getStoredType(); - // UUID columns: add hashCode of canonical UUID string (see aggregate() for rationale). - if (dataType == DataType.UUID) { - byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); - for (int i = 0; i < length; i++) { - getValueBitmap(groupByResultHolder, groupKeyArray[i]) - .add(Arrays.hashCode(uuidBytesValues[i])); - } - return; - } - // Treat BYTES value as serialized RoaringBitmap - if (storedType == DataType.BYTES) { + if (storedType == DataType.BYTES && dataType != DataType.UUID) { byte[][] bytesValues = blockValSet.getBytesValuesSV(); for (int i = 0; i < length; i++) { RoaringBitmap value = RoaringBitmapUtils.deserialize(bytesValues[i]); @@ -304,6 +298,13 @@ protected void aggregateSVGroupBySV(int length, int[] groupKeyArray, GroupByResu getValueBitmap(groupByResultHolder, groupKeyArray[i]).add(stringValues[i].hashCode()); } break; + // Reached only by UUID: a real BYTES column is serialized sketch state and is handled above. + case BYTES: + byte[][] uuidValues = blockValSet.getBytesValuesSV(); + for (int i = 0; i < length; i++) { + getValueBitmap(groupByResultHolder, groupKeyArray[i]).add(Arrays.hashCode(uuidValues[i])); + } + break; default: throw new IllegalStateException( "Illegal data type for DISTINCT_COUNT_BITMAP aggregation function: " + storedType); @@ -366,6 +367,16 @@ protected void aggregateMVGroupBySV(int length, int[] groupKeyArray, GroupByResu } } break; + // Reached only by UUID: a real BYTES column is serialized sketch state and is handled above. + case BYTES: + byte[][][] uuidValues = blockValSet.getBytesValuesMV(); + for (int i = 0; i < length; i++) { + RoaringBitmap bitmap = getValueBitmap(groupByResultHolder, groupKeyArray[i]); + for (byte[] value : uuidValues[i]) { + bitmap.add(Arrays.hashCode(value)); + } + } + break; default: throw new IllegalStateException( "Illegal data type for DISTINCT_COUNT_BITMAP aggregation function: " + storedType); @@ -380,20 +391,8 @@ public void aggregateGroupByMV(int length, int[][] groupKeysArray, GroupByResult DataType dataType = blockValSet.getValueType(); DataType storedType = dataType.getStoredType(); - // UUID columns: add hashCode of canonical UUID string (see aggregate() for rationale). - if (dataType == DataType.UUID) { - byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); - for (int i = 0; i < length; i++) { - int hash = Arrays.hashCode(uuidBytesValues[i]); - for (int groupKey : groupKeysArray[i]) { - getValueBitmap(groupByResultHolder, groupKey).add(hash); - } - } - return; - } - // Treat BYTES value as serialized RoaringBitmap - if (storedType == DataType.BYTES) { + if (storedType == DataType.BYTES && dataType != DataType.UUID) { byte[][] bytesValues = blockValSet.getBytesValuesSV(); for (int i = 0; i < length; i++) { RoaringBitmap value = RoaringBitmapUtils.deserialize(bytesValues[i]); @@ -461,6 +460,13 @@ protected void aggregateSVGroupByMV(int length, int[][] groupKeysArray, GroupByR setValueForGroupKeys(groupByResultHolder, groupKeysArray[i], stringValues[i].hashCode()); } break; + // Reached only by UUID: a real BYTES column is serialized sketch state and is handled above. + case BYTES: + byte[][] uuidValues = blockValSet.getBytesValuesSV(); + for (int i = 0; i < length; i++) { + setValueForGroupKeys(groupByResultHolder, groupKeysArray[i], Arrays.hashCode(uuidValues[i])); + } + break; default: throw new IllegalStateException( "Illegal data type for DISTINCT_COUNT_BITMAP aggregation function: " + storedType); @@ -535,6 +541,18 @@ protected void aggregateMVGroupByMV(int length, int[][] groupKeysArray, GroupByR } } break; + // Reached only by UUID: a real BYTES column is serialized sketch state and is handled above. + case BYTES: + byte[][][] uuidValues = blockValSet.getBytesValuesMV(); + for (int i = 0; i < length; i++) { + for (int groupKey : groupKeysArray[i]) { + RoaringBitmap bitmap = getValueBitmap(groupByResultHolder, groupKey); + for (byte[] value : uuidValues[i]) { + bitmap.add(Arrays.hashCode(value)); + } + } + } + break; default: throw new IllegalStateException( "Illegal data type for DISTINCT_COUNT_BITMAP aggregation function: " + storedType); @@ -701,6 +719,13 @@ private static RoaringBitmap convertToValueBitmap(DictIdsWrapper dictIdsWrapper) valueBitmap.add(dictionary.getStringValue(iterator.next()).hashCode()); } break; + // A UUID column's dictionary reports BYTES (it is a plain BytesDictionary); hash the stored bytes to match + // the scan path above. A real BYTES column holds serialized bitmaps and never reaches the dictionary path. + case BYTES: + while (iterator.hasNext()) { + valueBitmap.add(Arrays.hashCode(dictionary.getBytesValue(iterator.next()))); + } + break; default: throw new IllegalStateException( "Illegal data type for DISTINCT_COUNT_BITMAP aggregation function: " + storedType); From b50e1eeecb0f26486d29ca7ab6f2acbb9e065c34 Mon Sep 17 00:00:00 2001 From: Xiang Fu Date: Wed, 12 Aug 2026 18:14:56 -0700 Subject: [PATCH 8/8] Route UUID through the normal dispatch in DistinctCountHLL Same shape as DistinctCountBitmap: the three `if (dataType == DataType.UUID)` early-returns are gone, the serialized-HLL guard is narrowed to if (storedType == DataType.BYTES && dataType != DataType.UUID) and UUID falls through to aggregateSV/aggregateMV and the group-by variants, handled by a `case BYTES` in each of the six switches. This fixes MV UUID, which the early-returns broke by calling getBytesValuesSV() above the isSingleValue() dispatch. No seventh switch here: unlike DistinctCountBitmap, HLL's dictionary path lives in AggregationFunctionUtils#getDistinctCountHLLResult, which already excludes UUID from the serialized branch so it falls through to the scalar path. --- .../DistinctCountHLLAggregationFunction.java | 94 +++++++++++-------- 1 file changed, 56 insertions(+), 38 deletions(-) diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLAggregationFunction.java b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLAggregationFunction.java index 8a837029e977..666903857e9b 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLAggregationFunction.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLAggregationFunction.java @@ -84,22 +84,8 @@ public void aggregate(int length, AggregationResultHolder aggregationResultHolde DataType dataType = blockValSet.getValueType(); DataType storedType = dataType.getStoredType(); - // UUID columns are stored as 16-byte BYTES, but a UUID value is a logical scalar — not a serialized - // HyperLogLog. Offer the canonical UUID string so the result matches DISTINCTCOUNTHLL on a STRING column - // holding the same logical UUIDs. NOTE: fetch raw bytes and convert explicitly — for identifier expressions - // the BlockValSet is a ProjectionBlockValSet whose getStringValuesSV() renders stored BYTES as bare hex, - // not the canonical RFC-4122 form. - if (dataType == DataType.UUID) { - byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); - HyperLogLog hyperLogLog = getHyperLogLog(aggregationResultHolder); - for (int i = 0; i < length; i++) { - hyperLogLog.offer(uuidBytesValues[i]); - } - return; - } - // Treat BYTES value as serialized HyperLogLog - if (storedType == DataType.BYTES) { + if (storedType == DataType.BYTES && dataType != DataType.UUID) { byte[][] bytesValues = blockValSet.getBytesValuesSV(); try { HyperLogLog hyperLogLog = aggregationResultHolder.getResult(); @@ -175,6 +161,13 @@ protected void aggregateSV(int length, AggregationResultHolder aggregationResult hyperLogLog.offer(stringValues[i]); } break; + // Reached only by UUID: a real BYTES column is serialized HLL state and is handled above. + case BYTES: + byte[][] uuidValues = blockValSet.getBytesValuesSV(); + for (int i = 0; i < length; i++) { + hyperLogLog.offer(uuidValues[i]); + } + break; default: throw new IllegalStateException("Illegal data type for DISTINCT_COUNT_HLL aggregation function: " + storedType); } @@ -238,6 +231,15 @@ protected void aggregateMV(int length, AggregationResultHolder aggregationResult } } break; + // Reached only by UUID: a real BYTES column is serialized HLL state and is handled above. + case BYTES: + byte[][][] uuidValuesArray = blockValSet.getBytesValuesMV(); + for (int i = 0; i < length; i++) { + for (byte[] value : uuidValuesArray[i]) { + hyperLogLog.offer(value); + } + } + break; default: throw new IllegalStateException("Illegal data type for DISTINCT_COUNT_HLL aggregation function: " + storedType); } @@ -251,17 +253,8 @@ public void aggregateGroupBySV(int length, int[] groupKeyArray, GroupByResultHol DataType dataType = blockValSet.getValueType(); DataType storedType = dataType.getStoredType(); - // UUID columns: offer canonical UUID strings converted from raw bytes (see aggregate() for rationale). - if (dataType == DataType.UUID) { - byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); - for (int i = 0; i < length; i++) { - getHyperLogLog(groupByResultHolder, groupKeyArray[i]).offer(uuidBytesValues[i]); - } - return; - } - // Treat BYTES value as serialized HyperLogLog - if (storedType == DataType.BYTES) { + if (storedType == DataType.BYTES && dataType != DataType.UUID) { byte[][] bytesValues = blockValSet.getBytesValuesSV(); try { for (int i = 0; i < length; i++) { @@ -334,6 +327,13 @@ protected void aggregateSVGroupBySV(int length, int[] groupKeyArray, GroupByResu getHyperLogLog(groupByResultHolder, groupKeyArray[i]).offer(stringValues[i]); } break; + // Reached only by UUID: a real BYTES column is serialized HLL state and is handled above. + case BYTES: + byte[][] uuidValues = blockValSet.getBytesValuesSV(); + for (int i = 0; i < length; i++) { + getHyperLogLog(groupByResultHolder, groupKeyArray[i]).offer(uuidValues[i]); + } + break; default: throw new IllegalStateException("Illegal data type for DISTINCT_COUNT_HLL aggregation function: " + storedType); } @@ -398,6 +398,16 @@ protected void aggregateMVGroupBySV(int length, int[] groupKeyArray, GroupByResu } } break; + // Reached only by UUID: a real BYTES column is serialized HLL state and is handled above. + case BYTES: + byte[][][] uuidValuesArray = blockValSet.getBytesValuesMV(); + for (int i = 0; i < length; i++) { + HyperLogLog hyperLogLog = getHyperLogLog(groupByResultHolder, groupKeyArray[i]); + for (byte[] value : uuidValuesArray[i]) { + hyperLogLog.offer(value); + } + } + break; default: throw new IllegalStateException("Illegal data type for DISTINCT_COUNT_HLL aggregation function: " + storedType); } @@ -411,20 +421,8 @@ public void aggregateGroupByMV(int length, int[][] groupKeysArray, GroupByResult DataType dataType = blockValSet.getValueType(); DataType storedType = dataType.getStoredType(); - // UUID columns: offer canonical UUID strings converted from raw bytes (see aggregate() for rationale). - if (dataType == DataType.UUID) { - byte[][] uuidBytesValues = blockValSet.getBytesValuesSV(); - for (int i = 0; i < length; i++) { - byte[] canonical = uuidBytesValues[i]; - for (int groupKey : groupKeysArray[i]) { - getHyperLogLog(groupByResultHolder, groupKey).offer(canonical); - } - } - return; - } - // Treat BYTES value as serialized HyperLogLog - if (storedType == DataType.BYTES) { + if (storedType == DataType.BYTES && dataType != DataType.UUID) { byte[][] bytesValues = blockValSet.getBytesValuesSV(); try { for (int i = 0; i < length; i++) { @@ -500,6 +498,13 @@ protected void aggregateSVGroupByMV(int length, int[][] groupKeysArray, GroupByR setValueForGroupKeys(groupByResultHolder, groupKeysArray[i], stringValues[i]); } break; + // Reached only by UUID: a real BYTES column is serialized HLL state and is handled above. + case BYTES: + byte[][] uuidValues = blockValSet.getBytesValuesSV(); + for (int i = 0; i < length; i++) { + setValueForGroupKeys(groupByResultHolder, groupKeysArray[i], uuidValues[i]); + } + break; default: throw new IllegalStateException("Illegal data type for DISTINCT_COUNT_HLL aggregation function: " + storedType); } @@ -582,6 +587,19 @@ protected void aggregateMVGroupByMV(int length, int[][] groupKeysArray, GroupByR } } break; + // Reached only by UUID: a real BYTES column is serialized HLL state and is handled above. + case BYTES: + byte[][][] uuidValuesArray = blockValSet.getBytesValuesMV(); + for (int i = 0; i < length; i++) { + byte[][] uuidValues = uuidValuesArray[i]; + for (int groupKey : groupKeysArray[i]) { + HyperLogLog hyperLogLog = getHyperLogLog(groupByResultHolder, groupKey); + for (byte[] value : uuidValues) { + hyperLogLog.offer(value); + } + } + } + break; default: throw new IllegalStateException("Illegal data type for DISTINCT_COUNT_HLL aggregation function: " + storedType); }