Skip to content

Commit 4d3bcc0

Browse files
github-actions[bot]tkobayas
github-actions[bot]
and
tkobayas
authored
[KIE-775] drools executable-model fails with a bind variable to a calculation result of int and BigDecimal (apache#5636) (apache#2) (apache#5)
Co-authored-by: Toshiya Kobayashi <[email protected]>
1 parent e80ff92 commit 4d3bcc0

File tree

4 files changed

+110
-15
lines changed

4 files changed

+110
-15
lines changed

‎drools-model/drools-model-compiler/src/main/java/org/drools/modelcompiler/builder/generator/drlxparse/ArithmeticCoercedExpression.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@ public ArithmeticCoercedExpression(TypedExpression left, TypedExpression right,
5151
this.operator = operator;
5252
}
5353

54+
/*
55+
* This coercion only deals with String vs Numeric types.
56+
* BigDecimal arithmetic operation is handled by ExpressionTyper.convertArithmeticBinaryToMethodCall()
57+
*/
5458
public ArithmeticCoercedExpressionResult coerce() {
5559

5660
if (!requiresCoercion()) {

‎drools-model/drools-model-compiler/src/main/java/org/drools/modelcompiler/builder/generator/drlxparse/ConstraintParser.java‎

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,9 @@
9797
import static org.drools.modelcompiler.builder.generator.DrlxParseUtil.stripEnclosedExpr;
9898
import static org.drools.modelcompiler.builder.generator.drlxparse.MultipleDrlxParseSuccess.createMultipleDrlxParseSuccess;
9999
import static org.drools.modelcompiler.builder.generator.drlxparse.SpecialComparisonCase.specialComparisonFactory;
100+
import static org.drools.modelcompiler.builder.generator.expressiontyper.ExpressionTyper.convertArithmeticBinaryToMethodCall;
101+
import static org.drools.modelcompiler.builder.generator.expressiontyper.ExpressionTyper.getBinaryTypeAfterConversion;
102+
import static org.drools.modelcompiler.builder.generator.expressiontyper.ExpressionTyper.shouldConvertArithmeticBinaryToMethodCall;
100103
import static org.drools.modelcompiler.builder.generator.expressiontyper.FlattenScope.transformFullyQualifiedInlineCastExpr;
101104
import static org.drools.mvel.parser.printer.PrintUtil.printNode;
102105
import static org.drools.mvel.parser.utils.AstUtils.isLogicalOperator;
@@ -189,11 +192,23 @@ private void logWarnIfNoReactOnCausedByVariableFromDifferentPattern(DrlxParseRes
189192
}
190193

191194
private void addDeclaration(DrlxExpression drlx, SingleDrlxParseSuccess singleResult, String bindId) {
192-
DeclarationSpec decl = context.addDeclaration( bindId, singleResult.getLeftExprTypeBeforeCoercion() );
195+
DeclarationSpec decl = context.addDeclaration(bindId, getDeclarationType(drlx, singleResult));
193196
if (drlx.getExpr() instanceof NameExpr) {
194197
decl.setBoundVariable( PrintUtil.printNode(drlx.getExpr()) );
195198
} else if (drlx.getExpr() instanceof BinaryExpr) {
196-
decl.setBoundVariable(PrintUtil.printNode(drlx.getExpr().asBinaryExpr().getLeft()));
199+
Expression leftMostExpression = getLeftMostExpression(drlx.getExpr().asBinaryExpr());
200+
decl.setBoundVariable(PrintUtil.printNode(leftMostExpression));
201+
if (singleResult.getExpr() instanceof MethodCallExpr) {
202+
// BinaryExpr was converted to MethodCallExpr. Create a TypedExpression for the leftmost expression of the BinaryExpr
203+
ExpressionTyperContext expressionTyperContext = new ExpressionTyperContext();
204+
ExpressionTyper expressionTyper = new ExpressionTyper(context, singleResult.getPatternType(), bindId, false, expressionTyperContext);
205+
TypedExpressionResult leftTypedExpressionResult = expressionTyper.toTypedExpression(leftMostExpression);
206+
Optional<TypedExpression> optLeft = leftTypedExpressionResult.getTypedExpression();
207+
if (!optLeft.isPresent()) {
208+
throw new IllegalStateException("Cannot create TypedExpression for " + drlx.getExpr().asBinaryExpr().getLeft());
209+
}
210+
singleResult.setBoundExpr(optLeft.get());
211+
}
197212
}
198213
decl.setBelongingPatternDescr(context.getCurrentPatternDescr());
199214
singleResult.setExprBinding( bindId );
@@ -203,6 +218,24 @@ private void addDeclaration(DrlxExpression drlx, SingleDrlxParseSuccess singleRe
203218
}
204219
}
205220

221+
private static Class<?> getDeclarationType(DrlxExpression drlx, SingleDrlxParseSuccess singleResult) {
222+
if (drlx.getBind() != null && drlx.getExpr() instanceof EnclosedExpr) {
223+
// in case of enclosed, bind type should be the calculation result type
224+
// If drlx.getBind() == null, a bind variable is inside the enclosed expression, leave it to the default behavior
225+
return (Class<?>)singleResult.getExprType();
226+
} else {
227+
return singleResult.getLeftExprTypeBeforeCoercion();
228+
}
229+
}
230+
231+
private Expression getLeftMostExpression(BinaryExpr binaryExpr) {
232+
Expression left = binaryExpr.getLeft();
233+
if (left instanceof BinaryExpr) {
234+
return getLeftMostExpression((BinaryExpr) left);
235+
}
236+
return left;
237+
}
238+
206239
/*
207240
This is the entry point for Constraint Transformation from a parsed MVEL constraint
208241
to a Java Expression
@@ -626,17 +659,16 @@ private DrlxParseResult parseBinaryExpr(BinaryExpr binaryExpr, Class<?> patternT
626659

627660
Expression combo;
628661

629-
boolean arithmeticExpr = isArithmeticOperator(operator);
630662
boolean isBetaConstraint = right.getExpression() != null && hasDeclarationFromOtherPattern( expressionTyperContext );
631663
boolean requiresSplit = operator == BinaryExpr.Operator.AND && binaryExpr.getRight() instanceof HalfBinaryExpr && !isBetaConstraint;
632664

665+
Type exprType = isBooleanOperator( operator ) ? boolean.class : left.getType();
666+
633667
if (equalityExpr) {
634668
combo = getEqualityExpression( left, right, operator ).expression;
635-
} else if (arithmeticExpr && (left.isBigDecimal())) {
636-
ConstraintCompiler constraintCompiler = createConstraintCompiler(this.context, of(patternType));
637-
CompiledExpressionResult compiledExpressionResult = constraintCompiler.compileExpression(binaryExpr);
638-
639-
combo = compiledExpressionResult.getExpression();
669+
} else if (shouldConvertArithmeticBinaryToMethodCall(operator, left.getType(), right.getType())) {
670+
combo = convertArithmeticBinaryToMethodCall(binaryExpr, of(patternType), this.context);
671+
exprType = getBinaryTypeAfterConversion(left.getType(), right.getType());
640672
} else {
641673
if (left.getExpression() == null || right.getExpression() == null) {
642674
return new DrlxParseFail(new ParseExpressionErrorResult(drlxExpr));
@@ -664,7 +696,7 @@ private DrlxParseResult parseBinaryExpr(BinaryExpr binaryExpr, Class<?> patternT
664696
constraintType = Index.ConstraintType.FORALL_SELF_JOIN;
665697
}
666698

667-
return new SingleDrlxParseSuccess(patternType, bindingId, combo, isBooleanOperator( operator ) ? boolean.class : left.getType())
699+
return new SingleDrlxParseSuccess(patternType, bindingId, combo, exprType)
668700
.setDecodeConstraintType( constraintType )
669701
.setUsedDeclarations( expressionTyperContext.getUsedDeclarations() )
670702
.setUsedDeclarationsOnLeft( usedDeclarationsOnLeft )

‎drools-model/drools-model-compiler/src/main/java/org/drools/modelcompiler/builder/generator/expressiontyper/ExpressionTyper.java‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,14 @@ private Optional<TypedExpression> toTypedExpressionRec(Expression drlxExpr) {
237237
right = coerced.getCoercedRight();
238238

239239
final BinaryExpr combo = new BinaryExpr(left.getExpression(), right.getExpression(), operator);
240-
return of(new TypedExpression(combo, left.getType()));
240+
241+
if (shouldConvertArithmeticBinaryToMethodCall(operator, left.getType(), right.getType())) {
242+
Expression expression = convertArithmeticBinaryToMethodCall(combo, of(typeCursor), ruleContext);
243+
java.lang.reflect.Type binaryType = getBinaryTypeAfterConversion(left.getType(), right.getType());
244+
return of(new TypedExpression(expression, binaryType));
245+
} else {
246+
return of(new TypedExpression(combo, left.getType()));
247+
}
241248
}
242249

243250
if (drlxExpr instanceof HalfBinaryExpr) {
@@ -810,11 +817,12 @@ private TypedExpressionCursor binaryExpr(BinaryExpr binaryExpr) {
810817
TypedExpression rightTypedExpression = right.getTypedExpression()
811818
.orElseThrow(() -> new NoSuchElementException("TypedExpressionResult doesn't contain TypedExpression!"));
812819
binaryExpr.setRight(rightTypedExpression.getExpression());
813-
java.lang.reflect.Type binaryType = getBinaryType(leftTypedExpression, rightTypedExpression, binaryExpr.getOperator());
814-
if (shouldConvertArithmeticBinaryToMethodCall(binaryExpr.getOperator(), binaryType)) {
820+
if (shouldConvertArithmeticBinaryToMethodCall(binaryExpr.getOperator(), leftTypedExpression.getType(), rightTypedExpression.getType())) {
815821
Expression compiledExpression = convertArithmeticBinaryToMethodCall(binaryExpr, leftTypedExpression.getOriginalPatternType(), ruleContext);
822+
java.lang.reflect.Type binaryType = getBinaryTypeAfterConversion(leftTypedExpression.getType(), rightTypedExpression.getType());
816823
return new TypedExpressionCursor(compiledExpression, binaryType);
817824
} else {
825+
java.lang.reflect.Type binaryType = getBinaryType(leftTypedExpression, rightTypedExpression, binaryExpr.getOperator());
818826
return new TypedExpressionCursor(binaryExpr, binaryType);
819827
}
820828
}
@@ -823,7 +831,7 @@ private TypedExpressionCursor binaryExpr(BinaryExpr binaryExpr) {
823831
* Converts arithmetic binary expression (including coercion) to method call using ConstraintCompiler.
824832
* This method can be generic, so we may centralize the calls in drools-model
825833
*/
826-
private static Expression convertArithmeticBinaryToMethodCall(BinaryExpr binaryExpr, Optional<Class<?>> originalPatternType, RuleContext ruleContext) {
834+
public static Expression convertArithmeticBinaryToMethodCall(BinaryExpr binaryExpr, Optional<Class<?>> originalPatternType, RuleContext ruleContext) {
827835
ConstraintCompiler constraintCompiler = createConstraintCompiler(ruleContext, originalPatternType);
828836
CompiledExpressionResult compiledExpressionResult = constraintCompiler.compileExpression(printNode(binaryExpr));
829837
return compiledExpressionResult.getExpression();
@@ -832,8 +840,15 @@ private static Expression convertArithmeticBinaryToMethodCall(BinaryExpr binaryE
832840
/*
833841
* BigDecimal arithmetic operations should be converted to method calls. We may also apply this to BigInteger.
834842
*/
835-
private static boolean shouldConvertArithmeticBinaryToMethodCall(BinaryExpr.Operator operator, java.lang.reflect.Type type) {
836-
return isArithmeticOperator(operator) && type.equals(BigDecimal.class);
843+
public static boolean shouldConvertArithmeticBinaryToMethodCall(BinaryExpr.Operator operator, java.lang.reflect.Type leftType, java.lang.reflect.Type rightType) {
844+
return isArithmeticOperator(operator) && (leftType.equals(BigDecimal.class) || rightType.equals(BigDecimal.class));
845+
}
846+
847+
/*
848+
* After arithmetic to method call conversion, BigDecimal should take precedence regardless of left or right. We may also apply this to BigInteger.
849+
*/
850+
public static java.lang.reflect.Type getBinaryTypeAfterConversion(java.lang.reflect.Type leftType, java.lang.reflect.Type rightType) {
851+
return (leftType.equals(BigDecimal.class) || rightType.equals(BigDecimal.class)) ? BigDecimal.class : leftType;
837852
}
838853

839854
private java.lang.reflect.Type getBinaryType(TypedExpression leftTypedExpression, TypedExpression rightTypedExpression, Operator operator) {
@@ -940,6 +955,9 @@ private void promoteBigDecimalParameters(MethodCallExpr methodCallExpr, Class[]
940955
Expression argumentExpression = methodCallExpr.getArgument(i);
941956

942957
if (argumentType != actualArgumentType) {
958+
// unbind the original argumentExpression first, otherwise setArgument() will remove the argumentExpression from coercedExpression.childrenNodes
959+
// It will result in failing to find DrlNameExpr in AST at DrlsParseUtil.transformDrlNameExprToNameExpr()
960+
methodCallExpr.replace(argumentExpression, new NameExpr("placeholder"));
943961
Expression coercedExpression = new BigDecimalArgumentCoercion().coercedArgument(argumentType, actualArgumentType, argumentExpression);
944962
methodCallExpr.setArgument(i, coercedExpression);
945963
}

‎drools-model/drools-model-compiler/src/test/java/org/drools/modelcompiler/bigdecimaltest/BigDecimalTest.java‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -682,4 +682,45 @@ public void bigDecimalCoercionInNestedMethodArgument_shouldNotFailToBuild() {
682682
public static String intToString(int value) {
683683
return Integer.toString(value);
684684
}
685+
686+
@Test
687+
public void bindVariableToBigDecimalCoercion2Operands_shouldBindCorrectResult() {
688+
bindVariableToBigDecimalCoercion("$var : (1000 * value1)");
689+
}
690+
691+
@Test
692+
public void bindVariableToBigDecimalCoercion3Operands_shouldBindCorrectResult() {
693+
bindVariableToBigDecimalCoercion("$var : (100000 * value1 / 100)");
694+
}
695+
696+
@Test
697+
public void bindVariableToBigDecimalCoercion3OperandsWithParentheses_shouldBindCorrectResult() {
698+
bindVariableToBigDecimalCoercion("$var : ((100000 * value1) / 100)");
699+
}
700+
701+
private void bindVariableToBigDecimalCoercion(String binding) {
702+
// KIE-775
703+
String str =
704+
"package org.drools.modelcompiler.bigdecimals\n" +
705+
"import " + BDFact.class.getCanonicalName() + ";\n" +
706+
"global java.util.List result;\n" +
707+
"rule R1\n" +
708+
" when\n" +
709+
" BDFact( " + binding + " )\n" +
710+
" then\n" +
711+
" result.add($var);\n" +
712+
"end";
713+
714+
KieSession ksession = getKieSession(str);
715+
List<BigDecimal> result = new ArrayList<>();
716+
ksession.setGlobal("result", result);
717+
718+
BDFact bdFact = new BDFact();
719+
bdFact.setValue1(new BigDecimal("80"));
720+
721+
ksession.insert(bdFact);
722+
ksession.fireAllRules();
723+
724+
assertThat(result).contains(new BigDecimal("80000"));
725+
}
685726
}

0 commit comments

Comments
 (0)