[GitHub] dawidwys commented on issue #7177: [FLINK-7599] [table] Support for aggregates in MATCH_RECOGNIZE

[GitBox <gi...@apache.org>]

dawidwys commented on issue #7177: [FLINK-7599] [table] Support for aggregates in MATCH_RECOGNIZE
URL: https://github.com/apache/flink/pull/7177#issuecomment-443675670
 
 
   Hi @dianfu, @twalthr . Thank you for your review. I've updated the PR according to your comments. 
   
   There are still two main discussion points from @twalthr review:
   1. Moving `PatternVariableFinder`, which also validates that there is only a single pattern variable in an aggregation, to e.g. `DataStreamMatchRule`. The main role of this class is to extract that single variable though. If I performed the validation at an earlier stage, I would have to traverse the expression with the same code in `MatchCodeGenerator` anyway, but I am ok with it, if you think it is worth it.
   2. Using `aggCall.toString` as a key for reusing code for aggregation for a single variable that was used multiple times in an expression. I could not think of better way to identify that it is the same expression. Do you have an idea what could I use instead?
   
   I would appreciate if you had another look.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services