Jackie-Jiang commented on code in PR #9086:
URL: https://github.com/apache/pinot/pull/9086#discussion_r929370127
##########
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:
Understood. We definitely need to track the null values in the aggregated
column, but do we need to track the `_groupKeyForNullValue` in the aggregation
function as well? I think we already track it in the group-by executor, which
should be enough
--
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
