walterddr commented on code in PR #10799:
URL: https://github.com/apache/pinot/pull/10799#discussion_r1204839223


##########
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:
   try to piece out what this comment is about, here is my understanding:
   
   - intermediate stage is run on `List<Object[]>` so it can naturally 
represent null with the object array
   - what you mentioned with nullValueBitmap is being used in 2 places
       - in leaf operators that will read it out when `enableNullHandling` flag 
is turned on
       - in data table ser/de --> this also means when intermediate stage 
received the payload, it will deserialized it into `List<Object[]>` with the 
null put right into places. 
   
   please let me know if this answers your question



-- 
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

Reply via email to