gortiz commented on code in PR #14342:
URL: https://github.com/apache/pinot/pull/14342#discussion_r1827448259


##########
pinot-core/src/main/java/org/apache/pinot/core/plan/AggregationPlanNode.java:
##########
@@ -118,6 +120,38 @@ public Operator<AggregationResultsBlock> 
buildNonFilteredAggOperator() {
     return new AggregationOperator(_queryContext, aggregationInfo, 
numTotalDocs);
   }
 
+  /**
+   * Returns {@code true} if any of the aggregation functions have null 
values, {@code false} otherwise.
+   *
+   * The current implementation is pessimistic and returns {@code true} if any 
of the arguments to the aggregation
+   * functions is of function type. This is because we do not have a way to 
determine if the function will return null
+   * values without actually evaluating it.
+   */
+  private boolean hasNullValues(AggregationFunction[] aggregationFunctions) {
+    for (AggregationFunction aggregationFunction : aggregationFunctions) {
+      List<?> inputExpressions = aggregationFunction.getInputExpressions();
+      if (inputExpressions.isEmpty()) {
+        continue;
+      }

Review Comment:
   > What if aggregation function has more than one input expression ?
   
   I don't get it. Then all of them need to fulfill the conditions imposed 
here. This method returns true if and only if we are sure all inputs for all 
aggregation functions are not nullable. We cannot be sure in the case of 
functions and an aggregation function without inputs trivially fulfill this 
requirement.
   
   > btw, it'd be good to add some test(s).
   
   This is actually being tested: 
https://app.codecov.io/gh/apache/pinot/pull/14342/blob/pinot-core/src/main/java/org/apache/pinot/core/plan/AggregationPlanNode.java



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