somandal commented on code in PR #10799:
URL: https://github.com/apache/pinot/pull/10799#discussion_r1204834820
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/utils/AggregationUtils.java:
##########
@@ -54,54 +54,92 @@ public static Key extractEmptyKey() {
return new Key(new Object[0]);
}
- private static Object mergeSum(Object left, Object right) {
- return ((Number) left).doubleValue() + ((Number) right).doubleValue();
+ // TODO: Use the correct type for SUM/MIN/MAX instead of always using double
+
+ @Nullable
+ private static Object mergeSum(@Nullable Object agg, @Nullable Object value)
{
+ if (agg == null) {
+ return value;
+ }
+ if (value == null) {
+ return agg;
+ }
+ return ((Number) agg).doubleValue() + ((Number) value).doubleValue();
}
- private static Object mergeMin(Object left, Object right) {
- return Math.min(((Number) left).doubleValue(), ((Number)
right).doubleValue());
+ @Nullable
+ private static Object mergeMin(@Nullable Object agg, @Nullable Object value)
{
+ if (agg == null) {
+ return value;
+ }
+ if (value == null) {
+ return agg;
+ }
+ return Math.min(((Number) agg).doubleValue(), ((Number)
value).doubleValue());
}
- private static Object mergeMax(Object left, Object right) {
- return Math.max(((Number) left).doubleValue(), ((Number)
right).doubleValue());
+ @Nullable
+ private static Object mergeMax(@Nullable Object agg, @Nullable Object value)
{
+ if (agg == null) {
Review Comment:
Just curious, today the leaf node leverages the nullValueBitmap, but this is
not available in the intermediate stages.
Example:
```
if (_nullHandlingEnabled) {
// TODO: avoid the null bitmap check when it is null or empty for
better performance.
RoaringBitmap nullBitmap = blockValSet.getNullBitmap();
if (nullBitmap == null) {
nullBitmap = new RoaringBitmap();
}
aggregateNullHandlingEnabled(length, aggregationResultHolder,
blockValSet, nullBitmap);
return;
}
```
Now we usually split the LogicalAggregate into an intermediate stage (which
may or may not run on leaf) and a final stage (which probably won't run on leaf
as it needs to do the final aggregations). Does this mean that the
nullValueBitmap won't be leveraged if this intermediate stage doesn't run in
the leaf? will that affect query results?
--
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
