Jackie-Jiang commented on code in PR #18873:
URL: https://github.com/apache/pinot/pull/18873#discussion_r3787702649
##########
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:
Change this to `(dataType == DataType.BYTES)`
##########
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:
(minor) Cache `dataSource.getDataSourceMetadata()` into a local variable,
same for other places
##########
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:
Move `DataType storedType = dataType.getStoredType()` here
--
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]
