yashmayya commented on code in PR #17519:
URL: https://github.com/apache/pinot/pull/17519#discussion_r2766346682
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/DistinctCountBitmapValueAggregator.java:
##########
@@ -61,12 +61,28 @@ public RoaringBitmap applyRawValue(RoaringBitmap value,
Object rawValue) {
if (rawValue instanceof byte[]) {
value.or(deserializeAggregatedValue((byte[]) rawValue));
} else {
- value.add(rawValue.hashCode());
+ addToValue(value, rawValue);
}
_maxByteSize = Math.max(_maxByteSize, value.serializedSizeInBytes());
return value;
}
+ /**
+ * Adds a raw value (single value or multi-value array) to the RoaringBitmap.
+ */
+ protected void addToValue(RoaringBitmap bitmap, Object rawValue) {
+ if (rawValue instanceof Object[]) {
+ Object[] values = (Object[]) rawValue;
+ for (Object value : values) {
+ if (value != null) {
Review Comment:
Hm yeah I'm not able to find any scenarios where the MV elements can be
`null`, does look like they should be removed before this path is reached. I
just followed the pattern from all the existing ones. I've updated all of them
now to remove this redundant `null` check.
--
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]
