bziobrowski commented on code in PR #14342:
URL: https://github.com/apache/pinot/pull/14342#discussion_r1827473958
##########
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 I meant is - shouldn't the function be checking all input expressions
instead of only the first one ?
```java
inputExpressions.get(0);
```
As for testing - is there a test asserting a plan similar to the one you
mention in PR description ?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
