[GitHub] [flink] godfreyhe commented on a change in pull request #15341: [FLINK-21923][table-planner-blink] Fix SplitAggregateRule not support both filter, count/sum, avg

[GitBox]

godfreyhe commented on a change in pull request #15341:
URL: https://github.com/apache/flink/pull/15341#discussion_r618880846



##########
File path: 
flink-table/flink-table-planner-blink/src/main/scala/org/apache/flink/table/planner/plan/rules/logical/SplitAggregateRule.scala
##########
@@ -280,11 +282,15 @@ class SplitAggregateRule extends RelOptRule(
     }
 
     // STEP 2.3: construct partial aggregates
-    relBuilder.aggregate(
-      relBuilder.groupKey(fullGroupSet, 
ImmutableList.of[ImmutableBitSet](fullGroupSet)),
+    // Create aggregate node directly to avoid ClassCastException,
+    // Please see https://issues.apache.org/jira/browse/FLINK-21923 for more 
details.
+    val partialAggregate = FlinkLogicalAggregate.create(

Review comment:
       can you add a TODO here (such as `// TODO reuse aggregate function, see 
FLINK-22412`)
    and add some tests about window aggregate




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

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