kristoffSC commented on PR #21393:
URL: https://github.com/apache/flink/pull/21393#issuecomment-1419116469
I already have fix, will provide PR shortly.
--
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
kristoffSC commented on PR #21393:
URL: https://github.com/apache/flink/pull/21393#issuecomment-1419069671
@snuyanzin
> it looks like it generates methods with same signature for some cases e.g.
I will take a look at this one.
This was not causing CI build to fail?
--
kristoffSC commented on PR #21393:
URL: https://github.com/apache/flink/pull/21393#issuecomment-1415919713
Hi @tsreaper
I've implemented test you requested for. It's 'CodeSplitITCase
::testManyAggregationsWithGroupBy'
CI/CD is green.
Anything else you would like me to do?
kristoffSC commented on PR #21393:
URL: https://github.com/apache/flink/pull/21393#issuecomment-1412469928
Hi @tsreaper
thanks for kind words. I'm really happy that we are getting closer to merge
this change.
I was using Maciej Bryński's SQL example from FLINK-27246 ticket to
kristoffSC commented on PR #21393:
URL: https://github.com/apache/flink/pull/21393#issuecomment-1408677439
@tsreaper
Regarding `I'd like to see more examples. Could you please provide examples
and explain`
If we focus only on IF/ELSE statements without `else if` blocks which are
kristoffSC commented on PR #21393:
URL: https://github.com/apache/flink/pull/21393#issuecomment-1405755647
@tsreaper
I've refactored both `BlockStatementSplitter` and `BlockStatementGrouper` so
they use one `for` loop and one `visitor` instance like you suggested in your
comments.
kristoffSC commented on PR #21393:
URL: https://github.com/apache/flink/pull/21393#issuecomment-1405572025
@flinkbot run azure
--
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
kristoffSC commented on PR #21393:
URL: https://github.com/apache/flink/pull/21393#issuecomment-1405227888
Hi @tsreaper
I'm currently working on the refactoring to include your suggestions, it
does look promising. I should have new version by the end of this week.
I have a
kristoffSC commented on PR #21393:
URL: https://github.com/apache/flink/pull/21393#issuecomment-1396174097
Hi @tsreaper
Thank you for your valuable feedback, no need to apologize :) Please see my
response below.
> In BlockStatementSplitter you group all single statements between
kristoffSC commented on PR #21393:
URL: https://github.com/apache/flink/pull/21393#issuecomment-1384601282
Hi @tsreaper
I believe I have replayed to all your comments and suggestions.
I also provided some more explanation about the logic in few comments:
kristoffSC commented on PR #21393:
URL: https://github.com/apache/flink/pull/21393#issuecomment-1384203356
@flinkbot run azure
--
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
kristoffSC commented on PR #21393:
URL: https://github.com/apache/flink/pull/21393#issuecomment-1374392355
@flinkbot run azure
--
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
12 matches
Mail list logo
