NobiGo commented on code in PR #4201:
URL: https://github.com/apache/calcite/pull/4201#discussion_r2011560596


##########
core/src/main/java/org/apache/calcite/rel/rules/ProjectAggregateMergeRule.java:
##########
@@ -154,9 +156,22 @@ && kindCount(project.getProjects(), SqlKind.CASE) == 0) {
     builder.push(aggregate.getInput());
     builder.aggregate(
         builder.groupKey(aggregate.getGroupSet(), aggregate.groupSets), 
aggCallList);
-    builder.project(
-        RexPermuteInputsShuttle.of(mapping).visitList(projects2),
-            project.getRowType().getFieldNames());
+    // If the output type of Aggregate does not match the input type required 
by original Project,
+    // CAST needs to be added
+    List<RexNode> rexInputRefs = 
RexPermuteInputsShuttle.of(mapping).visitList(projects2);

Review Comment:
   Here, we convert `CASE(IS NOT NULL($0), CAST($0):INTEGER NOT NULL, 0)` into 
`SUM0(x)` when `$0` is `SUM(x)`.
   I attempted to directly define the return type of `SUM0(x)` as `INTEGER` 
when constructing it to be consistent with the output type of CASE. However, 
this was checked in the Aggregate's create method. When the parameter type is 
TINYINT, the result of SUM0(x) should also be TINYINT other throws exception. 
Therefore, this approach doesn't work.
   When the returned type of the constructed SUM0(x) is TINYINT, not in line 
with the original plan. Therefore, in the PR, I converted the final type of 
`SUM0(x)` to be consistent with `CAST`. 
   
   Why don't I directly use `CAST($0):INTEGER NOT NULL` as the parameter for 
constructing the Aggregate? From this AggregateCall#create API, the parameter 
is an ordinals, so we need to construct a Project containing `CAST($0):INTEGER 
NOT NULL`. This approach will make the problem more complicated.



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

Reply via email to