You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kie.apache.org by "yurloc (via GitHub)" <gi...@apache.org> on 2024/04/01 19:23:17 UTC
[PR] [new-parser] Complete support for built-in operators (after, before) [incubator-kie-drools]
yurloc opened a new pull request, #5812:
URL: https://github.com/apache/incubator-kie-drools/pull/5812
Fixes #5735.
<details>
<summary>
How to replicate CI configuration locally?
</summary>
Build Chain tool does "simple" maven build(s), the builds are just Maven commands, but because the repositories relates and depends on each other and any change in API or class method could affect several of those repositories there is a need to use [build-chain tool](https://github.com/kiegroup/github-action-build-chain) to handle cross repository builds and be sure that we always use latest version of the code for each repository.
[build-chain tool](https://github.com/kiegroup/github-action-build-chain) is a build tool which can be used on command line locally or in Github Actions workflow(s), in case you need to change multiple repositories and send multiple dependent pull requests related with a change you can easily reproduce the same build by executing it on Github hosted environment or locally in your development environment. See [local execution](https://github.com/kiegroup/github-action-build-chain#local-execution) details to get more information about it.
</details>
<details>
<summary>
How to retest this PR or trigger a specific build:
</summary>
- for <b>pull request and downstream checks</b>
- Push a new commit to the PR. An empty commit would be enough.
- for a <b>full downstream build</b>
- for <b>github actions</b> job: add the label `run_fdb`
- for <b>Jenkins PR check only</b>
- If you are an ASF committer for KIE podling, login to Jenkins (https://ci-builds.apache.org/job/KIE/job/drools/), go to the specific PR job, and click on `Build Now` button.
</details>
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] [new-parser] Complete support for built-in operators (after, before) [incubator-kie-drools]
Posted by "yurloc (via GitHub)" <gi...@apache.org>.
yurloc commented on PR #5812:
URL: https://github.com/apache/incubator-kie-drools/pull/5812#issuecomment-2032421292
Overall fixing progress: There were 531 failed tests.
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] [new-parser] Complete support for built-in operators (after, before) [incubator-kie-drools]
Posted by "yurloc (via GitHub)" <gi...@apache.org>.
yurloc commented on code in PR #5812:
URL: https://github.com/apache/incubator-kie-drools/pull/5812#discussion_r1546765479
##########
drools-drl/drools-drl-parser/src/main/antlr4/org/drools/drl/parser/antlr4/DRLParser.g4:
##########
@@ -142,7 +142,7 @@ inExpression : left=relationalExpression ( 'not'? 'in' LPAREN drlExpression (COM
relationalExpression : left=drlExpression (right=orRestriction)* ;
orRestriction : left=andRestriction (OR right=andRestriction)* ;
andRestriction : left=singleRestriction (AND right=singleRestriction)* ;
-singleRestriction : op=relationalOperator drlExpression ;
+singleRestriction : op=relationalOperator squareArguments? drlExpression ;
Review Comment:
Needed for some operators that take arguments, for example, `str`, `after`, and `before`. This may not be 100% accurate as not all operators take operators but it's completely in line with the expression parser.
##########
drools-drl/drools-drl-parser/src/main/antlr4/org/drools/drl/parser/antlr4/DRLParser.g4:
##########
@@ -142,7 +142,7 @@ inExpression : left=relationalExpression ( 'not'? 'in' LPAREN drlExpression (COM
relationalExpression : left=drlExpression (right=orRestriction)* ;
orRestriction : left=andRestriction (OR right=andRestriction)* ;
andRestriction : left=singleRestriction (AND right=singleRestriction)* ;
-singleRestriction : op=relationalOperator drlExpression ;
+singleRestriction : op=relationalOperator squareArguments? drlExpression ;
Review Comment:
Needed for some operators that take arguments, for example, `str`, `after`, and `before`. This may not be 100% accurate as not all operators take arguments but it's completely in line with the expression parser.
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] [new-parser] Complete support for built-in operators (after, before) [incubator-kie-drools]
Posted by "yurloc (via GitHub)" <gi...@apache.org>.
yurloc commented on PR #5812:
URL: https://github.com/apache/incubator-kie-drools/pull/5812#issuecomment-2031301371
Suggestion added. Please merge.
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] [new-parser] Complete support for built-in operators (after, before) [incubator-kie-drools]
Posted by "tkobayas (via GitHub)" <gi...@apache.org>.
tkobayas merged PR #5812:
URL: https://github.com/apache/incubator-kie-drools/pull/5812
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] [new-parser] Complete support for built-in operators (after, before) [incubator-kie-drools]
Posted by "yurloc (via GitHub)" <gi...@apache.org>.
yurloc commented on code in PR #5812:
URL: https://github.com/apache/incubator-kie-drools/pull/5812#discussion_r1546760980
##########
drools-drl/drools-drl-parser/src/main/antlr4/org/drools/drl/parser/antlr4/DRL6Expressions.g4:
##########
@@ -908,11 +925,13 @@ in_key
;
operator_key
- // At the moment, we list possible DRLLexer tokens here, but we may be able to improve this by isolating lexers. IDENTIFIER is required to accept custom operators
- // We need to keep this semantic predicate for custom operators
- : {(helper.isPluggableEvaluator(false))}? id=(IDENTIFIER|DRL_MATCHES|DRL_MEMBEROF|DRL_CONTAINS|DRL_EXCLUDES|DRL_SOUNDSLIKE|DRL_STR) { helper.emit($id, DroolsEditorType.KEYWORD); }
+ // IDENTIFIER is required to accept custom operators. We need to keep this semantic predicate for custom operators
+ : {(helper.isPluggableEvaluator(false))}? id=IDENTIFIER { helper.emit($id, DroolsEditorType.KEYWORD); }
+ // At the moment, we list possible DRLLexer tokens here, but we may be able to improve this by isolating lexers.
Review Comment:
@tkobayas This comment may need improving. We no longer list the keywords here but in the `builtInOperator` rule. Plus, I don't know what's meant by isolating lexers and how that can help.
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] [new-parser] Complete support for built-in operators (after, before) [incubator-kie-drools]
Posted by "yurloc (via GitHub)" <gi...@apache.org>.
yurloc commented on code in PR #5812:
URL: https://github.com/apache/incubator-kie-drools/pull/5812#discussion_r1546919404
##########
drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/MiscDRLParserTest.java:
##########
@@ -1980,7 +1980,6 @@ public void parse_ExtraLhsNewline() throws Exception {
assertThat(parser.hasErrors()).as(parser.getErrors().toString()).isFalse();
}
- @Disabled("Priority : Low | Implement soundslike")
Review Comment:
Fixed some time ago by #5767.
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] [new-parser] Complete support for built-in operators (after, before) [incubator-kie-drools]
Posted by "tkobayas (via GitHub)" <gi...@apache.org>.
tkobayas commented on code in PR #5812:
URL: https://github.com/apache/incubator-kie-drools/pull/5812#discussion_r1547037312
##########
drools-drl/drools-drl-parser/src/main/antlr4/org/drools/drl/parser/antlr4/DRL6Expressions.g4:
##########
@@ -908,11 +925,13 @@ in_key
;
operator_key
- // At the moment, we list possible DRLLexer tokens here, but we may be able to improve this by isolating lexers. IDENTIFIER is required to accept custom operators
- // We need to keep this semantic predicate for custom operators
- : {(helper.isPluggableEvaluator(false))}? id=(IDENTIFIER|DRL_MATCHES|DRL_MEMBEROF|DRL_CONTAINS|DRL_EXCLUDES|DRL_SOUNDSLIKE|DRL_STR) { helper.emit($id, DroolsEditorType.KEYWORD); }
+ // IDENTIFIER is required to accept custom operators. We need to keep this semantic predicate for custom operators
+ : {(helper.isPluggableEvaluator(false))}? id=IDENTIFIER { helper.emit($id, DroolsEditorType.KEYWORD); }
+ // At the moment, we list possible DRLLexer tokens here, but we may be able to improve this by isolating lexers.
Review Comment:
```suggestion
```
Yeah, we no longer list tokens. Also we are going forward sharing `DRLLexer.g4`, so I think we can simply remove this comment.
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] [new-parser] Complete support for built-in operators (after, before) [incubator-kie-drools]
Posted by "tkobayas (via GitHub)" <gi...@apache.org>.
tkobayas commented on code in PR #5812:
URL: https://github.com/apache/incubator-kie-drools/pull/5812#discussion_r1547037312
##########
drools-drl/drools-drl-parser/src/main/antlr4/org/drools/drl/parser/antlr4/DRL6Expressions.g4:
##########
@@ -908,11 +925,13 @@ in_key
;
operator_key
- // At the moment, we list possible DRLLexer tokens here, but we may be able to improve this by isolating lexers. IDENTIFIER is required to accept custom operators
- // We need to keep this semantic predicate for custom operators
- : {(helper.isPluggableEvaluator(false))}? id=(IDENTIFIER|DRL_MATCHES|DRL_MEMBEROF|DRL_CONTAINS|DRL_EXCLUDES|DRL_SOUNDSLIKE|DRL_STR) { helper.emit($id, DroolsEditorType.KEYWORD); }
+ // IDENTIFIER is required to accept custom operators. We need to keep this semantic predicate for custom operators
+ : {(helper.isPluggableEvaluator(false))}? id=IDENTIFIER { helper.emit($id, DroolsEditorType.KEYWORD); }
+ // At the moment, we list possible DRLLexer tokens here, but we may be able to improve this by isolating lexers.
Review Comment:
```suggestion
```
Yeah, we no longer list tokens. Also we are going forward sharing `DRLLexer.g4`, so I think we can simply remove this comment.
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] [new-parser] Complete support for built-in operators (after, before) [incubator-kie-drools]
Posted by "yurloc (via GitHub)" <gi...@apache.org>.
yurloc commented on code in PR #5812:
URL: https://github.com/apache/incubator-kie-drools/pull/5812#discussion_r1546773602
##########
drools-drl/drools-drl-parser/src/main/antlr4/org/drools/drl/parser/antlr4/DRLParser.g4:
##########
@@ -161,13 +161,7 @@ relationalOperator
| temporalOperator
;
-drlRelationalOperator
- : DRL_NOT? DRL_MATCHES
- | DRL_NOT? DRL_MEMBEROF
- | DRL_NOT? DRL_CONTAINS
- | DRL_NOT? DRL_EXCLUDES
- | DRL_NOT? DRL_SOUNDSLIKE
- | DRL_NOT? DRL_STR LBRACK IDENTIFIER RBRACK ;
+drlRelationalOperator : DRL_NOT? builtInOperator ;
Review Comment:
Not 100% accurate as some operators do not support negation (`==`, `!=`, `<`, etc.) but I believe we can afford this inaccuracy at this point in the DRLParser, which is more high-level and not primarily focused on expressions.
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] [new-parser] Complete support for built-in operators (after, before) [incubator-kie-drools]
Posted by "tkobayas (via GitHub)" <gi...@apache.org>.
tkobayas commented on code in PR #5812:
URL: https://github.com/apache/incubator-kie-drools/pull/5812#discussion_r1547037786
##########
drools-drl/drools-drl-parser/src/main/antlr4/org/drools/drl/parser/antlr4/DRL6Expressions.g4:
##########
@@ -908,11 +925,13 @@ in_key
;
operator_key
- // At the moment, we list possible DRLLexer tokens here, but we may be able to improve this by isolating lexers. IDENTIFIER is required to accept custom operators
- // We need to keep this semantic predicate for custom operators
- : {(helper.isPluggableEvaluator(false))}? id=(IDENTIFIER|DRL_MATCHES|DRL_MEMBEROF|DRL_CONTAINS|DRL_EXCLUDES|DRL_SOUNDSLIKE|DRL_STR) { helper.emit($id, DroolsEditorType.KEYWORD); }
+ // IDENTIFIER is required to accept custom operators. We need to keep this semantic predicate for custom operators
+ : {(helper.isPluggableEvaluator(false))}? id=IDENTIFIER { helper.emit($id, DroolsEditorType.KEYWORD); }
+ // At the moment, we list possible DRLLexer tokens here, but we may be able to improve this by isolating lexers.
Review Comment:
```suggestion
```
Yeah, we no longer list tokens. Also we are going forward sharing `DRLLexer.g4`, so I think we can simply remove this comment.
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] [new-parser] Complete support for built-in operators (after, before) [incubator-kie-drools]
Posted by "yurloc (via GitHub)" <gi...@apache.org>.
yurloc commented on PR #5812:
URL: https://github.com/apache/incubator-kie-drools/pull/5812#issuecomment-2030581052
@tkobayas @mariofusco @gitgabrio please review.
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org