[GitHub] flink issue #3477: [Flink-3318][cep] Add support for quantifiers to CEP's pa...

Github user eliaslevy commented on the issue: https://github.com/apache/flink/pull/3477 I created [FLINK-6419](https://issues.apache.org/jira/browse/FLINK-6419). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your

[GitHub] flink issue #3477: [Flink-3318][cep] Add support for quantifiers to CEP's pa...

Github user kl0u commented on the issue: https://github.com/apache/flink/pull/3477 Hi @eliaslevy , @dawidwys is right, in fact if the state's name is "a" and it has 2 matching events, there will be two returned keys "a_0" and "a_1". This is definitely counterintuitive and the reason

[GitHub] flink issue #3477: [Flink-3318][cep] Add support for quantifiers to CEP's pa...

Github user dawidwys commented on the issue: https://github.com/apache/flink/pull/3477 Right now if the pattern name is "a" the events will be returned with keys: "a[0]", "a[1]" and so on, but agree it may be counterintuitive. Please file a JIRA for it. --- If your project is set

[GitHub] flink issue #3477: [Flink-3318][cep] Add support for quantifiers to CEP's pa...

Github user eliaslevy commented on the issue: https://github.com/apache/flink/pull/3477 Am I missing something or is there no way to get access to access to multiple events matched by these new quantifiers? The `PatternSelectFunction.select` takes an argument of `Map` and

[GitHub] flink issue #3477: [Flink-3318][cep] Add support for quantifiers to CEP's pa...

Github user dawidwys commented on the issue: https://github.com/apache/flink/pull/3477 Great! Thanks for your help @kl0u ! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature

[GitHub] flink issue #3477: [Flink-3318][cep] Add support for quantifiers to CEP's pa...

Github user kl0u commented on the issue: https://github.com/apache/flink/pull/3477 Merged this! Thanks for the work @dawidwys. Can you close this PR and the JIRA? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If

[GitHub] flink issue #3477: [Flink-3318][cep] Add support for quantifiers to CEP's pa...

Github user kl0u commented on the issue: https://github.com/apache/flink/pull/3477 Hi @dawidwys , I have 2 comments for this PR: 1) in the `findFinalStateAfterProceed()` in the `NFA` you should also check the filter condition as you traverse the `PROCEED` edges towards the end.

[GitHub] flink issue #3477: [Flink-3318][cep] Add support for quantifiers to CEP's pa...

Github user dawidwys commented on the issue: https://github.com/apache/flink/pull/3477 I've updated the PR with a solution for the previously failing test. Also tried to add some more comments. @kl0u I think it is ready for another round of review :) --- If your project is set up

[GitHub] flink issue #3477: [Flink-3318][cep] Add support for quantifiers to CEP's pa...

Github user kl0u commented on the issue: https://github.com/apache/flink/pull/3477 Sounds good! Thanks @dawidwys --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and

[GitHub] flink issue #3477: [Flink-3318][cep] Add support for quantifiers to CEP's pa...

Github user dawidwys commented on the issue: https://github.com/apache/flink/pull/3477 Hi, I've addressed the style comments. Still have some doubts about the `Preconditions`. I will try to add more comments on the `NFACompiler` with the implementation that will handle the failing

[GitHub] flink issue #3477: [Flink-3318][cep] Add support for quantifiers to CEP's pa...

Github user kl0u commented on the issue: https://github.com/apache/flink/pull/3477 Hi @dawidwys , the following produces no output, which should not be the case. The same holds if you change the pattern to `oneOrMore`. ``` public void testZeroOrMore() {

[GitHub] flink issue #3477: [Flink-3318][cep] Add support for quantifiers to CEP's pa...

Github user kl0u commented on the issue: https://github.com/apache/flink/pull/3477 Thanks @dawidwys I will have a look now. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature

[GitHub] flink issue #3477: [Flink-3318][cep] Add support for quantifiers to CEP's pa...

Github user dawidwys commented on the issue: https://github.com/apache/flink/pull/3477 Thanks @kl0u for your comments, I've changed all code style specific. Some comments were addressing a behaviour that one `Pattern` can be translated into multiple "equivalent" `States`

[GitHub] flink issue #3477: [Flink-3318][cep] Add support for quantifiers to CEP's pa...

Github user kl0u commented on the issue: https://github.com/apache/flink/pull/3477 Hi @dawidwys , I will start reviewing your PR now. I would say that for now it makes sense to have it with exceptions and not change the `Pattern` hierarchy. The reason is: 1) not sure