[incubator-kie-drools] ArithmeticException: Non-terminating decimal e…

[tkobayas]

…xpansion with non-executable model after jitting

Issue:

• ArithmeticException: Non-terminating decimal expansion with non-executable model after jitting #6249

[tkobayas]

GHA kogito-runtimes : Not related to this PR

2025-02-12T05:58:45.3603582Z [ERROR] org.kie.kogito.quarkus.workflows.PojoServiceIT.testPojo -- Time elapsed: 15.25 s <<< FAILURE!
2025-02-12T05:58:45.3604400Z java.lang.AssertionError:
2025-02-12T05:58:45.3604655Z
2025-02-12T05:58:45.3604805Z Expected size: 1 but was: 2 in:
2025-02-12T05:58:45.3605180Z {"age"=666, "name"="javierito"}

[tkobayas]

In jitting, ASMConditionEvaluatorJitter generates bigDecimal1.divide(bigDecimal2) for / operation, which causes this ArithmeticException.

https://github.com/apache/incubator-kie-drools/blob/main/drools-mvel/src/main/java/org/drools/mvel/ASMConditionEvaluatorJitter.java#L885

Note: Mvel (= before jitting) and executable-model use bigDecimal1.divide(bigDecimal2, MathContext.DECIMAL128) for the calculation, so we should align with it.

[gitgabrio]

Hi @tkobayas , thanks for the PR.

IIUC:

1. the addMathContext() method has some kind of hidden side effect
2. the invoke(operationType.getMethod("subtract", operationType, MathContext.class)); need that side effect, to work.
3. this has to be replicated/duplicated for all the arithmetic operations

Would not be better to create a specific invoke method, to be called for arithmetic operations, that

1. execute mv.visitFieldInsn(GETSTATIC, "java/math/MathContext", "DECIMAL128", "Ljava/math/MathContext;");
2. execute invoke(operationType.getMethod("multiply", operationType, MathContext.class)); (with name of operation received from parameter:)

e.g. (inside ClassGenerator)

        protected final void invokeArithmeticaOperation(Class<?> operationType, String operation) throws NoSuchMethodException {
            mv.visitFieldInsn(GETSTATIC, "java/math/MathContext", "DECIMAL128", "Ljava/math/MathContext;");
            invoke(operationType.getMethod("operation", operationType, MathContext.class));
        }

What do you think ? Does this make sense ?

[tkobayas]

@gitgabrio Thank you for the suggestion! Yes, it's better. I have refactored accordingly.

I also noticed that BigInteger needs to be handled differently. Updated with test cases.

[gitgabrio]

Thx @tkobayas !