Jackie-Jiang commented on code in PR #9086: URL: https://github.com/apache/pinot/pull/9086#discussion_r930400432
########## pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/MaxAggregationFunction.java: ########## @@ -28,13 +28,19 @@ import org.apache.pinot.core.query.aggregation.groupby.DoubleGroupByResultHolder; import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder; import org.apache.pinot.segment.spi.AggregationFunctionType; +import org.roaringbitmap.RoaringBitmap; public class MaxAggregationFunction extends BaseSingleInputAggregationFunction<Double, Double> { private static final double DEFAULT_INITIAL_VALUE = Double.NEGATIVE_INFINITY; + private final boolean _nullHandlingEnabled; - public MaxAggregationFunction(ExpressionContext expression) { + // stores id of the groupKey where the corresponding value is null. + private Integer _groupKeyForNullValue = null; Review Comment: I'm still confused. In `aggregateGroupBySVNullHandlingEnabled()`, the `nullBitmap` is for the aggregated value instead of the group-by column. When the aggregated value is null, why do we set the `_groupKeyForNullValue`? There can be multiple aggregated value to be null, but with different group key. -- 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: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org