[GitHub] [pinot] andiem-bq commented on a diff in pull request #10268: Resolve NullPointerException when COUNT() is passed literal and nullHandling is enabled

["andiem-bq (via GitHub)" <gi...@apache.org>]

andiem-bq commented on code in PR #10268:
URL: https://github.com/apache/pinot/pull/10268#discussion_r1106108192


##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/CountAggregationFunction.java:
##########
@@ -50,7 +50,11 @@ public CountAggregationFunction(ExpressionContext expression) {
   public CountAggregationFunction(ExpressionContext expression, boolean nullHandlingEnabled) {
     super(expression);
     // Consider null values only when null handling is enabled and function is not COUNT(*)
-    _nullHandlingEnabled = nullHandlingEnabled && !expression.getIdentifier().equals("*");
+    // Note COUNT on any literal gives same result as COUNT(*)
+    // So allow for identifiers that are not * and functions, disable for literals and *
+    _nullHandlingEnabled = nullHandlingEnabled
+            && ((expression.getType() == ExpressionContext.Type.IDENTIFIER && !expression.getIdentifier().equals("*"))
+            || (expression.getType() == ExpressionContext.Type.FUNCTION));

Review Comment:
   My thought was to give COUNT(literal) same treatment as COUNT(*) as these give same result of returning total # of rows in a collection regardless of nulls. 
   But main goal of fix was to avoid the getIdentifier.equals() NPE. Should I revise the statement to allow null handling for identifiers that are not * and all non-identifiers?



-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org