[DROOLS-970] fix Date coercion when the constraint uses a declaration or an instance field #543
Conversation
… or an instance field
| return declaration.getValueType() == ValueType.DATE_TYPE; | ||
| } | ||
|
|
||
| if (pattern.getObjectType() instanceof FactTemplateObjectType) { |
There was a problem hiding this comment.
Do we still support FactTemplates? I think we should clean up the code and remove all references to FactTemplates as we haven't supported this since the Drools 4 days I believe.
There was a problem hiding this comment.
I think so, or at least without that check this test case fails https://github.com/droolsjbpm/drools/blob/master/drools-compiler/src/test/java/org/drools/compiler/integrationtests/Misc2Test.java#L6968
There was a problem hiding this comment.
Ok, leave the code as is for now, but we need to talk to Mark and clean up the code. I am sure there are problems with fact templates and I am not aware of anyone using them, so it is likely this is just code garbage now.
There was a problem hiding this comment.
One reason we have not removed them, is I do believe there is a new for an abstracted type system in Drools, it's just type declarations need more work. I've left them in, as they expose a lot of the plumbing points, if we ever decide to re-investigate this work.
|
+1 to merge, looks good. |
|
@etirelli I think that we should also consider to cherry-pick this commit to 6.3.x not only because it fixes the bug reported here https://issues.jboss.org/browse/DROOLS-970 but, probably more important, because while investigating it I've found another way to alleviate the build time performance degradation reported here https://bugzilla.redhat.com/show_bug.cgi?id=1276311 After this fix I rerun the benchmark mentioned in this comment https://bugzilla.redhat.com/show_bug.cgi?id=1276311#c1 obtaining the following result: 747.449 ± 3.265 ms/op |
|
@mariofusco ok, submit another PR targeting 6.3.x and linking to the performance issue. |
Due to Quarkus future update
No description provided.