Tartarus0zm commented on pull request #15341: URL: https://github.com/apache/flink/pull/15341#issuecomment-813945427
> @Tartarus0zm Thanks for contribution. I have two problems: > > 1. If I understand correctly, the root cause of the exception is it's not safe assume the top `RelNode` of `RelBuilder` is `Aggregate` after call `RelBuilder`#aggregate. I found there are also other class make the same mistake: > > * Line 162 in `FlinkRelBuilder`#windowAggregate > * Line 286 in `SplitAggregateRule`#onMatch to set PartialFinalType.PARTIAL > * Line 314 in `SplitAggregateRule`#onMatch to set PartialFinalType.FINAL > It's better to take all those places into consideration. > 2. If we only want to support two cases after call `RelBuilder`#aggregate in `SplitAggregateRule` currently: 1. top `RelNode` is project 2. top `RelNode` is aggregate. I think it's better to throw a `TableException` for other pattern in default behavior. @beyond1920 thanks for your review. I has fixed it, please take a look again. -- 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: [email protected]
