beyond1920 edited a comment on pull request #16669: URL: https://github.com/apache/flink/pull/16669#issuecomment-894123926
@cshuo, about point 2, if routine the session window TVF directly to StreamExecGroupWindowAggregate, some valid cases would not work. For example, Session window aggregate followed by WindowRank or WindowJoin. Because we only infer WindowProperties in `FlinkRelMdWindowProperties` for new Window TVF RelNodes which does not include legacy `GroupWindowAggregate`. You may say we could handle `GroupWindowAggregate` in `FlinkRelMdWindowProperties`. But I think it is not a good idea. (1) It's a little ugly to mix the `GroupWindowAggregate` and WindowJoin/WindowRank in a planner of the above example. (2) The planner would not be compatible with previous one after we remove the GroupWindowAggregate in the later version. About the point 1 (deduplicate code), they would not exist after we totally remove the GroupWindowAggregate in the later version. I think it is OK, just like before we remove legacy planner, there are also deduplicate code in Blink planner and legacy planner. So I prefer the 1st choice. The Job planner would not updated after we totally remove the `GroupWindowAggregate` in the later version. That means a lot. -- 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]
