dang-stripe commented on code in PR #18471:
URL: https://github.com/apache/pinot/pull/18471#discussion_r3229362644
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/MultistageAggregationExecutor.java:
##########
@@ -65,6 +65,10 @@ public MultistageAggregationExecutor(AggregationFunction[]
aggFunctions, int[] f
_mergeResultHolder = null;
} else {
_mergeResultHolder = new Object[numFunctions];
+ for (int i = 0; i < numFunctions; i++) {
+ _mergeResultHolder[i] = aggFunctions[i].extractAggregationResult(
Review Comment:
I originally added this in our fork before I saw the fix in
https://github.com/apache/pinot/pull/17750. It was intended to be a different
way of addressing the same bug.
When all segments are pruned by the broker, the final aggregate stage
receives no input blocks so `_mergeResultHolder[i]` will be null and calling
`getResult()` will trigger an NPE. This felt like a more comprehensive change
so we don't need to update each aggregation function for null handling.
But then I realized returning identity for all aggregation types might not
be right here (depends on the aggregation) so I've reverted this change.
--
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]
