Re: [PR] Precompile regex pattern for 'matches' operator and make it part of the lambda predicate class [incubator-kie-drools]
DimaSol commented on code in PR #6469:
URL:
https://github.com/apache/incubator-kie-drools/pull/6469#discussion_r2402859785
##
drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/operatorspec/RegexOperatorSpec.java:
##
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.drools.model.codegen.execmodel.generator.operatorspec;
+
+import com.github.javaparser.ast.Modifier;
+import com.github.javaparser.ast.NodeList;
+import com.github.javaparser.ast.body.FieldDeclaration;
+import com.github.javaparser.ast.body.VariableDeclarator;
+import com.github.javaparser.ast.expr.*;
+import org.drools.model.codegen.execmodel.generator.RuleContext;
+import org.drools.model.codegen.execmodel.generator.TypedExpression;
+import
org.drools.model.codegen.execmodel.generator.expressiontyper.ExpressionTyper;
+import org.drools.mvel.parser.ast.expr.PointFreeExpr;
+import org.drools.mvel.parser.ast.expr.RegexExpr;
+
+import java.util.regex.Pattern;
+
+import static
org.drools.model.codegen.execmodel.generator.DrlxParseUtil.toClassOrInterfaceType;
+import static org.drools.util.StringUtils.md5Hash;
+
+public class RegexOperatorSpec extends NativeOperatorSpec {
+public static final RegexOperatorSpec INSTANCE = new RegexOperatorSpec();
+
+public Expression getExpression(RuleContext context, PointFreeExpr
pointFreeExpr, TypedExpression left, ExpressionTyper expressionTyper) {
+if (shouldInlineRegex(pointFreeExpr.getRight())) {
+return super.getExpression(context, pointFreeExpr, left,
expressionTyper);
+} else {
+// TODO(?) throw some exception instead of creating
regexp_failed_to_parse expressions
+Expression regexExpression = pointFreeExpr.getRight().getFirst()
+.orElse(new StringLiteralExpr("regexp_failed_to_parse"));
+
+NameExpr regexFieldName = new NameExpr("regexp_" +
md5Hash(regexExpression.toString()));
+
+return new RegexExpr(
+getClassMemberFieldDeclaration(regexFieldName,
regexExpression),
+getClassMemberInvocationExpression(pointFreeExpr, left,
regexFieldName)
+);
+}
+}
+
+private EnclosedExpr getClassMemberInvocationExpression(PointFreeExpr
pointFreeExpr, TypedExpression left, NameExpr regexFieldName) {
+// we need to cast the left expression to CharSequence, otherwise it
might fail in cases of missing type safety
+// e.g. test method parameters (_this) is of a raw type Map
+EnclosedExpr typeSafeStringToMatch = new EnclosedExpr(new
CastExpr(toClassOrInterfaceType(CharSequence.class), left.getExpression()));
+
+MethodCallExpr matchesCallExpr = new MethodCallExpr(
+new MethodCallExpr(regexFieldName, "matcher",
NodeList.nodeList(typeSafeStringToMatch)),
+"matches"
+);
+
+// null string should not match
+BinaryExpr compoundRegexCallExpr = new BinaryExpr(
+new BinaryExpr(left.getExpression(), new NullLiteralExpr(),
BinaryExpr.Operator.NOT_EQUALS),
+pointFreeExpr.isNegated() ? new UnaryExpr(matchesCallExpr,
UnaryExpr.Operator.LOGICAL_COMPLEMENT) : matchesCallExpr,
+BinaryExpr.Operator.AND
+);
+
+return new EnclosedExpr(compoundRegexCallExpr);
+}
+
+private FieldDeclaration getClassMemberFieldDeclaration(NameExpr
regexFieldName, Expression regexExpression) {
+
+MethodCallExpr compiledRegexField = new MethodCallExpr(
+new NameExpr(Pattern.class.getName()), "compile",
NodeList.nodeList(regexExpression)
+);
+
+VariableDeclarator variableDeclarator = new VariableDeclarator(
+toClassOrInterfaceType(Pattern.class),
+regexFieldName.getName(),
+compiledRegexField
+);
+
+return new FieldDeclaration(
+NodeList.nodeList(Modifier.privateModifier(),
Modifier.staticModifier(), Modifier.finalModifier()),
+variableDeclarator
+);
+}
+
+/**
+ * Recursively check if the regexPatternExpression contains a
Re: [PR] Precompile regex pattern for 'matches' operator and make it part of the lambda predicate class [incubator-kie-drools]
tkobayas commented on PR #6469: URL: https://github.com/apache/incubator-kie-drools/pull/6469#issuecomment-3345481105 Thank you for the PR, @DimaSol As you see in the failed test cases, there are some things to consider. A) `MvelOperatorsTest.testMatchesWithFunction` : Dynamic operand like `name matches addStar(likes)` results in `regexp_failed_to_parse`. I think the "static regex pattern" approach is applicable only to non dynamic oprand (literal String). For dynamic cases, we still need `MatchesOperator`...? B) `MvelOperatorsTest.testMatchesOnNullString` etc. : Negative hash results in compilation error. It could be resolved by `Integer.toUnsignedString(s.hashCode())` Probably some more, but solving the above 2 points would help us to see other issues clearer. Btw, before going into that deep, how about just changing from `Collections.synchronizedMap` to `ConcurrentHashMap` in `MatchesOperator`? `ConcurrentHashMap` doesn't block concurrent `get`, it's match faster than `Collections.synchronizedMap` in the concurrent read use case. ``` Benchmark (_factsNumber) (_rulesNumber) (cacheEnabled) Mode Cnt Score Error Units MatchesOperatorMultiThreadBenchmark.test 256 8 truess 100 0.523 ± 0.096 ms/op MatchesOperatorMultiThreadBenchmark.test 256 8 falsess 100 1.032 ± 0.157 ms/op ``` If it also works for your rules, I think this is a better (easy) option. WDYT? -- 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] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] Precompile regex pattern for 'matches' operator and make it part of the lambda predicate class [incubator-kie-drools]
tkobayas commented on code in PR #6469:
URL:
https://github.com/apache/incubator-kie-drools/pull/6469#discussion_r2405444375
##
drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/operatorspec/RegexOperatorSpec.java:
##
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.drools.model.codegen.execmodel.generator.operatorspec;
+
+import com.github.javaparser.ast.Modifier;
+import com.github.javaparser.ast.NodeList;
+import com.github.javaparser.ast.body.FieldDeclaration;
+import com.github.javaparser.ast.body.VariableDeclarator;
+import com.github.javaparser.ast.expr.*;
+import org.drools.model.codegen.execmodel.generator.RuleContext;
+import org.drools.model.codegen.execmodel.generator.TypedExpression;
+import
org.drools.model.codegen.execmodel.generator.expressiontyper.ExpressionTyper;
+import org.drools.mvel.parser.ast.expr.PointFreeExpr;
+import org.drools.mvel.parser.ast.expr.RegexExpr;
+
+import java.util.regex.Pattern;
+
+import static
org.drools.model.codegen.execmodel.generator.DrlxParseUtil.toClassOrInterfaceType;
+import static org.drools.util.StringUtils.md5Hash;
+
+public class RegexOperatorSpec extends NativeOperatorSpec {
+public static final RegexOperatorSpec INSTANCE = new RegexOperatorSpec();
+
+public Expression getExpression(RuleContext context, PointFreeExpr
pointFreeExpr, TypedExpression left, ExpressionTyper expressionTyper) {
+if (shouldInlineRegex(pointFreeExpr.getRight())) {
+return super.getExpression(context, pointFreeExpr, left,
expressionTyper);
+} else {
+// TODO(?) throw some exception instead of creating
regexp_failed_to_parse expressions
+Expression regexExpression = pointFreeExpr.getRight().getFirst()
+.orElse(new StringLiteralExpr("regexp_failed_to_parse"));
+
+NameExpr regexFieldName = new NameExpr("regexp_" +
md5Hash(regexExpression.toString()));
+
+return new RegexExpr(
+getClassMemberFieldDeclaration(regexFieldName,
regexExpression),
+getClassMemberInvocationExpression(pointFreeExpr, left,
regexFieldName)
+);
+}
+}
+
+private EnclosedExpr getClassMemberInvocationExpression(PointFreeExpr
pointFreeExpr, TypedExpression left, NameExpr regexFieldName) {
+// we need to cast the left expression to CharSequence, otherwise it
might fail in cases of missing type safety
+// e.g. test method parameters (_this) is of a raw type Map
+EnclosedExpr typeSafeStringToMatch = new EnclosedExpr(new
CastExpr(toClassOrInterfaceType(CharSequence.class), left.getExpression()));
+
+MethodCallExpr matchesCallExpr = new MethodCallExpr(
+new MethodCallExpr(regexFieldName, "matcher",
NodeList.nodeList(typeSafeStringToMatch)),
+"matches"
+);
+
+// null string should not match
+BinaryExpr compoundRegexCallExpr = new BinaryExpr(
+new BinaryExpr(left.getExpression(), new NullLiteralExpr(),
BinaryExpr.Operator.NOT_EQUALS),
+pointFreeExpr.isNegated() ? new UnaryExpr(matchesCallExpr,
UnaryExpr.Operator.LOGICAL_COMPLEMENT) : matchesCallExpr,
+BinaryExpr.Operator.AND
+);
+
+return new EnclosedExpr(compoundRegexCallExpr);
+}
+
+private FieldDeclaration getClassMemberFieldDeclaration(NameExpr
regexFieldName, Expression regexExpression) {
+
+MethodCallExpr compiledRegexField = new MethodCallExpr(
+new NameExpr(Pattern.class.getName()), "compile",
NodeList.nodeList(regexExpression)
+);
+
+VariableDeclarator variableDeclarator = new VariableDeclarator(
+toClassOrInterfaceType(Pattern.class),
+regexFieldName.getName(),
+compiledRegexField
+);
+
+return new FieldDeclaration(
+NodeList.nodeList(Modifier.privateModifier(),
Modifier.staticModifier(), Modifier.finalModifier()),
+variableDeclarator
+);
+}
+
+/**
+ * Recursively check if the regexPatternExpression contains a
Re: [PR] Precompile regex pattern for 'matches' operator and make it part of the lambda predicate class [incubator-kie-drools]
tkobayas commented on PR #6469: URL: https://github.com/apache/incubator-kie-drools/pull/6469#issuecomment-3349839484 Okay, it's good to go with the direction. > My main open question is about the benefits of using D.eval in the lambda test method. Do you think there’s any advantage to going down this path for static regex as well, or would it be fine to use the class regex member directly without D.eval? D.eval is just to invoke `Operator.test`. RegexOperatorSpec should be fine with the current implementation without D.eval. -- 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] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] Precompile regex pattern for 'matches' operator and make it part of the lambda predicate class [incubator-kie-drools]
DimaSol commented on code in PR #6469:
URL:
https://github.com/apache/incubator-kie-drools/pull/6469#discussion_r2402859785
##
drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/operatorspec/RegexOperatorSpec.java:
##
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.drools.model.codegen.execmodel.generator.operatorspec;
+
+import com.github.javaparser.ast.Modifier;
+import com.github.javaparser.ast.NodeList;
+import com.github.javaparser.ast.body.FieldDeclaration;
+import com.github.javaparser.ast.body.VariableDeclarator;
+import com.github.javaparser.ast.expr.*;
+import org.drools.model.codegen.execmodel.generator.RuleContext;
+import org.drools.model.codegen.execmodel.generator.TypedExpression;
+import
org.drools.model.codegen.execmodel.generator.expressiontyper.ExpressionTyper;
+import org.drools.mvel.parser.ast.expr.PointFreeExpr;
+import org.drools.mvel.parser.ast.expr.RegexExpr;
+
+import java.util.regex.Pattern;
+
+import static
org.drools.model.codegen.execmodel.generator.DrlxParseUtil.toClassOrInterfaceType;
+import static org.drools.util.StringUtils.md5Hash;
+
+public class RegexOperatorSpec extends NativeOperatorSpec {
+public static final RegexOperatorSpec INSTANCE = new RegexOperatorSpec();
+
+public Expression getExpression(RuleContext context, PointFreeExpr
pointFreeExpr, TypedExpression left, ExpressionTyper expressionTyper) {
+if (shouldInlineRegex(pointFreeExpr.getRight())) {
+return super.getExpression(context, pointFreeExpr, left,
expressionTyper);
+} else {
+// TODO(?) throw some exception instead of creating
regexp_failed_to_parse expressions
+Expression regexExpression = pointFreeExpr.getRight().getFirst()
+.orElse(new StringLiteralExpr("regexp_failed_to_parse"));
+
+NameExpr regexFieldName = new NameExpr("regexp_" +
md5Hash(regexExpression.toString()));
+
+return new RegexExpr(
+getClassMemberFieldDeclaration(regexFieldName,
regexExpression),
+getClassMemberInvocationExpression(pointFreeExpr, left,
regexFieldName)
+);
+}
+}
+
+private EnclosedExpr getClassMemberInvocationExpression(PointFreeExpr
pointFreeExpr, TypedExpression left, NameExpr regexFieldName) {
+// we need to cast the left expression to CharSequence, otherwise it
might fail in cases of missing type safety
+// e.g. test method parameters (_this) is of a raw type Map
+EnclosedExpr typeSafeStringToMatch = new EnclosedExpr(new
CastExpr(toClassOrInterfaceType(CharSequence.class), left.getExpression()));
+
+MethodCallExpr matchesCallExpr = new MethodCallExpr(
+new MethodCallExpr(regexFieldName, "matcher",
NodeList.nodeList(typeSafeStringToMatch)),
+"matches"
+);
+
+// null string should not match
+BinaryExpr compoundRegexCallExpr = new BinaryExpr(
+new BinaryExpr(left.getExpression(), new NullLiteralExpr(),
BinaryExpr.Operator.NOT_EQUALS),
+pointFreeExpr.isNegated() ? new UnaryExpr(matchesCallExpr,
UnaryExpr.Operator.LOGICAL_COMPLEMENT) : matchesCallExpr,
+BinaryExpr.Operator.AND
+);
+
+return new EnclosedExpr(compoundRegexCallExpr);
+}
+
+private FieldDeclaration getClassMemberFieldDeclaration(NameExpr
regexFieldName, Expression regexExpression) {
+
+MethodCallExpr compiledRegexField = new MethodCallExpr(
+new NameExpr(Pattern.class.getName()), "compile",
NodeList.nodeList(regexExpression)
+);
+
+VariableDeclarator variableDeclarator = new VariableDeclarator(
+toClassOrInterfaceType(Pattern.class),
+regexFieldName.getName(),
+compiledRegexField
+);
+
+return new FieldDeclaration(
+NodeList.nodeList(Modifier.privateModifier(),
Modifier.staticModifier(), Modifier.finalModifier()),
+variableDeclarator
+);
+}
+
+/**
+ * Recursively check if the regexPatternExpression contains a
Re: [PR] Precompile regex pattern for 'matches' operator and make it part of the lambda predicate class [incubator-kie-drools]
tkobayas commented on PR #6469: URL: https://github.com/apache/incubator-kie-drools/pull/6469#issuecomment-3370652909 @mariofusco @Rikkola Please review, thanks! -- 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] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] Precompile regex pattern for 'matches' operator and make it part of the lambda predicate class [incubator-kie-drools]
tkobayas commented on PR #6469: URL: https://github.com/apache/incubator-kie-drools/pull/6469#issuecomment-3375267747 Benchmark result: ``` Benchmark (_factsNumber) (_rulesNumber) (cacheEnabled) Mode Cnt Score Error Units MatchesOperatorMultiThreadBenchmark.test 256 8 truess 100 0.463 ± 0.048 ms/op MatchesOperatorMultiThreadBenchmark.test 256 8 falsess 100 1.067 ± 0.151 ms/op ``` Here `cacheEnabled` is used to enable the regex precompile (not "drools.matches.compiled.cache.count"). -- 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] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] Precompile regex pattern for 'matches' operator and make it part of the lambda predicate class [incubator-kie-drools]
kie-ci3 commented on PR #6469:
URL:
https://github.com/apache/incubator-kie-drools/pull/6469#issuecomment-3348871323
**PR job** `#2` was: **UNSTABLE**
Possible explanation: This should be test failures
Reproducer
build-chain build full_downstream -f
'https://raw.githubusercontent.com/${AUTHOR:apache}/incubator-kie-kogito-pipelines/${BRANCH:main}/.ci/buildchain-config-pr-cdb.yaml'
-o 'bc' -p apache/incubator-kie-drools -u
https://github.com/apache/incubator-kie-drools/pull/6469 --skipParallelCheckout
NOTE: To install the build-chain tool, please refer to
https://github.com/kiegroup/github-action-build-chain#local-execution
Please look here:
https://ci-builds.apache.org/job/KIE/job/drools/job/main/job/pullrequest_jobs/job/drools-pr/job/PR-6469/2/display/redirect
**Test results:**
- PASSED: 23849
- FAILED: 18
Those are the test failures:
https://ci-builds.apache.org/job/KIE/job/drools/job/main/job/pullrequest_jobs/job/drools-pr/job/PR-6469/2/testReport/org.drools.compiler.integrationtests.operators/EvalRewriteTest/testEvalRewriteMatches(KieBaseTestConfiguration)[2]/">org.drools.compiler.integrationtests.operators.EvalRewriteTest.testEvalRewriteMatches(KieBaseTestConfiguration)[2]
expected: 2 but was: 1
https://ci-builds.apache.org/job/KIE/job/drools/job/main/job/pullrequest_jobs/job/drools-pr/job/PR-6469/2/testReport/org.drools.compiler.integrationtests.operators/MatchesTest/testMatchesMVEL2(KieBaseTestConfiguration)[2]/">org.drools.compiler.integrationtests.operators.MatchesTest.testMatchesMVEL2(KieBaseTestConfiguration)[2]
[[Message [id=1, level=ERROR,
path=src/main/java/org/drools/compiler/integrationtests/operators/PD4/LambdaPredicateD43379AA7DBCF6D747E734A3787A1A81.java,
line=24, column=959 text=The method matcher(CharSequence) in the type
Pattern is not applicable for the arguments (Object)], Message [id=2,
level=ERROR,
path=src/main/java/org/drools/compiler/integrationtests/operators/PD4/LambdaPredicateD43379AA7DBCF6D747E734A3787A1A81.java,
line=0, column=0 text=Java source of
src/main/java/org/drools/compiler/integrationtests/operators/PD4/LambdaPredicateD43379AA7DBCF6D747E734A3787A1A81.java
in error:package
org.drools.compiler.integrationtests.operators.PD4;import static
org.drools.compiler.integrationtests.operators.RulesF1046587C8E74AB2592535F9A160317A.*;import
org.drools.compiler.integrationtests.operators.*;import
java.util.Map;import
org.drools.modelcompiler.dsl.pattern.D;@org.drools.compiler.kie.builder.MaterializedLambda()
public enum LambdaPredicateD43379AA7DBCF6D747E734A3787A1A81 implements
org.drools.model.functions.Predicate1,
org.drools.model.functions.HashedExpression {INSTANCE;
public static final String EXPRESSION_HASH =
"41458643CFD03EC531D80C079D39604E";public java.lang.String
getExpressionHash() {return EXPRESSION_HASH;}
private static final java.util.regex.Pattern regexp =
java.util.regex.Pattern.compile("[^\\.]*\\(.*");@Override()
public boolean test(java.util.Map _this) throws java.lang.Exception {
return _this.get("content") != null &&
regexp.matcher(_this.get("content")).matches();}
@Override()public org.drools.model.functions.PredicateInformation
predicateInformation() {
org.drools.model.functions.PredicateInformation info = new
org.drools.model.functions.PredicateInformation("this[\"content\"] matches
\".*..*(.
*\"");info.addRuleNames("Matches mvel", "rules1.drl", "Matches
mvel 2", "rules1.drl", "Matches mvel 3", "rules1.drl");return
info;}}], Message [id=3, level=ERROR,
path=src/main/java/org/drools/compiler/integrationtests/operators/PD1/LambdaPredicateD13CB8D8B72164203579AD67C8535D22.java,
line=24, column=959 text=The method matcher(CharSequence) in the type
Pattern is not applicable for the arguments (Object)], Message [id=4,
level=ERROR,
path=src/main/java/org/drools/compiler/integrationtests/operators/PD1/LambdaPredicateD13CB8D8B72164203579AD67C8535D22.java,
line=0, column=0 text=Java source of
src/main/java/org/drools/compiler/integrationtests/operators/PD1/LambdaPredicateD13CB8D8B72164203579AD67C8535D22.java
in error:package
org.drools.compiler.integrationtests.operators.PD1;import static
org.drools.compiler.integrationtests.operators.RulesF1046587C8E74AB2592535F9A160317A.*;import
org.drools.compile
r.integrationtests.operators.*;import java.util.Map;import
org.drools.modelcompiler.dsl.pattern.D;@org.drools.compiler.kie.builder.MaterializedLambda()public
enum LambdaPredicateD13CB8D8B72164203579AD67C8535D22 implements
org.drools.model.functions.Predicate1,
org.drools.model.functions.HashedExpression {INSTANCE;
public static final String EXPRESSION_HASH =
"41458643CFD03EC531D80C079D39604E";public java.lang.String
getExpressionHash() {return EXPRESSION_HASH;}
private static final jav
Re: [PR] Precompile regex pattern for 'matches' operator and make it part of the lambda predicate class [incubator-kie-drools]
tkobayas commented on PR #6469:
URL:
https://github.com/apache/incubator-kie-drools/pull/6469#issuecomment-3350004666
Btw,
> I find the current caching approach even more concerning: this map could
grow unbounded over time
The map is limited to `MAX_SIZE_CACHE` with the insertion order (not LRU
though). Am I missing something?
```
private static final Map patternMap =
Collections.synchronizedMap(new LinkedHashMap<>() {
@Override
protected boolean removeEldestEntry(Map.Entry
eldest) {
return size() > (MAX_SIZE_CACHE);
}
});
```
--
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]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] Precompile regex pattern for 'matches' operator and make it part of the lambda predicate class [incubator-kie-drools]
Copilot commented on code in PR #6469:
URL:
https://github.com/apache/incubator-kie-drools/pull/6469#discussion_r2397728169
##
drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/util/lambdareplace/MaterializedLambdaPredicate.java:
##
@@ -76,6 +79,22 @@ private void createTestMethod(EnumDeclaration
classDeclaration) {
methodDeclaration.setBody(new BlockStmt(NodeList.nodeList(new
ReturnStmt(clone.getExpression();
}
+/**
+ * recursively traverse the node tree and collect all RegexExpr
+ * @param node
+ * @return
+ */
+private Collection collectRegexExpressions(Node node) {
+if (node instanceof RegexExpr) {
+return Collections.singletonList((RegexExpr) node);
+} else {
+return node.getChildNodes().stream()
+.map(this::collectRegexExpressions)
+.flatMap(Collection::stream)
+.toList();
Review Comment:
Stream.toList() requires Java 16+. Drools builds target Java 11; this will
not compile there. Replace with Collectors.toList() and add import for
java.util.stream.Collectors.
##
drools-model/drools-mvel-parser/src/main/java/org/drools/mvel/parser/ast/expr/RegexExpr.java:
##
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.drools.mvel.parser.ast.expr;
+
+import com.github.javaparser.ast.AllFieldsConstructor;
+import com.github.javaparser.ast.Node;
+import com.github.javaparser.ast.body.FieldDeclaration;
+import com.github.javaparser.ast.expr.BinaryExpr;
Review Comment:
Unused import: BinaryExpr is not referenced in this class. Please remove to
avoid warnings.
```suggestion
```
##
drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/operatorspec/RegexOperatorSpec.java:
##
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.drools.model.codegen.execmodel.generator.operatorspec;
+
+import com.github.javaparser.ast.Modifier;
+import com.github.javaparser.ast.NodeList;
+import com.github.javaparser.ast.body.FieldDeclaration;
+import com.github.javaparser.ast.body.VariableDeclarator;
+import com.github.javaparser.ast.expr.*;
+import org.drools.model.codegen.execmodel.generator.RuleContext;
+import org.drools.model.codegen.execmodel.generator.TypedExpression;
+import
org.drools.model.codegen.execmodel.generator.expressiontyper.ExpressionTyper;
+import org.drools.mvel.parser.ast.expr.PointFreeExpr;
+import org.drools.mvel.parser.ast.expr.RegexExpr;
+
+import java.util.regex.Pattern;
+
+import static
org.drools.model.codegen.execmodel.generator.DrlxParseUtil.toClassOrInterfaceType;
+import static org.drools.util.StringUtils.md5Hash;
+
+public class RegexOperatorSpec extends NativeOperatorSpec {
+public static final RegexOperatorSpec INSTANCE = new RegexOperatorSpec();
+
+public Expression getExpression(RuleContext context, PointFreeExpr
pointFreeExpr, TypedExpression left, ExpressionTyper expressionTyper) {
+if (shouldInlineRegex(pointFreeExpr.getRight())) {
+return super.getExpression(context, pointFreeExpr, left,
expressionTyper);
+} else {
+// TODO(?) throw some exception instead of creating
regexp_failed_to_parse expressions
+Expression regexExpression = pointFreeExpr.getRight().getFi
Re: [PR] Precompile regex pattern for 'matches' operator and make it part of the lambda predicate class [incubator-kie-drools]
tkobayas merged PR #6469: URL: https://github.com/apache/incubator-kie-drools/pull/6469 -- 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] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] Precompile regex pattern for 'matches' operator and make it part of the lambda predicate class [incubator-kie-drools]
DimaSol commented on code in PR #6469:
URL:
https://github.com/apache/incubator-kie-drools/pull/6469#discussion_r2402859785
##
drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/operatorspec/RegexOperatorSpec.java:
##
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.drools.model.codegen.execmodel.generator.operatorspec;
+
+import com.github.javaparser.ast.Modifier;
+import com.github.javaparser.ast.NodeList;
+import com.github.javaparser.ast.body.FieldDeclaration;
+import com.github.javaparser.ast.body.VariableDeclarator;
+import com.github.javaparser.ast.expr.*;
+import org.drools.model.codegen.execmodel.generator.RuleContext;
+import org.drools.model.codegen.execmodel.generator.TypedExpression;
+import
org.drools.model.codegen.execmodel.generator.expressiontyper.ExpressionTyper;
+import org.drools.mvel.parser.ast.expr.PointFreeExpr;
+import org.drools.mvel.parser.ast.expr.RegexExpr;
+
+import java.util.regex.Pattern;
+
+import static
org.drools.model.codegen.execmodel.generator.DrlxParseUtil.toClassOrInterfaceType;
+import static org.drools.util.StringUtils.md5Hash;
+
+public class RegexOperatorSpec extends NativeOperatorSpec {
+public static final RegexOperatorSpec INSTANCE = new RegexOperatorSpec();
+
+public Expression getExpression(RuleContext context, PointFreeExpr
pointFreeExpr, TypedExpression left, ExpressionTyper expressionTyper) {
+if (shouldInlineRegex(pointFreeExpr.getRight())) {
+return super.getExpression(context, pointFreeExpr, left,
expressionTyper);
+} else {
+// TODO(?) throw some exception instead of creating
regexp_failed_to_parse expressions
+Expression regexExpression = pointFreeExpr.getRight().getFirst()
+.orElse(new StringLiteralExpr("regexp_failed_to_parse"));
+
+NameExpr regexFieldName = new NameExpr("regexp_" +
md5Hash(regexExpression.toString()));
+
+return new RegexExpr(
+getClassMemberFieldDeclaration(regexFieldName,
regexExpression),
+getClassMemberInvocationExpression(pointFreeExpr, left,
regexFieldName)
+);
+}
+}
+
+private EnclosedExpr getClassMemberInvocationExpression(PointFreeExpr
pointFreeExpr, TypedExpression left, NameExpr regexFieldName) {
+// we need to cast the left expression to CharSequence, otherwise it
might fail in cases of missing type safety
+// e.g. test method parameters (_this) is of a raw type Map
+EnclosedExpr typeSafeStringToMatch = new EnclosedExpr(new
CastExpr(toClassOrInterfaceType(CharSequence.class), left.getExpression()));
+
+MethodCallExpr matchesCallExpr = new MethodCallExpr(
+new MethodCallExpr(regexFieldName, "matcher",
NodeList.nodeList(typeSafeStringToMatch)),
+"matches"
+);
+
+// null string should not match
+BinaryExpr compoundRegexCallExpr = new BinaryExpr(
+new BinaryExpr(left.getExpression(), new NullLiteralExpr(),
BinaryExpr.Operator.NOT_EQUALS),
+pointFreeExpr.isNegated() ? new UnaryExpr(matchesCallExpr,
UnaryExpr.Operator.LOGICAL_COMPLEMENT) : matchesCallExpr,
+BinaryExpr.Operator.AND
+);
+
+return new EnclosedExpr(compoundRegexCallExpr);
+}
+
+private FieldDeclaration getClassMemberFieldDeclaration(NameExpr
regexFieldName, Expression regexExpression) {
+
+MethodCallExpr compiledRegexField = new MethodCallExpr(
+new NameExpr(Pattern.class.getName()), "compile",
NodeList.nodeList(regexExpression)
+);
+
+VariableDeclarator variableDeclarator = new VariableDeclarator(
+toClassOrInterfaceType(Pattern.class),
+regexFieldName.getName(),
+compiledRegexField
+);
+
+return new FieldDeclaration(
+NodeList.nodeList(Modifier.privateModifier(),
Modifier.staticModifier(), Modifier.finalModifier()),
+variableDeclarator
+);
+}
+
+/**
+ * Recursively check if the regexPatternExpression contains a
Re: [PR] Precompile regex pattern for 'matches' operator and make it part of the lambda predicate class [incubator-kie-drools]
DimaSol commented on PR #6469: URL: https://github.com/apache/incubator-kie-drools/pull/6469#issuecomment-3347798498 Thanks @tkobayas. I’ve seen the failures and I’m planning to work on a fix to properly distinguish between static and dynamic expressions. Initially, I overlooked the dynamic expression use case, but I find the current caching approach even more concerning: this map could grow unbounded over time, leading to significant memory usage at scale (e.g., millions of facts with many dynamic values). That’s why I’d prefer to finalize the implementation I started. Even if we keep caching for dynamic regex expressions, it should at least use an LRU cache with strict, configurable limits. My main open question is about the benefits of using D.eval in the lambda test method. Do you think there’s any advantage to going down this path for static regex as well, or would it be fine to use the class regex member directly without D.eval? -- 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] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] Precompile regex pattern for 'matches' operator and make it part of the lambda predicate class [incubator-kie-drools]
tkobayas commented on PR #6469: URL: https://github.com/apache/incubator-kie-drools/pull/6469#issuecomment-3360071968 Thank you for updating. Please check comments from Copilot and me, thanks! -- 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] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] Precompile regex pattern for 'matches' operator and make it part of the lambda predicate class [incubator-kie-drools]
tkobayas commented on code in PR #6469: URL: https://github.com/apache/incubator-kie-drools/pull/6469#discussion_r2397909427 ## drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/operatorspec/RegexOperatorSpec.java: ## @@ -0,0 +1,120 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.drools.model.codegen.execmodel.generator.operatorspec; + +import com.github.javaparser.ast.Modifier; +import com.github.javaparser.ast.NodeList; +import com.github.javaparser.ast.body.FieldDeclaration; +import com.github.javaparser.ast.body.VariableDeclarator; +import com.github.javaparser.ast.expr.*; Review Comment: please avoid a wildcard in import. Thanks. -- 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] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] Precompile regex pattern for 'matches' operator and make it part of the lambda predicate class [incubator-kie-drools]
tkobayas commented on code in PR #6469:
URL:
https://github.com/apache/incubator-kie-drools/pull/6469#discussion_r2397862964
##
drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/operatorspec/RegexOperatorSpec.java:
##
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.drools.model.codegen.execmodel.generator.operatorspec;
+
+import com.github.javaparser.ast.Modifier;
+import com.github.javaparser.ast.NodeList;
+import com.github.javaparser.ast.body.FieldDeclaration;
+import com.github.javaparser.ast.body.VariableDeclarator;
+import com.github.javaparser.ast.expr.*;
+import org.drools.model.codegen.execmodel.generator.RuleContext;
+import org.drools.model.codegen.execmodel.generator.TypedExpression;
+import
org.drools.model.codegen.execmodel.generator.expressiontyper.ExpressionTyper;
+import org.drools.mvel.parser.ast.expr.PointFreeExpr;
+import org.drools.mvel.parser.ast.expr.RegexExpr;
+
+import java.util.regex.Pattern;
+
+import static
org.drools.model.codegen.execmodel.generator.DrlxParseUtil.toClassOrInterfaceType;
+import static org.drools.util.StringUtils.md5Hash;
+
+public class RegexOperatorSpec extends NativeOperatorSpec {
+public static final RegexOperatorSpec INSTANCE = new RegexOperatorSpec();
+
+public Expression getExpression(RuleContext context, PointFreeExpr
pointFreeExpr, TypedExpression left, ExpressionTyper expressionTyper) {
+if (shouldInlineRegex(pointFreeExpr.getRight())) {
+return super.getExpression(context, pointFreeExpr, left,
expressionTyper);
+} else {
+// TODO(?) throw some exception instead of creating
regexp_failed_to_parse expressions
+Expression regexExpression = pointFreeExpr.getRight().getFirst()
+.orElse(new StringLiteralExpr("regexp_failed_to_parse"));
+
+NameExpr regexFieldName = new NameExpr("regexp_" +
md5Hash(regexExpression.toString()));
+
+return new RegexExpr(
+getClassMemberFieldDeclaration(regexFieldName,
regexExpression),
+getClassMemberInvocationExpression(pointFreeExpr, left,
regexFieldName)
+);
+}
+}
+
+private EnclosedExpr getClassMemberInvocationExpression(PointFreeExpr
pointFreeExpr, TypedExpression left, NameExpr regexFieldName) {
+// we need to cast the left expression to CharSequence, otherwise it
might fail in cases of missing type safety
+// e.g. test method parameters (_this) is of a raw type Map
+EnclosedExpr typeSafeStringToMatch = new EnclosedExpr(new
CastExpr(toClassOrInterfaceType(CharSequence.class), left.getExpression()));
+
+MethodCallExpr matchesCallExpr = new MethodCallExpr(
+new MethodCallExpr(regexFieldName, "matcher",
NodeList.nodeList(typeSafeStringToMatch)),
+"matches"
+);
+
+// null string should not match
+BinaryExpr compoundRegexCallExpr = new BinaryExpr(
+new BinaryExpr(left.getExpression(), new NullLiteralExpr(),
BinaryExpr.Operator.NOT_EQUALS),
+pointFreeExpr.isNegated() ? new UnaryExpr(matchesCallExpr,
UnaryExpr.Operator.LOGICAL_COMPLEMENT) : matchesCallExpr,
+BinaryExpr.Operator.AND
+);
+
+return new EnclosedExpr(compoundRegexCallExpr);
+}
+
+private FieldDeclaration getClassMemberFieldDeclaration(NameExpr
regexFieldName, Expression regexExpression) {
+
+MethodCallExpr compiledRegexField = new MethodCallExpr(
+new NameExpr(Pattern.class.getName()), "compile",
NodeList.nodeList(regexExpression)
+);
+
+VariableDeclarator variableDeclarator = new VariableDeclarator(
+toClassOrInterfaceType(Pattern.class),
+regexFieldName.getName(),
+compiledRegexField
+);
+
+return new FieldDeclaration(
+NodeList.nodeList(Modifier.privateModifier(),
Modifier.staticModifier(), Modifier.finalModifier()),
+variableDeclarator
+);
+}
+
+/**
+ * Recursively check if the regexPatternExpression contains a
Re: [PR] Precompile regex pattern for 'matches' operator and make it part of the lambda predicate class [incubator-kie-drools]
tkobayas commented on code in PR #6469:
URL:
https://github.com/apache/incubator-kie-drools/pull/6469#discussion_r2397869503
##
drools-model/drools-mvel-parser/src/main/java/org/drools/mvel/parser/ast/expr/RegexExpr.java:
##
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.drools.mvel.parser.ast.expr;
+
+import com.github.javaparser.ast.AllFieldsConstructor;
+import com.github.javaparser.ast.Node;
+import com.github.javaparser.ast.body.FieldDeclaration;
+import com.github.javaparser.ast.expr.BinaryExpr;
+import com.github.javaparser.ast.expr.EnclosedExpr;
+import com.github.javaparser.ast.expr.LiteralExpr;
+import com.github.javaparser.ast.visitor.CloneVisitor;
+import com.github.javaparser.ast.visitor.GenericVisitor;
+import com.github.javaparser.ast.visitor.VoidVisitor;
+
+public final class RegexExpr extends LiteralExpr {
+
+private final EnclosedExpr matchesEvaluationExpr;
+private final FieldDeclaration compiledRegexMember;
+
+@AllFieldsConstructor
+public RegexExpr(FieldDeclaration compiledRegexMember, EnclosedExpr
matchesEvaluationExpr) {
+super();
+
+this.matchesEvaluationExpr = matchesEvaluationExpr;
+this.compiledRegexMember = compiledRegexMember;
+}
Review Comment:
I think you can ignore this suggestion. `@AllFieldsConstructor` doesn't do
harm at least for now.
--
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]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] Precompile regex pattern for 'matches' operator and make it part of the lambda predicate class [incubator-kie-drools]
tkobayas commented on code in PR #6469:
URL:
https://github.com/apache/incubator-kie-drools/pull/6469#discussion_r2397903466
##
drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/operatorspec/RegexOperatorSpec.java:
##
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.drools.model.codegen.execmodel.generator.operatorspec;
+
+import com.github.javaparser.ast.Modifier;
+import com.github.javaparser.ast.NodeList;
+import com.github.javaparser.ast.body.FieldDeclaration;
+import com.github.javaparser.ast.body.VariableDeclarator;
+import com.github.javaparser.ast.expr.*;
+import org.drools.model.codegen.execmodel.generator.RuleContext;
+import org.drools.model.codegen.execmodel.generator.TypedExpression;
+import
org.drools.model.codegen.execmodel.generator.expressiontyper.ExpressionTyper;
+import org.drools.mvel.parser.ast.expr.PointFreeExpr;
+import org.drools.mvel.parser.ast.expr.RegexExpr;
+
+import java.util.regex.Pattern;
+
+import static
org.drools.model.codegen.execmodel.generator.DrlxParseUtil.toClassOrInterfaceType;
+import static org.drools.util.StringUtils.md5Hash;
+
+public class RegexOperatorSpec extends NativeOperatorSpec {
+public static final RegexOperatorSpec INSTANCE = new RegexOperatorSpec();
+
+public Expression getExpression(RuleContext context, PointFreeExpr
pointFreeExpr, TypedExpression left, ExpressionTyper expressionTyper) {
+if (shouldInlineRegex(pointFreeExpr.getRight())) {
+return super.getExpression(context, pointFreeExpr, left,
expressionTyper);
+} else {
+// TODO(?) throw some exception instead of creating
regexp_failed_to_parse expressions
+Expression regexExpression = pointFreeExpr.getRight().getFirst()
+.orElse(new StringLiteralExpr("regexp_failed_to_parse"));
+
+NameExpr regexFieldName = new NameExpr("regexp_" +
md5Hash(regexExpression.toString()));
+
+return new RegexExpr(
+getClassMemberFieldDeclaration(regexFieldName,
regexExpression),
+getClassMemberInvocationExpression(pointFreeExpr, left,
regexFieldName)
+);
+}
+}
+
+private EnclosedExpr getClassMemberInvocationExpression(PointFreeExpr
pointFreeExpr, TypedExpression left, NameExpr regexFieldName) {
+// we need to cast the left expression to CharSequence, otherwise it
might fail in cases of missing type safety
+// e.g. test method parameters (_this) is of a raw type Map
+EnclosedExpr typeSafeStringToMatch = new EnclosedExpr(new
CastExpr(toClassOrInterfaceType(CharSequence.class), left.getExpression()));
+
+MethodCallExpr matchesCallExpr = new MethodCallExpr(
+new MethodCallExpr(regexFieldName, "matcher",
NodeList.nodeList(typeSafeStringToMatch)),
+"matches"
+);
+
+// null string should not match
+BinaryExpr compoundRegexCallExpr = new BinaryExpr(
+new BinaryExpr(left.getExpression(), new NullLiteralExpr(),
BinaryExpr.Operator.NOT_EQUALS),
+pointFreeExpr.isNegated() ? new UnaryExpr(matchesCallExpr,
UnaryExpr.Operator.LOGICAL_COMPLEMENT) : matchesCallExpr,
+BinaryExpr.Operator.AND
+);
+
+return new EnclosedExpr(compoundRegexCallExpr);
+}
+
+private FieldDeclaration getClassMemberFieldDeclaration(NameExpr
regexFieldName, Expression regexExpression) {
+
+MethodCallExpr compiledRegexField = new MethodCallExpr(
+new NameExpr(Pattern.class.getName()), "compile",
NodeList.nodeList(regexExpression)
+);
+
+VariableDeclarator variableDeclarator = new VariableDeclarator(
+toClassOrInterfaceType(Pattern.class),
+regexFieldName.getName(),
+compiledRegexField
+);
+
+return new FieldDeclaration(
+NodeList.nodeList(Modifier.privateModifier(),
Modifier.staticModifier(), Modifier.finalModifier()),
+variableDeclarator
+);
+}
+
+/**
+ * Recursively check if the regexPatternExpression contains a
Re: [PR] Precompile regex pattern for 'matches' operator and make it part of the lambda predicate class [incubator-kie-drools]
tkobayas commented on code in PR #6469:
URL:
https://github.com/apache/incubator-kie-drools/pull/6469#discussion_r2397876507
##
drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/operatorspec/RegexOperatorSpec.java:
##
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.drools.model.codegen.execmodel.generator.operatorspec;
+
+import com.github.javaparser.ast.Modifier;
+import com.github.javaparser.ast.NodeList;
+import com.github.javaparser.ast.body.FieldDeclaration;
+import com.github.javaparser.ast.body.VariableDeclarator;
+import com.github.javaparser.ast.expr.*;
+import org.drools.model.codegen.execmodel.generator.RuleContext;
+import org.drools.model.codegen.execmodel.generator.TypedExpression;
+import
org.drools.model.codegen.execmodel.generator.expressiontyper.ExpressionTyper;
+import org.drools.mvel.parser.ast.expr.PointFreeExpr;
+import org.drools.mvel.parser.ast.expr.RegexExpr;
+
+import java.util.regex.Pattern;
+
+import static
org.drools.model.codegen.execmodel.generator.DrlxParseUtil.toClassOrInterfaceType;
+import static org.drools.util.StringUtils.md5Hash;
+
+public class RegexOperatorSpec extends NativeOperatorSpec {
+public static final RegexOperatorSpec INSTANCE = new RegexOperatorSpec();
+
+public Expression getExpression(RuleContext context, PointFreeExpr
pointFreeExpr, TypedExpression left, ExpressionTyper expressionTyper) {
+if (shouldInlineRegex(pointFreeExpr.getRight())) {
+return super.getExpression(context, pointFreeExpr, left,
expressionTyper);
+} else {
+// TODO(?) throw some exception instead of creating
regexp_failed_to_parse expressions
+Expression regexExpression = pointFreeExpr.getRight().getFirst()
+.orElse(new StringLiteralExpr("regexp_failed_to_parse"));
Review Comment:
`pointFreeExpr.getRight()` should not be empty, so please throw
`IllegalStateException` in that case.
--
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]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] Precompile regex pattern for 'matches' operator and make it part of the lambda predicate class [incubator-kie-drools]
tkobayas commented on code in PR #6469:
URL:
https://github.com/apache/incubator-kie-drools/pull/6469#discussion_r2397862964
##
drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/operatorspec/RegexOperatorSpec.java:
##
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.drools.model.codegen.execmodel.generator.operatorspec;
+
+import com.github.javaparser.ast.Modifier;
+import com.github.javaparser.ast.NodeList;
+import com.github.javaparser.ast.body.FieldDeclaration;
+import com.github.javaparser.ast.body.VariableDeclarator;
+import com.github.javaparser.ast.expr.*;
+import org.drools.model.codegen.execmodel.generator.RuleContext;
+import org.drools.model.codegen.execmodel.generator.TypedExpression;
+import
org.drools.model.codegen.execmodel.generator.expressiontyper.ExpressionTyper;
+import org.drools.mvel.parser.ast.expr.PointFreeExpr;
+import org.drools.mvel.parser.ast.expr.RegexExpr;
+
+import java.util.regex.Pattern;
+
+import static
org.drools.model.codegen.execmodel.generator.DrlxParseUtil.toClassOrInterfaceType;
+import static org.drools.util.StringUtils.md5Hash;
+
+public class RegexOperatorSpec extends NativeOperatorSpec {
+public static final RegexOperatorSpec INSTANCE = new RegexOperatorSpec();
+
+public Expression getExpression(RuleContext context, PointFreeExpr
pointFreeExpr, TypedExpression left, ExpressionTyper expressionTyper) {
+if (shouldInlineRegex(pointFreeExpr.getRight())) {
+return super.getExpression(context, pointFreeExpr, left,
expressionTyper);
+} else {
+// TODO(?) throw some exception instead of creating
regexp_failed_to_parse expressions
+Expression regexExpression = pointFreeExpr.getRight().getFirst()
+.orElse(new StringLiteralExpr("regexp_failed_to_parse"));
+
+NameExpr regexFieldName = new NameExpr("regexp_" +
md5Hash(regexExpression.toString()));
+
+return new RegexExpr(
+getClassMemberFieldDeclaration(regexFieldName,
regexExpression),
+getClassMemberInvocationExpression(pointFreeExpr, left,
regexFieldName)
+);
+}
+}
+
+private EnclosedExpr getClassMemberInvocationExpression(PointFreeExpr
pointFreeExpr, TypedExpression left, NameExpr regexFieldName) {
+// we need to cast the left expression to CharSequence, otherwise it
might fail in cases of missing type safety
+// e.g. test method parameters (_this) is of a raw type Map
+EnclosedExpr typeSafeStringToMatch = new EnclosedExpr(new
CastExpr(toClassOrInterfaceType(CharSequence.class), left.getExpression()));
+
+MethodCallExpr matchesCallExpr = new MethodCallExpr(
+new MethodCallExpr(regexFieldName, "matcher",
NodeList.nodeList(typeSafeStringToMatch)),
+"matches"
+);
+
+// null string should not match
+BinaryExpr compoundRegexCallExpr = new BinaryExpr(
+new BinaryExpr(left.getExpression(), new NullLiteralExpr(),
BinaryExpr.Operator.NOT_EQUALS),
+pointFreeExpr.isNegated() ? new UnaryExpr(matchesCallExpr,
UnaryExpr.Operator.LOGICAL_COMPLEMENT) : matchesCallExpr,
+BinaryExpr.Operator.AND
+);
+
+return new EnclosedExpr(compoundRegexCallExpr);
+}
+
+private FieldDeclaration getClassMemberFieldDeclaration(NameExpr
regexFieldName, Expression regexExpression) {
+
+MethodCallExpr compiledRegexField = new MethodCallExpr(
+new NameExpr(Pattern.class.getName()), "compile",
NodeList.nodeList(regexExpression)
+);
+
+VariableDeclarator variableDeclarator = new VariableDeclarator(
+toClassOrInterfaceType(Pattern.class),
+regexFieldName.getName(),
+compiledRegexField
+);
+
+return new FieldDeclaration(
+NodeList.nodeList(Modifier.privateModifier(),
Modifier.staticModifier(), Modifier.finalModifier()),
+variableDeclarator
+);
+}
+
+/**
+ * Recursively check if the regexPatternExpression contains a
Re: [PR] Precompile regex pattern for 'matches' operator and make it part of the lambda predicate class [incubator-kie-drools]
tkobayas commented on code in PR #6469:
URL:
https://github.com/apache/incubator-kie-drools/pull/6469#discussion_r2397811145
##
drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/util/lambdareplace/MaterializedLambdaPredicate.java:
##
@@ -76,6 +79,22 @@ private void createTestMethod(EnumDeclaration
classDeclaration) {
methodDeclaration.setBody(new BlockStmt(NodeList.nodeList(new
ReturnStmt(clone.getExpression();
}
+/**
+ * recursively traverse the node tree and collect all RegexExpr
+ * @param node
+ * @return
+ */
+private Collection collectRegexExpressions(Node node) {
+if (node instanceof RegexExpr) {
+return Collections.singletonList((RegexExpr) node);
+} else {
+return node.getChildNodes().stream()
+.map(this::collectRegexExpressions)
+.flatMap(Collection::stream)
+.toList();
Review Comment:
Copilot is wrong. Compiler level of drools project is 17 now.
--
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]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] Precompile regex pattern for 'matches' operator and make it part of the lambda predicate class [incubator-kie-drools]
kie-ci3 commented on PR #6469:
URL:
https://github.com/apache/incubator-kie-drools/pull/6469#issuecomment-3343630951
**PR job** `#1` was: **UNSTABLE**
Possible explanation: This should be test failures
Reproducer
build-chain build full_downstream -f
'https://raw.githubusercontent.com/${AUTHOR:apache}/incubator-kie-kogito-pipelines/${BRANCH:main}/.ci/buildchain-config-pr-cdb.yaml'
-o 'bc' -p apache/incubator-kie-drools -u
https://github.com/apache/incubator-kie-drools/pull/6469 --skipParallelCheckout
NOTE: To install the build-chain tool, please refer to
https://github.com/kiegroup/github-action-build-chain#local-execution
Please look here:
https://ci-builds.apache.org/job/KIE/job/drools/job/main/job/pullrequest_jobs/job/drools-pr/job/PR-6469/1/display/redirect
**Test results:**
- PASSED: 23839
- FAILED: 23
Those are the test failures:
https://ci-builds.apache.org/job/KIE/job/drools/job/main/job/pullrequest_jobs/job/drools-pr/job/PR-6469/1/testReport/org.drools.model.codegen.execmodel/ActivationGroupDualFactMatchTest/testRuleAMatchingTwoFactsButNeverFires(RUN_TYPE)[2]/">org.drools.model.codegen.execmodel.ActivationGroupDualFactMatchTest.testRuleAMatchingTwoFactsButNeverFires(RUN_TYPE)[2]
[Message [id=1, level=ERROR,
path=src/main/java/com/example/P12/LambdaPredicate1227B8544ACF3634D6BBA5E875D09471.java,
line=20, column=790 text=Syntax error on token "-", = expected],
Message [id=2, level=ERROR,
path=src/main/java/com/example/P12/LambdaPredicate1227B8544ACF3634D6BBA5E875D09471.java,
line=24, column=1024 text=The operator - is undefined for the argument
type(s) Pattern, double], Message [id=3, level=ERROR,
path=src/main/java/com/example/P12/LambdaPredicate1227B8544ACF3634D6BBA5E875D09471.java,
line=24, column=1032 text=Syntax error, insert ";" to complete
BlockStatements], Message [id=4, level=ERROR,
path=src/main/java/com/example/P12/LambdaPredicate1227B8544ACF3634D6BBA5E875D09471.java,
line=24, column=1042 text=The method matcher(String) is undefined for
the type LambdaPredicate1227B8544ACF3634D6BBA5E875D09471], Message [id=5,
level=ERROR,
path=src/main/java/com/example/P12/LambdaPredicate1227B8544ACF3634D6BBA5E875D09471.java,
line=0, c
olumn=0 text=Java source of
src/main/java/com/example/P12/LambdaPredicate1227B8544ACF3634D6BBA5E875D09471.java
in error:package com.example.P12;import static
com.example.RulesE267DBE2B6851D346FC16B03FB5A96FE.*;import
com.example.*;import
org.drools.model.codegen.execmodel.ActivationGroupDualFactMatchTest.FactWrapper;import
org.drools.modelcompiler.dsl.pattern.D;@org.drools.compiler.kie.builder.MaterializedLambda()public
enum LambdaPredicate1227B8544ACF3634D6BBA5E875D09471 implements
org.drools.model.functions.Predicate1,
org.drools.model.functions.HashedExpression {INSTANCE;
public static final String EXPRESSION_HASH =
"C7F0587F0F845AD4E6F78822A64F35B6";public java.lang.String
getExpressionHash() {return EXPRESSION_HASH;}
private static final java.util.regex.Pattern regexp
_-259849451 = java.util.regex.Pattern.compile("dynamic-\\d+");
@Override()public boolean
test(org.drools.model.codegen.execmodel.ActivationGroupDualFactMatchTest.FactWrapper
_this) throws java.lang.Exception {return
regexp_-259849451.matcher(_this.getId()).matches();}
@Override()public org.drools.model.functions.PredicateInformation
predicateInformation() {
org.drools.model.functions.PredicateInformation info = new
org.drools.model.functions.PredicateInformation("id matches
\"dynamic-d+\"");info.addRuleNames("Setup Rule - GroupA",
"r0.drl");return info;}}], Message [id=6,
level=ERROR,
path=src/main/java/com/example/PC4/LambdaPredicateC49D2FF72A7483E2EFD2335208954564.java,
line=20, column=790 text=Syntax error on token "-", = expected],
Message [id=7, level=ERROR,
path=src/main/java/com/example/PC4/LambdaPredicateC49D2FF72A7483E2EFD2335208954564.java,
line=24, column=1022 text=The operator - is undefined for the argument
type(s) Pattern, double], Message [id=8, level=ERROR,
path=src/main/java/com/example/PC4/LambdaPredicateC49D2FF72A7483E2EFD2335208954564.java,
line=24, column=1030 text=Syntax error, insert ";" to complete
BlockStatements], Message [id=9, level=ERROR,
path=src/main/java/com/example/PC4/LambdaPredicateC49D2FF72A7483E2EFD2335208954564.java,
line=24, column=1040 text=The method matcher(String) is undefined for
the type LambdaPredicateC49D2FF72A7483E2EFD2335208954564], Message [id=10,
level=ERROR,
path=src/main/java/com/example/PC4/LambdaPredicateC49D2FF72A7483E2EFD2335208954564.java,
line=0, column=0 text=Java source of
src/main/java/com/example/PC4/LambdaPredicateC49D2FF72A7483E2EFD2335208954564.java
in error:package com.example.PC4;import static
com.example.RulesE267DBE2B6851D346FC16B03FB5A96FE.*;import
com.example.*;import
