kotwal-itpro commented on PR #29395: URL: https://github.com/apache/flink/pull/29395#issuecomment-6322074632
Thanks @sangkyoonnam, this is a really useful review, and thanks for checking 2.3.0 too. I pushed both changes in a new commit, on top of a rebase onto current master: - I took the batch assertion you suggested. With the rule-set changes reverted, it now fails. - I added your streaming test to `stream/sql/AggregateITCase`. It fails without the `FlinkStreamRuleSets` change and passes with it. I'm happy to go either way on the question for @lincoln-lil. The rule-set change keeps the fix to one place and lets `AVG` share the sum with `SUM(x)`. Widening the sum in `reduceAvg` would keep the folding of empty input. If widening is preferred, I can rework the PR around that. -- 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]
