Jackie-Jiang commented on code in PR #13573:
URL: https://github.com/apache/pinot/pull/13573#discussion_r1674552807


##########
pinot-query-planner/src/main/java/org/apache/pinot/calcite/rel/rules/PinotAggregateExchangeNodeInsertRule.java:
##########
@@ -294,42 +299,37 @@ private static List<AggregateCall> 
buildAggCalls(Aggregate aggRel, AggType aggTy
   //   - argList is replaced with rexList
   private static AggregateCall buildAggCall(RelNode input, AggregateCall 
orgAggCall, List<RexNode> rexList,
       int numGroups, AggType aggType) {
-    String functionName = orgAggCall.getAggregation().getName();
+    SqlAggFunction orgAggFunction = orgAggCall.getAggregation();
+    String functionName = orgAggFunction.getName();
+    SqlKind kind = orgAggFunction.getKind();
+    SqlFunctionCategory functionCategory = orgAggFunction.getFunctionType();
     if (orgAggCall.isDistinct()) {
-      if (functionName.equals("COUNT")) {
+      if (kind == SqlKind.COUNT) {
         functionName = "DISTINCTCOUNT";
-      } else if (functionName.equals("LISTAGG")) {
+        kind = SqlKind.OTHER_FUNCTION;
+        functionCategory = SqlFunctionCategory.USER_DEFINED_FUNCTION;
+      } else if (kind == SqlKind.LISTAGG) {
         rexList.add(input.getCluster().getRexBuilder().makeLiteral(true));
       }
     }
-    AggregationFunctionType functionType = 
AggregationFunctionType.getAggregationFunctionType(functionName);
-    SqlAggFunction sqlAggFunction;
-    switch (aggType) {
-      case DIRECT:
-        sqlAggFunction = new PinotSqlAggFunction(functionName, null, 
functionType.getSqlKind(),
-            ReturnTypes.explicit(orgAggCall.getType()), null, 
functionType.getOperandTypeChecker(),
-            functionType.getSqlFunctionCategory());
-        break;
-      case LEAF:
-        sqlAggFunction = new PinotSqlAggFunction(functionName, null, 
functionType.getSqlKind(),
-            functionType.getIntermediateReturnTypeInference(), null, 
functionType.getOperandTypeChecker(),
-            functionType.getSqlFunctionCategory());
-        break;
-      case INTERMEDIATE:
-        sqlAggFunction = new PinotSqlAggFunction(functionName, null, 
functionType.getSqlKind(),
-            functionType.getIntermediateReturnTypeInference(), null, 
OperandTypes.ANY,
-            functionType.getSqlFunctionCategory());
-        break;
-      case FINAL:
-        sqlAggFunction = new PinotSqlAggFunction(functionName, null, 
functionType.getSqlKind(),
-            ReturnTypes.explicit(orgAggCall.getType()), null, 
OperandTypes.ANY, functionType.getSqlFunctionCategory());
-        break;
-      default:
-        throw new IllegalStateException("Unsupported AggType: " + aggType);
+    SqlReturnTypeInference returnTypeInference = null;
+    RelDataType returnType = null;
+    // Override the intermediate result type inference if it is provided
+    if (aggType.isOutputIntermediateFormat()) {
+      AggregationFunctionType functionType = 
AggregationFunctionType.getAggregationFunctionType(functionName);
+      returnTypeInference = functionType.getIntermediateReturnTypeInference();
     }
+    if (returnTypeInference == null) {
+      returnType = orgAggCall.getType();
+      returnTypeInference = ReturnTypes.explicit(returnType);
+    }

Review Comment:
   `AggregationFunctionType.getIntermediateReturnTypeInference` can return null 
when the return type is the same as final return type. Basically we want to 
directly use the return type if no intermediate return type inference override 
is provided. Added some comment



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

Reply via email to