xiangfu0 commented on code in PR #18873:
URL: https://github.com/apache/pinot/pull/18873#discussion_r3787800751
##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionUtils.java:
##########
@@ -799,10 +799,12 @@ 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) {
- if (dictionary.getValueType() == FieldSpec.DataType.BYTES) {
- // Treat BYTES value as serialized HyperLogLog
+ Dictionary dictionary = Objects.requireNonNull(dataSource.getDictionary());
+ if (dataSource.getDataSourceMetadata().getDataType() ==
FieldSpec.DataType.BYTES
Review Comment:
Cached DataSourceMetadata in the HLL, HLLPlus, and ULL paths, and the
serialized dictionary branch now checks exact logical DataType.BYTES.
##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountBitmapAggregationFunction.java:
##########
@@ -71,37 +72,21 @@ public void aggregate(int length, AggregationResultHolder
aggregationResultHolde
Map<ExpressionContext, BlockValSet> blockValSetMap) {
BlockValSet blockValSet = blockValSetMap.get(_expression);
- // Treat BYTES value as serialized RoaringBitmap
- DataType storedType = blockValSet.getValueType().getStoredType();
- if (storedType == DataType.BYTES) {
Review Comment:
Updated the distinct functions to handle exact logical DataType.BYTES first
via the legacy serialized-object path. UUID now falls through to the
stored-type BYTES switch branch like the other raw data types.
##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountBitmapAggregationFunction.java:
##########
@@ -71,37 +72,21 @@ public void aggregate(int length, AggregationResultHolder
aggregationResultHolde
Map<ExpressionContext, BlockValSet> blockValSetMap) {
BlockValSet blockValSet = blockValSetMap.get(_expression);
- // Treat BYTES value as serialized RoaringBitmap
- DataType storedType = blockValSet.getValueType().getStoredType();
- if (storedType == DataType.BYTES) {
- byte[][] bytesValues = blockValSet.getBytesValuesSV();
- RoaringBitmap valueBitmap = aggregationResultHolder.getResult();
- if (valueBitmap != null) {
- for (int i = 0; i < length; i++) {
- valueBitmap.or(RoaringBitmapUtils.deserialize(bytesValues[i]));
- }
- } else {
- valueBitmap = RoaringBitmapUtils.deserialize(bytesValues[0]);
- aggregationResultHolder.setValue(valueBitmap);
- for (int i = 1; i < length; i++) {
- valueBitmap.or(RoaringBitmapUtils.deserialize(bytesValues[i]));
- }
- }
- return;
- }
+ DataType dataType = blockValSet.getValueType();
+ DataType storedType = dataType.getStoredType();
Review Comment:
Moved storedType = dataType.getStoredType() after the logical BYTES early
return in each affected entry point, so UUID reaches the raw BYTES switch path.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
