Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DROOLS-7631] unify coercion checks between plain drl and executable model #6086

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

mariofusco
Copy link
Contributor

Fix for https://issues.redhat.com/browse/DROOLS-7631

The plain drl compilation already reports the compile time error

Error: Comparison operation requires compatible types. Found class java.time.LocalDateTime and class java.util.Date

but this doesn't happen when generating the executable model than then fails at runtime with the ClassCastException reported in that jira.

The purpose of this pull request is reusing the same code for coercion checks for both plain drl compilation and executable model generation.

return new CoercedExpressionResult(coercedLeft, coercedRight, rightAsStaticField);
}

private void checkCoercion(TypedExpression coercedLeft, TypedExpression coercedRight, Class<?> leftClass, Class<?> rightClass) {
if (equalityExpr) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is known at creation time you can inline the check and put instead a lambda inside the expression? so you would not repeat the check for every invocation of the cohercion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that this will reduce a bit the readability, and if you are suggesting this only for performance reason I'm pretty sure that invoking a bimorphic capturing lambda will be order of magnitude slower than accessing a local boolean variable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Performance wise I am ignorant and you are probably right. My point is that if something that affects the behavious of an object is known at creation time of an object you should make the object behave accordingly. But I admit it that this is a minor point and we can keep it as it is. thank you.

@mariofusco mariofusco merged commit d04ab13 into apache:main Sep 18, 2024
10 checks passed
@mariofusco mariofusco deleted the d7631 branch September 18, 2024 06:25
rgdoliveira pushed a commit to rgdoliveira/drools that referenced this pull request Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants