Jackie-Jiang commented on code in PR #15525:
URL: https://github.com/apache/pinot/pull/15525#discussion_r2045201020
##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/MinMaxRangeAggregationFunction.java:
##########
@@ -81,10 +84,13 @@ public void aggregate(int length, AggregationResultHolder
aggregationResultHolde
for (int i = from; i < to; i++) {
MinMaxRangePair minMaxRangePair =
ObjectSerDeUtils.MIN_MAX_RANGE_PAIR_SER_DE.deserialize(bytesValues[i]);
minMax.apply(minMaxRangePair);
+ empty.set(false);
}
});
}
- setAggregationResult(aggregationResultHolder, minMax.getMin(),
minMax.getMax());
+ if (!empty.get()) {
Review Comment:
My conclusion is mainly based on how many times the method is invoked. With
current approach, we need to update the atomic boolean for each doc, which
could also potentially introduce memory barrier (not sure about the overhead in
single thread scenario).
The reading part should be more or less the same, but we can skip the update
--
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]
