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


##########
pinot-common/src/main/java/org/apache/pinot/common/request/context/RequestContextUtils.java:
##########
@@ -81,11 +81,6 @@ public static ExpressionContext getExpression(Expression 
thriftExpression) {
    */
   public static FunctionContext getFunction(Function thriftFunction) {
     String functionName = thriftFunction.getOperator();
-    if 
(functionName.equalsIgnoreCase(AggregationFunctionType.COUNT.getName())) {
-      // NOTE: COUNT always take one single argument "*"
-      return new FunctionContext(FunctionContext.Type.AGGREGATION, 
AggregationFunctionType.COUNT.getName(),
-          new 
ArrayList<>(Collections.singletonList(ExpressionContext.forIdentifier("*"))));
-    }

Review Comment:
   yes. I definitely understand this. since `count(*)` by definition counts 
number of row since an entire row cannot be null; and `count(col)` only counts 
for non-null.
   
   however since the multi-stage engine executes a logical planner after 
semantic parsing. `*` will always be resolved into a full list of table schema 
(and in the case of count(*), `*` will be dropped as the logical count doesn't 
take argument. see
   
https://github.com/apache/calcite/blob/b9c2099ea92a575084b55a206efc5dd341c0df62/core/src/main/java/org/apache/calcite/sql/SqlSyntax.java#L43-L47
   
   so this is really the compatibility situation 
   - since pinot current SQL parser only takes it to the semantic level it 
doesn't parse out the `*`, thus `*` remains a operand.
   - but logically `*` means no argument in logical plan, thus in multi-stage 
engine it doesn't have argument. 
   



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