This is an automated email from the ASF dual-hosted git repository.
mariofusco pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-kie-drools.git
The following commit(s) were added to refs/heads/main by this push:
new fe2dbacafe [KIE-775] drools executable-model fails with a bind
variable to a calculation result of int and BigDecimal (#5636)
fe2dbacafe is described below
commit fe2dbacafed4f7c4809e91f829442cc6da348df5
Author: Toshiya Kobayashi <[email protected]>
AuthorDate: Wed Jan 10 00:13:53 2024 +0900
[KIE-775] drools executable-model fails with a bind variable to a
calculation result of int and BigDecimal (#5636)
---
.../drlxparse/ArithmeticCoercedExpression.java | 4 ++
.../generator/drlxparse/ConstraintParser.java | 50 ++++++++++++++++++----
.../generator/expressiontyper/ExpressionTyper.java | 30 ++++++++++---
.../execmodel/bigdecimaltest/BigDecimalTest.java | 41 ++++++++++++++++++
4 files changed, 110 insertions(+), 15 deletions(-)
diff --git
a/drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/drlxparse/ArithmeticCoercedExpression.java
b/drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/drlxparse/ArithmeticCoercedExpression.java
index 3cdea08ab3..765a6b313e 100644
---
a/drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/drlxparse/ArithmeticCoercedExpression.java
+++
b/drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/drlxparse/ArithmeticCoercedExpression.java
@@ -52,6 +52,10 @@ public class ArithmeticCoercedExpression {
this.operator = operator;
}
+ /*
+ * This coercion only deals with String vs Numeric types.
+ * BigDecimal arithmetic operation is handled by
ExpressionTyper.convertArithmeticBinaryToMethodCall()
+ */
public ArithmeticCoercedExpressionResult coerce() {
if (!requiresCoercion()) {
diff --git
a/drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/drlxparse/ConstraintParser.java
b/drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/drlxparse/ConstraintParser.java
index 2a85da6e2f..36b8f67a29 100644
---
a/drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/drlxparse/ConstraintParser.java
+++
b/drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/drlxparse/ConstraintParser.java
@@ -95,6 +95,9 @@ import static
org.drools.model.codegen.execmodel.generator.ConstraintUtil.GREATE
import static
org.drools.model.codegen.execmodel.generator.ConstraintUtil.GREATER_THAN_PREFIX;
import static
org.drools.model.codegen.execmodel.generator.ConstraintUtil.LESS_OR_EQUAL_PREFIX;
import static
org.drools.model.codegen.execmodel.generator.ConstraintUtil.LESS_THAN_PREFIX;
+import static
org.drools.model.codegen.execmodel.generator.expressiontyper.ExpressionTyper.convertArithmeticBinaryToMethodCall;
+import static
org.drools.model.codegen.execmodel.generator.expressiontyper.ExpressionTyper.getBinaryTypeAfterConversion;
+import static
org.drools.model.codegen.execmodel.generator.expressiontyper.ExpressionTyper.shouldConvertArithmeticBinaryToMethodCall;
import static org.drools.util.StringUtils.lcFirstForBean;
import static
org.drools.model.codegen.execmodel.generator.DrlxParseUtil.THIS_PLACEHOLDER;
import static
org.drools.model.codegen.execmodel.generator.DrlxParseUtil.createConstraintCompiler;
@@ -197,11 +200,23 @@ public class ConstraintParser {
}
private void addDeclaration(DrlxExpression drlx, SingleDrlxParseSuccess
singleResult, String bindId) {
- DeclarationSpec decl = context.addDeclaration( bindId,
singleResult.getLeftExprTypeBeforeCoercion() );
+ DeclarationSpec decl = context.addDeclaration(bindId,
getDeclarationType(drlx, singleResult));
if (drlx.getExpr() instanceof NameExpr) {
decl.setBoundVariable( PrintUtil.printNode(drlx.getExpr()) );
} else if (drlx.getExpr() instanceof BinaryExpr) {
-
decl.setBoundVariable(PrintUtil.printNode(drlx.getExpr().asBinaryExpr().getLeft()));
+ Expression leftMostExpression =
getLeftMostExpression(drlx.getExpr().asBinaryExpr());
+ decl.setBoundVariable(PrintUtil.printNode(leftMostExpression));
+ if (singleResult.getExpr() instanceof MethodCallExpr) {
+ // BinaryExpr was converted to MethodCallExpr. Create a
TypedExpression for the leftmost expression of the BinaryExpr
+ ExpressionTyperContext expressionTyperContext = new
ExpressionTyperContext();
+ ExpressionTyper expressionTyper = new ExpressionTyper(context,
singleResult.getPatternType(), bindId, false, expressionTyperContext);
+ TypedExpressionResult leftTypedExpressionResult =
expressionTyper.toTypedExpression(leftMostExpression);
+ Optional<TypedExpression> optLeft =
leftTypedExpressionResult.getTypedExpression();
+ if (!optLeft.isPresent()) {
+ throw new IllegalStateException("Cannot create
TypedExpression for " + drlx.getExpr().asBinaryExpr().getLeft());
+ }
+ singleResult.setBoundExpr(optLeft.get());
+ }
}
decl.setBelongingPatternDescr(context.getCurrentPatternDescr());
singleResult.setExprBinding( bindId );
@@ -211,6 +226,24 @@ public class ConstraintParser {
}
}
+ private static Class<?> getDeclarationType(DrlxExpression drlx,
SingleDrlxParseSuccess singleResult) {
+ if (drlx.getBind() != null && drlx.getExpr() instanceof EnclosedExpr) {
+ // in case of enclosed, bind type should be the calculation result
type
+ // If drlx.getBind() == null, a bind variable is inside the
enclosed expression, leave it to the default behavior
+ return (Class<?>)singleResult.getExprType();
+ } else {
+ return singleResult.getLeftExprTypeBeforeCoercion();
+ }
+ }
+
+ private Expression getLeftMostExpression(BinaryExpr binaryExpr) {
+ Expression left = binaryExpr.getLeft();
+ if (left instanceof BinaryExpr) {
+ return getLeftMostExpression((BinaryExpr) left);
+ }
+ return left;
+ }
+
/*
This is the entry point for Constraint Transformation from a parsed
MVEL constraint
to a Java Expression
@@ -657,17 +690,16 @@ public class ConstraintParser {
Expression combo;
- boolean arithmeticExpr = isArithmeticOperator(operator);
boolean isBetaConstraint = right.getExpression() != null &&
hasDeclarationFromOtherPattern( expressionTyperContext );
boolean requiresSplit = operator == BinaryExpr.Operator.AND &&
binaryExpr.getRight() instanceof HalfBinaryExpr && !isBetaConstraint;
+ Type exprType = isBooleanOperator( operator ) ? boolean.class :
left.getType();
+
if (equalityExpr) {
combo = getEqualityExpression( left, right, operator ).expression;
- } else if (arithmeticExpr && (left.isBigDecimal())) {
- ConstraintCompiler constraintCompiler =
createConstraintCompiler(this.context, of(patternType));
- CompiledExpressionResult compiledExpressionResult =
constraintCompiler.compileExpression(binaryExpr);
-
- combo = compiledExpressionResult.getExpression();
+ } else if (shouldConvertArithmeticBinaryToMethodCall(operator,
left.getType(), right.getType())) {
+ combo = convertArithmeticBinaryToMethodCall(binaryExpr,
of(patternType), this.context);
+ exprType = getBinaryTypeAfterConversion(left.getType(),
right.getType());
} else {
if (left.getExpression() == null || right.getExpression() == null)
{
return new DrlxParseFail(new
ParseExpressionErrorResult(drlxExpr));
@@ -695,7 +727,7 @@ public class ConstraintParser {
constraintType = Index.ConstraintType.FORALL_SELF_JOIN;
}
- return new SingleDrlxParseSuccess(patternType, bindingId, combo,
isBooleanOperator( operator ) ? boolean.class : left.getType())
+ return new SingleDrlxParseSuccess(patternType, bindingId, combo,
exprType)
.setDecodeConstraintType( constraintType )
.setUsedDeclarations(
expressionTyperContext.getUsedDeclarations() )
.setUsedDeclarationsOnLeft( usedDeclarationsOnLeft )
diff --git
a/drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/expressiontyper/ExpressionTyper.java
b/drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/expressiontyper/ExpressionTyper.java
index 328aa5466e..8fdfb696c4 100644
---
a/drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/expressiontyper/ExpressionTyper.java
+++
b/drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/expressiontyper/ExpressionTyper.java
@@ -235,7 +235,14 @@ public class ExpressionTyper {
right = coerced.getCoercedRight();
final BinaryExpr combo = new BinaryExpr(left.getExpression(),
right.getExpression(), operator);
- return of(new TypedExpression(combo, left.getType()));
+
+ if (shouldConvertArithmeticBinaryToMethodCall(operator,
left.getType(), right.getType())) {
+ Expression expression =
convertArithmeticBinaryToMethodCall(combo, of(typeCursor), ruleContext);
+ java.lang.reflect.Type binaryType =
getBinaryTypeAfterConversion(left.getType(), right.getType());
+ return of(new TypedExpression(expression, binaryType));
+ } else {
+ return of(new TypedExpression(combo, left.getType()));
+ }
}
if (drlxExpr instanceof HalfBinaryExpr) {
@@ -806,11 +813,12 @@ public class ExpressionTyper {
TypedExpression rightTypedExpression = right.getTypedExpression()
.orElseThrow(() -> new
NoSuchElementException("TypedExpressionResult doesn't contain
TypedExpression!"));
binaryExpr.setRight(rightTypedExpression.getExpression());
- java.lang.reflect.Type binaryType = getBinaryType(leftTypedExpression,
rightTypedExpression, binaryExpr.getOperator());
- if
(shouldConvertArithmeticBinaryToMethodCall(binaryExpr.getOperator(),
binaryType)) {
+ if
(shouldConvertArithmeticBinaryToMethodCall(binaryExpr.getOperator(),
leftTypedExpression.getType(), rightTypedExpression.getType())) {
Expression compiledExpression =
convertArithmeticBinaryToMethodCall(binaryExpr,
leftTypedExpression.getOriginalPatternType(), ruleContext);
+ java.lang.reflect.Type binaryType =
getBinaryTypeAfterConversion(leftTypedExpression.getType(),
rightTypedExpression.getType());
return new TypedExpressionCursor(compiledExpression, binaryType);
} else {
+ java.lang.reflect.Type binaryType =
getBinaryType(leftTypedExpression, rightTypedExpression,
binaryExpr.getOperator());
return new TypedExpressionCursor(binaryExpr, binaryType);
}
}
@@ -819,7 +827,7 @@ public class ExpressionTyper {
* Converts arithmetic binary expression (including coercion) to method
call using ConstraintCompiler.
* This method can be generic, so we may centralize the calls in
drools-model
*/
- private static Expression convertArithmeticBinaryToMethodCall(BinaryExpr
binaryExpr, Optional<Class<?>> originalPatternType, RuleContext ruleContext) {
+ public static Expression convertArithmeticBinaryToMethodCall(BinaryExpr
binaryExpr, Optional<Class<?>> originalPatternType, RuleContext ruleContext) {
ConstraintCompiler constraintCompiler =
createConstraintCompiler(ruleContext, originalPatternType);
CompiledExpressionResult compiledExpressionResult =
constraintCompiler.compileExpression(printNode(binaryExpr));
return compiledExpressionResult.getExpression();
@@ -828,8 +836,15 @@ public class ExpressionTyper {
/*
* BigDecimal arithmetic operations should be converted to method calls.
We may also apply this to BigInteger.
*/
- private static boolean
shouldConvertArithmeticBinaryToMethodCall(BinaryExpr.Operator operator,
java.lang.reflect.Type type) {
- return isArithmeticOperator(operator) && type.equals(BigDecimal.class);
+ public static boolean
shouldConvertArithmeticBinaryToMethodCall(BinaryExpr.Operator operator,
java.lang.reflect.Type leftType, java.lang.reflect.Type rightType) {
+ return isArithmeticOperator(operator) &&
(leftType.equals(BigDecimal.class) || rightType.equals(BigDecimal.class));
+ }
+
+ /*
+ * After arithmetic to method call conversion, BigDecimal should take
precedence regardless of left or right. We may also apply this to BigInteger.
+ */
+ public static java.lang.reflect.Type
getBinaryTypeAfterConversion(java.lang.reflect.Type leftType,
java.lang.reflect.Type rightType) {
+ return (leftType.equals(BigDecimal.class) ||
rightType.equals(BigDecimal.class)) ? BigDecimal.class : leftType;
}
private java.lang.reflect.Type getBinaryType(TypedExpression
leftTypedExpression, TypedExpression rightTypedExpression, Operator operator) {
@@ -936,6 +951,9 @@ public class ExpressionTyper {
Expression argumentExpression = methodCallExpr.getArgument(i);
if (argumentType != actualArgumentType) {
+ // unbind the original argumentExpression first, otherwise
setArgument() will remove the argumentExpression from
coercedExpression.childrenNodes
+ // It will result in failing to find DrlNameExpr in AST at
DrlsParseUtil.transformDrlNameExprToNameExpr()
+ methodCallExpr.replace(argumentExpression, new
NameExpr("placeholder"));
Expression coercedExpression = new
BigDecimalArgumentCoercion().coercedArgument(argumentType, actualArgumentType,
argumentExpression);
methodCallExpr.setArgument(i, coercedExpression);
}
diff --git
a/drools-model/drools-model-codegen/src/test/java/org/drools/model/codegen/execmodel/bigdecimaltest/BigDecimalTest.java
b/drools-model/drools-model-codegen/src/test/java/org/drools/model/codegen/execmodel/bigdecimaltest/BigDecimalTest.java
index 0cc90ca317..407462de39 100644
---
a/drools-model/drools-model-codegen/src/test/java/org/drools/model/codegen/execmodel/bigdecimaltest/BigDecimalTest.java
+++
b/drools-model/drools-model-codegen/src/test/java/org/drools/model/codegen/execmodel/bigdecimaltest/BigDecimalTest.java
@@ -772,4 +772,45 @@ public class BigDecimalTest extends BaseModelTest {
public static String intToString(int value) {
return Integer.toString(value);
}
+
+ @Test
+ public void
bindVariableToBigDecimalCoercion2Operands_shouldBindCorrectResult() {
+ bindVariableToBigDecimalCoercion("$var : (1000 * value1)");
+ }
+
+ @Test
+ public void
bindVariableToBigDecimalCoercion3Operands_shouldBindCorrectResult() {
+ bindVariableToBigDecimalCoercion("$var : (100000 * value1 / 100)");
+ }
+
+ @Test
+ public void
bindVariableToBigDecimalCoercion3OperandsWithParentheses_shouldBindCorrectResult()
{
+ bindVariableToBigDecimalCoercion("$var : ((100000 * value1) / 100)");
+ }
+
+ private void bindVariableToBigDecimalCoercion(String binding) {
+ // KIE-775
+ String str =
+ "package org.drools.modelcompiler.bigdecimals\n" +
+ "import " + BDFact.class.getCanonicalName() + ";\n" +
+ "global java.util.List result;\n" +
+ "rule R1\n" +
+ " when\n" +
+ " BDFact( " + binding + " )\n" +
+ " then\n" +
+ " result.add($var);\n" +
+ "end";
+
+ KieSession ksession = getKieSession(str);
+ List<BigDecimal> result = new ArrayList<>();
+ ksession.setGlobal("result", result);
+
+ BDFact bdFact = new BDFact();
+ bdFact.setValue1(new BigDecimal("80"));
+
+ ksession.insert(bdFact);
+ ksession.fireAllRules();
+
+ assertThat(result).contains(new BigDecimal("80000"));
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
