Jackie-Jiang commented on code in PR #13758:
URL: https://github.com/apache/pinot/pull/13758#discussion_r1715974434
##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/NullableSingleInputAggregationFunction.java:
##########
@@ -81,7 +81,9 @@ public void forEachNotNull(int length, BlockValSet
blockValSet, BatchConsumer co
return;
}
- forEachNotNull(length, roaringBitmap.getIntIterator(), consumer);
+ if (roaringBitmap.getCardinality() < length) {
Review Comment:
As @richardstartin said, `getCardinality()` can be linear time complexity in
the worst case. Here we want to check if the bitmap is fully set, and a cheaper
method for the same purpose is `roaringBitmap.contains(0, length)` which early
terminate when it detects an unset bit.
We are doing similar checks in quite some places. Do you want to merge this
one first, then open a separate PR to fix all of them?
--
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]
