Re: [PR] [Incubator kie issues#2173] Decouple FEELDialect from BooleanEvalHelper [incubator-kie-drools]
gitgabrio merged PR #6537: URL: https://github.com/apache/incubator-kie-drools/pull/6537 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [Incubator kie issues#2173] Decouple FEELDialect from BooleanEvalHelper [incubator-kie-drools]
ChinchuAjith commented on code in PR #6537:
URL:
https://github.com/apache/incubator-kie-drools/pull/6537#discussion_r2589080560
##
kie-dmn/kie-dmn-feel/src/test/java/org/kie/dmn/feel/util/BooleanEvalHelperTest.java:
##
@@ -68,21 +61,24 @@ void getBooleanOrDialectDefaultBFEEL() {
assertThat(getBooleanOrDialectDefault(null,
FEELDialect.BFEEL)).isEqualTo(Boolean.FALSE);
}
-@Test
-void getFalseOrDialectDefaultFEEL() {
-assertThat(getFalseOrDialectDefault(false,
FEELDialect.FEEL)).isEqualTo(Boolean.FALSE);
-assertThat(getFalseOrDialectDefault(true, FEELDialect.FEEL)).isNull();
-assertThat(getFalseOrDialectDefault("true",
FEELDialect.FEEL)).isNull();
-assertThat(getFalseOrDialectDefault(null, FEELDialect.FEEL)).isNull();
-}
-
-@Test
-void getFalseOrDialectDefaultBFEEL() {
-assertThat(getFalseOrDialectDefault(false,
FEELDialect.BFEEL)).isEqualTo(Boolean.FALSE);
-assertThat(getFalseOrDialectDefault(true,
FEELDialect.BFEEL)).isEqualTo(Boolean.FALSE);
-assertThat(getFalseOrDialectDefault("true",
FEELDialect.BFEEL)).isEqualTo(Boolean.FALSE);
-assertThat(getFalseOrDialectDefault(null,
FEELDialect.BFEEL)).isEqualTo(Boolean.FALSE);
-}
+//TODO to be removed
+/*
+ * @Test
+ * void getFalseOrDialectDefaultFEEL() {
+ * assertThat(getFalseOrDialectDefault(false,
FEELDialect.FEEL)).isEqualTo(Boolean.FALSE);
+ * assertThat(getFalseOrDialectDefault(true, FEELDialect.FEEL)).isNull();
+ * assertThat(getFalseOrDialectDefault("true", FEELDialect.FEEL)).isNull();
+ * assertThat(getFalseOrDialectDefault(null, FEELDialect.FEEL)).isNull();
+ * }
+ *
+ * @Test
+ * void getFalseOrDialectDefaultBFEEL() {
+ * assertThat(getFalseOrDialectDefault(false,
FEELDialect.BFEEL)).isEqualTo(Boolean.FALSE);
+ * assertThat(getFalseOrDialectDefault(true,
FEELDialect.BFEEL)).isEqualTo(Boolean.FALSE);
+ * assertThat(getFalseOrDialectDefault("true",
FEELDialect.BFEEL)).isEqualTo(Boolean.FALSE);
+ * assertThat(getFalseOrDialectDefault(null,
FEELDialect.BFEEL)).isEqualTo(Boolean.FALSE);
+ * }
+ */
Review Comment:
Keeping this as of now as it will be handled later
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] [Incubator kie issues#2173] Decouple FEELDialect from BooleanEvalHelper [incubator-kie-drools]
ChinchuAjith commented on code in PR #6537:
URL:
https://github.com/apache/incubator-kie-drools/pull/6537#discussion_r2588996989
##
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/lang/ast/UnaryTestNode.java:
##
@@ -117,37 +117,59 @@ public UnaryTest evaluate(EvaluationContext ctx) {
}
public UnaryTest getUnaryTest() {
-switch ( operator ) {
-case LTE:
-return new UnaryTestImpl( createCompareUnaryTest( (l, r) ->
l.compareTo( r ) <= 0 ) , value.getText() );
-case LT:
-return new UnaryTestImpl( createCompareUnaryTest( (l, r) ->
l.compareTo( r ) < 0 ) , value.getText() );
-case GT:
-return new UnaryTestImpl( createCompareUnaryTest( (l, r) ->
l.compareTo( r ) > 0 ) , value.getText() );
-case GTE:
-return new UnaryTestImpl( createCompareUnaryTest( (l, r) ->
l.compareTo( r ) >= 0 ) , value.getText() );
-case EQ:
-return new UnaryTestImpl( createIsEqualUnaryTest( ) ,
value.getText() );
-case NE:
-return new UnaryTestImpl( createIsNotEqualUnaryTest( ) ,
value.getText() );
-case IN:
-return new UnaryTestImpl( createInUnaryTest() ,
value.getText() );
-case NOT:
-return new UnaryTestImpl( createNotUnaryTest() ,
value.getText() );
-case TEST:
-return new UnaryTestImpl( createBooleanUnaryTest(),
value.getText() );
-}
-return null;
-}
+return new UnaryTestImpl((context, left) -> {
+Object right = value.evaluate(context);
+DialectHandler handler = DialectHandlerFactory.getHandler(context);
-private UnaryTest createCompareUnaryTest( BiPredicate op ) {
-return (context, left) -> {
-Object right = value.evaluate( context );
-// Defaulting FEELDialect to FEEL
-return BooleanEvalHelper.compare(left, right, FEELDialect.FEEL, op
);
-};
+Object result;
+switch (operator) {
+case LTE:
+result = handler.executeLte(left, right, context);
+break;
+case LT:
+result = handler.executeLt(left, right, context);
+break;
+case GT:
+result = handler.executeGt(left, right, context);
+break;
+case GTE:
+result = handler.executeGte(left, right, context);
+break;
+case EQ:
+return createIsEqualUnaryTest().apply(context, left);
+case NE:
+return createIsNotEqualUnaryTest().apply(context, left);
+case IN:
+return createInUnaryTest().apply(context, left);
+case NOT:
+return createNotUnaryTest().apply(context, left);
+case TEST:
+return createBooleanUnaryTest().apply(context, left);
+
+default:
+throw new UnsupportedOperationException("Unsupported
operator: " + operator);
+}
+
+return (result instanceof Boolean) ? (Boolean) result :
Boolean.FALSE;
+}, value.getText());
}
+/*
+ * private UnaryTest createCompareUnaryTest(InfixExecutor executor) {
+ * return (context, left) -> {
+ * Object right = value.evaluate(context);
+ *
+ * Object result = executor.evaluate(left, right, context);
+ * //return (result instanceof Boolean) ? (Boolean) result : null;
+ * if (result == null) {
+ * // treat null comparison as false
+ * return Boolean.FALSE;
+ * }
+ * return (result instanceof Boolean) ? (Boolean) result : Boolean.FALSE;
+ * };
+ * }
+ */
+
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] [Incubator kie issues#2173] Decouple FEELDialect from BooleanEvalHelper [incubator-kie-drools]
ChinchuAjith commented on code in PR #6537:
URL:
https://github.com/apache/incubator-kie-drools/pull/6537#discussion_r253381
##
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/util/DateTimeEvalHelper.java:
##
@@ -51,10 +45,10 @@ public static String toParsableString(TemporalAccessor
temporalAccessor) {
}
/**
- * DMNv1.2 10.3.2.3.6 date-time, valuedt(date and time), for use in this
{@link BooleanEvalHelper#compare(Object, Object, EvaluationContext,
BiPredicate)}
+ * DMNv1.2 10.3.2.3.6 date-time, valuedt(date and time), for use in this
{@link compare(Object, Object, EvaluationContext, BiPredicate)}
Review Comment:
Corrected
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] [Incubator kie issues#2173] Decouple FEELDialect from BooleanEvalHelper [incubator-kie-drools]
ChinchuAjith commented on code in PR #6537:
URL:
https://github.com/apache/incubator-kie-drools/pull/6537#discussion_r2588847281
##
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/runtime/impl/RangeImpl.java:
##
@@ -232,4 +239,11 @@ private String withUndefinedtoString() {
sb.append(" )");
return sb.toString();
}
+
+private static Boolean checkIsAssignable(Comparable left, Object right,
BiPredicate op) {
+if (right != null &&
left.getClass().isAssignableFrom(right.getClass())) { // short path
+return op.test(left, (Comparable) right);
+}
+return null; // shortcut not applicable
+}
Review Comment:
Removing this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] [Incubator kie issues#2173] Decouple FEELDialect from BooleanEvalHelper [incubator-kie-drools]
yesamer commented on code in PR #6537:
URL:
https://github.com/apache/incubator-kie-drools/pull/6537#discussion_r2587957886
##
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/lang/ast/BetweenNode.java:
##
@@ -107,18 +109,30 @@ public Object evaluate(EvaluationContext ctx) {
ctx.notifyEvt(astEvent(severity, Msg.createMessage(Msg.IS_NULL,
"end")));
problem = true;
}
-if (problem && ctx.getFEELDialect() != FEELDialect.BFEEL) return null;
+if (problem && ctx.getFEELDialect() != FEELDialect.BFEEL)
+return null;
+
+/*
+ * Object gte = InfixExecutorUtils.or(BooleanEvalHelper.compare(o_val,
o_s, ctx.getFEELDialect(), (l, r) -> l.compareTo(r) > 0),
+ * BooleanEvalHelper.isEqual(o_val, o_s, ctx.getFEELDialect()),
+ * ctx);
+ */ // do not use Java || to avoid potential NPE due to FEEL 3vl.
+//Object gte = GteExecutor.instance().evaluate(o_val, o_s, ctx);
+DialectHandler handler = DialectHandlerFactory.getHandler(ctx);
+Object gte = handler.executeGte(o_val, o_s, ctx);
-Object gte = InfixExecutorUtils.or(BooleanEvalHelper.compare(o_val,
o_s, ctx.getFEELDialect(), (l, r) -> l.compareTo(r) > 0),
- BooleanEvalHelper.isEqual(o_val,
o_s, ctx.getFEELDialect()),
- ctx); // do not use Java || to
avoid potential NPE due to FEEL 3vl.
if (gte == null) {
ctx.notifyEvt(astEvent(Severity.ERROR,
Msg.createMessage(Msg.X_TYPE_INCOMPATIBLE_WITH_Y_TYPE, "value", "start")));
}
-Object lte = InfixExecutorUtils.or(BooleanEvalHelper.compare(o_val,
o_e, ctx.getFEELDialect(), (l, r) -> l.compareTo(r) < 0),
-BooleanEvalHelper.isEqual(o_val, o_e, ctx.getFEELDialect()),
-ctx); // do not use Java || to avoid potential NPE due to FEEL
3vl.
+/*
+ * Object lte = InfixExecutorUtils.or(BooleanEvalHelper.compare(o_val,
o_e, ctx.getFEELDialect(), (l, r) -> l.compareTo(r) < 0),
+ * BooleanEvalHelper.isEqual(o_val, o_e, ctx.getFEELDialect()),
+ * ctx); // do not use Java || to avoid potential NPE due to FEEL 3vl.
+ */
Review Comment:
@ChinchuAjith There's a a good reason to keep these commented-out blocks of
code?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] [Incubator kie issues#2173] Decouple FEELDialect from BooleanEvalHelper [incubator-kie-drools]
yesamer commented on code in PR #6537:
URL:
https://github.com/apache/incubator-kie-drools/pull/6537#discussion_r2587955874
##
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/runtime/impl/RangeImpl.java:
##
@@ -232,4 +239,11 @@ private String withUndefinedtoString() {
sb.append(" )");
return sb.toString();
}
+
+private static Boolean checkIsAssignable(Comparable left, Object right,
BiPredicate op) {
+if (right != null &&
left.getClass().isAssignableFrom(right.getClass())) { // short path
+return op.test(left, (Comparable) right);
+}
+return null; // shortcut not applicable
+}
Review Comment:
@ChinchuAjith Can you please check this ^ ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] [Incubator kie issues#2173] Decouple FEELDialect from BooleanEvalHelper [incubator-kie-drools]
Copilot commented on code in PR #6537:
URL:
https://github.com/apache/incubator-kie-drools/pull/6537#discussion_r2585125934
##
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/runtime/impl/RangeImpl.java:
##
@@ -232,4 +239,11 @@ private String withUndefinedtoString() {
sb.append(" )");
return sb.toString();
}
+
+private static Boolean checkIsAssignable(Comparable left, Object right,
BiPredicate op) {
+if (right != null &&
left.getClass().isAssignableFrom(right.getClass())) { // short path
+return op.test(left, (Comparable) right);
+}
+return null; // shortcut not applicable
+}
Review Comment:
Commented-out code and old implementation should be removed. This includes
both the old method calls and the `checkIsAssignable` helper method that
appears to be unused.
```suggestion
```
##
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/util/DateTimeEvalHelper.java:
##
@@ -51,10 +45,10 @@ public static String toParsableString(TemporalAccessor
temporalAccessor) {
}
/**
- * DMNv1.2 10.3.2.3.6 date-time, valuedt(date and time), for use in this
{@link BooleanEvalHelper#compare(Object, Object, EvaluationContext,
BiPredicate)}
+ * DMNv1.2 10.3.2.3.6 date-time, valuedt(date and time), for use in this
{@link compare(Object, Object, EvaluationContext, BiPredicate)}
Review Comment:
Incorrect JavaDoc reference. The comment refers to `{@link
BooleanEvalHelper#compare(Object, Object, EvaluationContext, BiPredicate)}` but
this method signature doesn't exist anymore. The reference should be updated or
the link syntax should be corrected to match the new method signature.
##
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/lang/ast/BetweenNode.java:
##
@@ -107,18 +109,30 @@ public Object evaluate(EvaluationContext ctx) {
ctx.notifyEvt(astEvent(severity, Msg.createMessage(Msg.IS_NULL,
"end")));
problem = true;
}
-if (problem && ctx.getFEELDialect() != FEELDialect.BFEEL) return null;
+if (problem && ctx.getFEELDialect() != FEELDialect.BFEEL)
+return null;
+
+/*
+ * Object gte = InfixExecutorUtils.or(BooleanEvalHelper.compare(o_val,
o_s, ctx.getFEELDialect(), (l, r) -> l.compareTo(r) > 0),
+ * BooleanEvalHelper.isEqual(o_val, o_s, ctx.getFEELDialect()),
+ * ctx);
+ */ // do not use Java || to avoid potential NPE due to FEEL 3vl.
+//Object gte = GteExecutor.instance().evaluate(o_val, o_s, ctx);
+DialectHandler handler = DialectHandlerFactory.getHandler(ctx);
+Object gte = handler.executeGte(o_val, o_s, ctx);
-Object gte = InfixExecutorUtils.or(BooleanEvalHelper.compare(o_val,
o_s, ctx.getFEELDialect(), (l, r) -> l.compareTo(r) > 0),
- BooleanEvalHelper.isEqual(o_val,
o_s, ctx.getFEELDialect()),
- ctx); // do not use Java || to
avoid potential NPE due to FEEL 3vl.
if (gte == null) {
ctx.notifyEvt(astEvent(Severity.ERROR,
Msg.createMessage(Msg.X_TYPE_INCOMPATIBLE_WITH_Y_TYPE, "value", "start")));
}
-Object lte = InfixExecutorUtils.or(BooleanEvalHelper.compare(o_val,
o_e, ctx.getFEELDialect(), (l, r) -> l.compareTo(r) < 0),
-BooleanEvalHelper.isEqual(o_val, o_e, ctx.getFEELDialect()),
-ctx); // do not use Java || to avoid potential NPE due to FEEL
3vl.
+/*
+ * Object lte = InfixExecutorUtils.or(BooleanEvalHelper.compare(o_val,
o_e, ctx.getFEELDialect(), (l, r) -> l.compareTo(r) < 0),
+ * BooleanEvalHelper.isEqual(o_val, o_e, ctx.getFEELDialect()),
+ * ctx); // do not use Java || to avoid potential NPE due to FEEL 3vl.
+ */
Review Comment:
Commented-out code blocks should be removed to improve code cleanliness.
This includes old implementations of comparison operations that have been
replaced.
##
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/lang/ast/dialectHandlers/DefaultDialectHandler.java:
##
@@ -27,23 +27,29 @@
import java.time.LocalTime;
import java.time.chrono.ChronoPeriod;
import java.time.temporal.Temporal;
+import java.time.temporal.TemporalAccessor;
import java.time.temporal.TemporalAmount;
-import java.util.LinkedHashMap;
-import java.util.Map;
-import java.util.Objects;
-import java.util.Optional;
+import java.time.temporal.TemporalQueries;
+import java.util.*;
Review Comment:
[nitpick] Using wildcard imports (`import java.time.*;` and `import
java.util.*;`) is discouraged. Consider replacing with explicit imports to make
dependencies clearer.
```suggestion
import java.util.Map;
import java.util.LinkedHashMap;
```
##
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/util/BooleanEvalHelper.java:
##
@@ -62,86 +50,149 @@ public static Bool
Re: [PR] [Incubator kie issues#2173] Decouple FEELDialect from BooleanEvalHelper [incubator-kie-drools]
ChinchuAjith commented on code in PR #6537:
URL:
https://github.com/apache/incubator-kie-drools/pull/6537#discussion_r2584623582
##
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/util/BooleanEvalHelper.java:
##
@@ -62,9 +60,13 @@ public static Boolean getBooleanOrNull(Object value) {
* @param op
* @return
*/
-public static Boolean compare(Object left, Object right, FEELDialect
feelDialect, BiPredicate op) {
+public static Boolean compare(Object left, Object right,
BiPredicate op, Supplier nullFallback,
Review Comment:
Moved to the handler itself
##
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/util/BooleanEvalHelper.java:
##
@@ -96,19 +98,69 @@ public static Boolean compare(Object left, Object right,
FEELDialect feelDialect
Comparable r = (Comparable) right;
return op.test(l, r);
}
-return getBooleanOrDialectDefault(null, feelDialect);
+return defaultFallback.get();
}
+/**
+ * Compares left and right operands using the given predicate and returns
TRUE/FALSE accordingly
+ *
+ * @param left
+ * @param right
+ * @param op
+ * @return
+ */
+/*
+ * public static Boolean compare(Object left, Object right, FEELDialect
feelDialect, BiPredicate op) {
+ * if (left == null || right == null) {
+ * return getBooleanOrDialectDefault(null, feelDialect);
+ * }
+ * if (left instanceof ChronoPeriod && right instanceof ChronoPeriod) {
+ * // periods have special compare semantics in FEEL as it ignores "days".
Only months and years are compared
+ * Long l = ComparablePeriod.toTotalMonths((ChronoPeriod) left);
+ * Long r = ComparablePeriod.toTotalMonths((ChronoPeriod) right);
+ * return op.test(l, r);
+ * }
+ * if (left instanceof TemporalAccessor && right instanceof
TemporalAccessor) {
+ * // Handle specific cases when both time / datetime
+ * TemporalAccessor l = (TemporalAccessor) left;
+ * TemporalAccessor r = (TemporalAccessor) right;
+ * if (BuiltInTypeUtils.determineTypeFromInstance(left) ==
BuiltInType.TIME && BuiltInTypeUtils.determineTypeFromInstance(right) ==
BuiltInType.TIME) {
+ * return op.test(valuet(l), valuet(r));
+ * } else if (BuiltInTypeUtils.determineTypeFromInstance(left) ==
BuiltInType.DATE_TIME && BuiltInTypeUtils.determineTypeFromInstance(right) ==
BuiltInType.DATE_TIME) {
+ * return op.test(valuedt(l, r.query(TemporalQueries.zone())), valuedt(r,
l.query(TemporalQueries.zone(;
+ * }
+ * }
+ * if (left instanceof Number && right instanceof Number) {
+ * // Handle specific cases when both are Number, converting both to
BigDecimal
+ * BigDecimal l = getBigDecimalOrNull(left);
+ * BigDecimal r = getBigDecimalOrNull(right);
+ * return op.test(l, r);
+ * }
+ * // last fallback:
+ * if ((left instanceof String && right instanceof String) ||
+ * (left instanceof Boolean && right instanceof Boolean) ||
+ * (left instanceof Comparable &&
left.getClass().isAssignableFrom(right.getClass( {
+ * Comparable l = (Comparable) left;
+ * Comparable r = (Comparable) right;
+ * return op.test(l, r);
+ * }
+ * return getBooleanOrDialectDefault(null, feelDialect);
+ * }
+ */
+
/**
* Compares left and right for equality applying FEEL semantics to
specific data types
*
* @param left
* @param right
* @return
*/
-public static Boolean isEqual(Object left, Object right, FEELDialect
feelDialect) {
+public static Boolean isEqual(Object left, Object right, Supplier
nullFallback, Supplier defaultFallback) {
Review Comment:
Moved to the handler itself
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] [Incubator kie issues#2173] Decouple FEELDialect from BooleanEvalHelper [incubator-kie-drools]
kie-ci3 commented on PR #6537:
URL:
https://github.com/apache/incubator-kie-drools/pull/6537#issuecomment-3602718825
**PR job** `#3` was: **UNSTABLE**
Possible explanation: This should be test failures
Reproducer
build-chain build full_downstream -f
'https://raw.githubusercontent.com/${AUTHOR:apache}/incubator-kie-kogito-pipelines/${BRANCH:main}/.ci/buildchain-config-pr-cdb.yaml'
-o 'bc' -p apache/incubator-kie-drools -u
https://github.com/apache/incubator-kie-drools/pull/6537 --skipParallelCheckout
NOTE: To install the build-chain tool, please refer to
https://github.com/kiegroup/github-action-build-chain#local-execution
Please look here:
https://ci-builds.apache.org/job/KIE/job/drools/job/main/job/pullrequest_jobs/job/drools-pr/job/PR-6537/3/display/redirect
**Test results:**
- PASSED: 23951
- FAILED: 11
Those are the test failures:
https://ci-builds.apache.org/job/KIE/job/drools/job/main/job/pullrequest_jobs/job/drools-pr/job/PR-6537/3/testReport/org.kie.dmn.core/DMNDecisionTableHitPolicyTest/shortCircuitFIRST(boolean)[1]/">org.kie.dmn.core.DMNDecisionTableHitPolicyTest.shortCircuitFIRST(boolean)[1]
Expecting value to be false but was true
https://ci-builds.apache.org/job/KIE/job/drools/job/main/job/pullrequest_jobs/job/drools-pr/job/PR-6537/3/testReport/org.kie.dmn.core/DMNRuntimeTest/soundLevelAllowNull(boolean)[1]/">org.kie.dmn.core.DMNRuntimeTest.soundLevelAllowNull(boolean)[1]
[Message [id=0, level=ERROR, path=RecommenderHitPolicy1_allowNull.dmn,
line=25, column=-1 text=DMN: Level='null' does not match any of the
valid values >=0,null for decision table 'Evaluation'. (DMN id:
_0cd2bcb7-1882-4e26-8d7f-3dd35b04c2d4, FEEL expression evaluation error)
]Message [id=0, level=ERROR, path=RecommenderHitPolicy1_allowNull.dmn,
line=25, column=-1 text=DMN: Error evaluating Decision node
'Evaluation': DMNType{ https://www.omg.org/spec/DMN/20240513/FEEL/ : string }
(DMN id: _0cd2bcb7-1882-4e26-8d7f-3dd35b04c2d4, Error evaluating node) ]]
Expecting value to be false but was true
https://ci-builds.apache.org/job/KIE/job/drools/job/main/job/pullrequest_jobs/job/drools-pr/job/PR-6537/3/testReport/org.kie.dmn.core/DMNRuntimeTest/testNull(boolean)[1]/">org.kie.dmn.core.DMNRuntimeTest.testNull(boolean)[1]
[Message [id=0, level=ERROR, path=null_values.dmn, line=29, column=-1
text=DMN: Error invoking decision table 'Null value': NullPointerException (DMN
id: d_GreetingMessage, FEEL expression evaluation error) ]Message [id=0,
level=ERROR, path=null_values.dmn, line=29, column=-1 text=DMN: Error
evaluating Decision node 'Null value': DMNType{
https://www.omg.org/spec/DMN/20240513/FEEL/ : string } (DMN id:
d_GreetingMessage, Error evaluating node) ]] Expecting value to be false
but was true
https://ci-builds.apache.org/job/KIE/job/drools/job/main/job/pullrequest_jobs/job/drools-pr/job/PR-6537/3/testReport/org.kie.dmn.core/DMNRuntimeTest/loanRecommendation2(boolean)[1]/">org.kie.dmn.core.DMNRuntimeTest.loanRecommendation2(boolean)[1]
[Message [id=0, level=WARNING, path=Loan_Recommendation2.dmn, line=394,
column=-1 text=DMN: No rule matched for decision table 'Credit Risk
Category' and no default values were defined. Setting result to null. (DMN id:
_7b21d3ff-1235-4d8e-8dff-d23d0e754e2c, FEEL expression evaluation error)
]Message [id=0, level=ERROR, path=Loan_Recommendation2.dmn, line=394,
column=-1 text=DMN: Error while evaluating node 'Credit Risk Category':
the declared result type is 'DMNType{
http://www.trisotech.com/definitions/_35c7339b-b868-43da-8f06-eb481708c73c :
tCreditRisk }' but the actual value 'null' is not an instance of that type (DMN
id: _7b21d3ff-1235-4d8e-8dff-d23d0e754e2c, Error evaluating node) ]Message
[id=0, level=ERROR, path=Loan_Recommendation2.dmn, line=117, column=-1
text=DMN: Unable to evaluate decision 'Loan Recommendation' as it depends on
decision 'Credit Risk Category' (DMN id: _7c538c61-f6bc-40da-896f-625cc5ae5dd4,
The referenced node was not foun
d) ]] Expecting value to be false but was true
https://ci-builds.apache.org/job/KIE/job/drools/job/main/job/pullrequest_jobs/job/drools-pr/job/PR-6537/3/testReport/org.kie.dmn.core/DMNRuntimeTest/soundLevelAllowNullItemDef(boolean)[1]/">org.kie.dmn.core.DMNRuntimeTest.soundLevelAllowNullItemDef(boolean)[1]
[Message [id=0, level=ERROR,
path=RecommenderHitPolicy1_allowNull_itemDef.dmn, line=29, column=-1
text=DMN: Error while evaluating node 'Evaluation' for dependency 'Level': the
dependency value 'null' is not allowed by the declared type (DMNType{
http://www.trisotech.com/definitions/_50aea0bb-4482-48f6-acfe-4abc1a1bd0d6 :
tLevel }) (DMN id: _3aad4dbe-ad99-4dc3-8fad-cba91d4a7c15, Error evaluating
node) ]] Expecting value to be false but was true
https://ci-builds.apache.org/job/KIE/job/drools/job/main/job/pullrequest_jobs/job
Re: [PR] [Incubator kie issues#2173] Decouple FEELDialect from BooleanEvalHelper [incubator-kie-drools]
kie-ci3 commented on PR #6537:
URL:
https://github.com/apache/incubator-kie-drools/pull/6537#issuecomment-3602145626
**PR job** `#2` was: **UNSTABLE**
Possible explanation: This should be test failures
Reproducer
build-chain build full_downstream -f
'https://raw.githubusercontent.com/${AUTHOR:apache}/incubator-kie-kogito-pipelines/${BRANCH:main}/.ci/buildchain-config-pr-cdb.yaml'
-o 'bc' -p apache/incubator-kie-drools -u
https://github.com/apache/incubator-kie-drools/pull/6537 --skipParallelCheckout
NOTE: To install the build-chain tool, please refer to
https://github.com/kiegroup/github-action-build-chain#local-execution
Please look here:
https://ci-builds.apache.org/job/KIE/job/drools/job/main/job/pullrequest_jobs/job/drools-pr/job/PR-6537/2/display/redirect
**Test results:**
- PASSED: 23951
- FAILED: 11
Those are the test failures:
https://ci-builds.apache.org/job/KIE/job/drools/job/main/job/pullrequest_jobs/job/drools-pr/job/PR-6537/2/testReport/org.kie.dmn.core/DMNDecisionTableHitPolicyTest/shortCircuitFIRST(boolean)[1]/">org.kie.dmn.core.DMNDecisionTableHitPolicyTest.shortCircuitFIRST(boolean)[1]
Expecting value to be false but was true
https://ci-builds.apache.org/job/KIE/job/drools/job/main/job/pullrequest_jobs/job/drools-pr/job/PR-6537/2/testReport/org.kie.dmn.core/DMNRuntimeTest/soundLevelAllowNull(boolean)[1]/">org.kie.dmn.core.DMNRuntimeTest.soundLevelAllowNull(boolean)[1]
[Message [id=0, level=ERROR, path=RecommenderHitPolicy1_allowNull.dmn,
line=25, column=-1 text=DMN: Level='null' does not match any of the
valid values >=0,null for decision table 'Evaluation'. (DMN id:
_0cd2bcb7-1882-4e26-8d7f-3dd35b04c2d4, FEEL expression evaluation error)
]Message [id=0, level=ERROR, path=RecommenderHitPolicy1_allowNull.dmn,
line=25, column=-1 text=DMN: Error evaluating Decision node
'Evaluation': DMNType{ https://www.omg.org/spec/DMN/20240513/FEEL/ : string }
(DMN id: _0cd2bcb7-1882-4e26-8d7f-3dd35b04c2d4, Error evaluating node) ]]
Expecting value to be false but was true
https://ci-builds.apache.org/job/KIE/job/drools/job/main/job/pullrequest_jobs/job/drools-pr/job/PR-6537/2/testReport/org.kie.dmn.core/DMNRuntimeTest/testNull(boolean)[1]/">org.kie.dmn.core.DMNRuntimeTest.testNull(boolean)[1]
[Message [id=0, level=ERROR, path=null_values.dmn, line=29, column=-1
text=DMN: Error invoking decision table 'Null value': NullPointerException (DMN
id: d_GreetingMessage, FEEL expression evaluation error) ]Message [id=0,
level=ERROR, path=null_values.dmn, line=29, column=-1 text=DMN: Error
evaluating Decision node 'Null value': DMNType{
https://www.omg.org/spec/DMN/20240513/FEEL/ : string } (DMN id:
d_GreetingMessage, Error evaluating node) ]] Expecting value to be false
but was true
https://ci-builds.apache.org/job/KIE/job/drools/job/main/job/pullrequest_jobs/job/drools-pr/job/PR-6537/2/testReport/org.kie.dmn.core/DMNRuntimeTest/loanRecommendation2(boolean)[1]/">org.kie.dmn.core.DMNRuntimeTest.loanRecommendation2(boolean)[1]
[Message [id=0, level=WARNING, path=Loan_Recommendation2.dmn, line=394,
column=-1 text=DMN: No rule matched for decision table 'Credit Risk
Category' and no default values were defined. Setting result to null. (DMN id:
_7b21d3ff-1235-4d8e-8dff-d23d0e754e2c, FEEL expression evaluation error)
]Message [id=0, level=ERROR, path=Loan_Recommendation2.dmn, line=394,
column=-1 text=DMN: Error while evaluating node 'Credit Risk Category':
the declared result type is 'DMNType{
http://www.trisotech.com/definitions/_35c7339b-b868-43da-8f06-eb481708c73c :
tCreditRisk }' but the actual value 'null' is not an instance of that type (DMN
id: _7b21d3ff-1235-4d8e-8dff-d23d0e754e2c, Error evaluating node) ]Message
[id=0, level=ERROR, path=Loan_Recommendation2.dmn, line=117, column=-1
text=DMN: Unable to evaluate decision 'Loan Recommendation' as it depends on
decision 'Credit Risk Category' (DMN id: _7c538c61-f6bc-40da-896f-625cc5ae5dd4,
The referenced node was not foun
d) ]] Expecting value to be false but was true
https://ci-builds.apache.org/job/KIE/job/drools/job/main/job/pullrequest_jobs/job/drools-pr/job/PR-6537/2/testReport/org.kie.dmn.core/DMNRuntimeTest/soundLevelAllowNullItemDef(boolean)[1]/">org.kie.dmn.core.DMNRuntimeTest.soundLevelAllowNullItemDef(boolean)[1]
[Message [id=0, level=ERROR,
path=RecommenderHitPolicy1_allowNull_itemDef.dmn, line=29, column=-1
text=DMN: Error while evaluating node 'Evaluation' for dependency 'Level': the
dependency value 'null' is not allowed by the declared type (DMNType{
http://www.trisotech.com/definitions/_50aea0bb-4482-48f6-acfe-4abc1a1bd0d6 :
tLevel }) (DMN id: _3aad4dbe-ad99-4dc3-8fad-cba91d4a7c15, Error evaluating
node) ]] Expecting value to be false but was true
https://ci-builds.apache.org/job/KIE/job/drools/job/main/job/pullrequest_jobs/job
Re: [PR] [Incubator kie issues#2173] Decouple FEELDialect from BooleanEvalHelper [incubator-kie-drools]
ChinchuAjith commented on code in PR #6537:
URL:
https://github.com/apache/incubator-kie-drools/pull/6537#discussion_r2580743851
##
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/runtime/impl/RangeImpl.java:
##
@@ -151,41 +148,58 @@ private Boolean posInfRangeIncludes(FEELDialect
feelDialect, Object param) {
private Boolean negInfRangeIncludes(FEELDialect feelDialect, Object param)
{
if (highBoundary == RangeBoundary.OPEN) {
-return compare(feelDialect, highEndPoint, param, (l, r) ->
l.compareTo(r) > 0);
+return compare(feelDialect, highEndPoint, param, (l, r) ->
l.compareTo(r) > 0);
} else {
-return compare(feelDialect, highEndPoint, param, (l, r) ->
l.compareTo(r) >= 0);
+return compare(feelDialect, highEndPoint, param, (l, r) ->
l.compareTo(r) >= 0);
}
}
-
+
private Boolean bothOrThrow(Boolean left, Boolean right, Object param) {
if (left == null || right == null) {
-throw new
IllegalArgumentException("Range.include("+classOf(param)+") not comparable with
"+classOf(lowEndPoint)+", "+classOf(highEndPoint));
+throw new IllegalArgumentException("Range.include(" +
classOf(param) + ") not comparable with " + classOf(lowEndPoint) + ", " +
classOf(highEndPoint));
}
return left && right;
}
-
+
private static String classOf(Object p) {
return p != null ? p.getClass().toString() : "null";
}
-
+
private static Boolean compare(FEELDialect feelDialect, Comparable left,
Object right, BiPredicate op) {
if (left.getClass().isAssignableFrom(right.getClass())) { // short path
-return op.test(left, (Comparable) right);
+return op.test(left, (Comparable) right);
+}
+// TODO to be removed
+//return BooleanEvalHelper.compare(left, right, feelDialect, op); //
defer to full DMN/FEEL semantic
+if (feelDialect == FEELDialect.BFEEL) {
Review Comment:
Changed this to use the handlers. Please review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] [Incubator kie issues#2173] Decouple FEELDialect from BooleanEvalHelper [incubator-kie-drools]
ChinchuAjith commented on code in PR #6537:
URL:
https://github.com/apache/incubator-kie-drools/pull/6537#discussion_r2580741707
##
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/lang/ast/BetweenNode.java:
##
@@ -107,18 +109,27 @@ public Object evaluate(EvaluationContext ctx) {
ctx.notifyEvt(astEvent(severity, Msg.createMessage(Msg.IS_NULL,
"end")));
problem = true;
}
-if (problem && ctx.getFEELDialect() != FEELDialect.BFEEL) return null;
+if (problem && ctx.getFEELDialect() != FEELDialect.BFEEL)
+return null;
+
+/*
+ * Object gte = InfixExecutorUtils.or(BooleanEvalHelper.compare(o_val,
o_s, ctx.getFEELDialect(), (l, r) -> l.compareTo(r) > 0),
+ * BooleanEvalHelper.isEqual(o_val, o_s, ctx.getFEELDialect()),
+ * ctx);
+ */ // do not use Java || to avoid potential NPE due to FEEL 3vl.
+Object gte = GteExecutor.instance().evaluate(o_val, o_s, ctx);
Review Comment:
Done
##
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/lang/ast/BetweenNode.java:
##
@@ -107,18 +109,27 @@ public Object evaluate(EvaluationContext ctx) {
ctx.notifyEvt(astEvent(severity, Msg.createMessage(Msg.IS_NULL,
"end")));
problem = true;
}
-if (problem && ctx.getFEELDialect() != FEELDialect.BFEEL) return null;
+if (problem && ctx.getFEELDialect() != FEELDialect.BFEEL)
+return null;
+
+/*
+ * Object gte = InfixExecutorUtils.or(BooleanEvalHelper.compare(o_val,
o_s, ctx.getFEELDialect(), (l, r) -> l.compareTo(r) > 0),
+ * BooleanEvalHelper.isEqual(o_val, o_s, ctx.getFEELDialect()),
+ * ctx);
+ */ // do not use Java || to avoid potential NPE due to FEEL 3vl.
+Object gte = GteExecutor.instance().evaluate(o_val, o_s, ctx);
-Object gte = InfixExecutorUtils.or(BooleanEvalHelper.compare(o_val,
o_s, ctx.getFEELDialect(), (l, r) -> l.compareTo(r) > 0),
- BooleanEvalHelper.isEqual(o_val,
o_s, ctx.getFEELDialect()),
- ctx); // do not use Java || to
avoid potential NPE due to FEEL 3vl.
if (gte == null) {
ctx.notifyEvt(astEvent(Severity.ERROR,
Msg.createMessage(Msg.X_TYPE_INCOMPATIBLE_WITH_Y_TYPE, "value", "start")));
}
-Object lte = InfixExecutorUtils.or(BooleanEvalHelper.compare(o_val,
o_e, ctx.getFEELDialect(), (l, r) -> l.compareTo(r) < 0),
-BooleanEvalHelper.isEqual(o_val, o_e, ctx.getFEELDialect()),
-ctx); // do not use Java || to avoid potential NPE due to FEEL
3vl.
+/*
+ * Object lte = InfixExecutorUtils.or(BooleanEvalHelper.compare(o_val,
o_e, ctx.getFEELDialect(), (l, r) -> l.compareTo(r) < 0),
+ * BooleanEvalHelper.isEqual(o_val, o_e, ctx.getFEELDialect()),
+ * ctx); // do not use Java || to avoid potential NPE due to FEEL 3vl.
+ */
+Object lte = LteExecutor.instance().evaluate(o_val, o_e, ctx);
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] [Incubator kie issues#2173] Decouple FEELDialect from BooleanEvalHelper [incubator-kie-drools]
ChinchuAjith commented on code in PR #6537:
URL:
https://github.com/apache/incubator-kie-drools/pull/6537#discussion_r2571718708
##
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/lang/ast/BetweenNode.java:
##
@@ -107,18 +109,27 @@ public Object evaluate(EvaluationContext ctx) {
ctx.notifyEvt(astEvent(severity, Msg.createMessage(Msg.IS_NULL,
"end")));
problem = true;
}
-if (problem && ctx.getFEELDialect() != FEELDialect.BFEEL) return null;
+if (problem && ctx.getFEELDialect() != FEELDialect.BFEEL)
+return null;
+
+/*
+ * Object gte = InfixExecutorUtils.or(BooleanEvalHelper.compare(o_val,
o_s, ctx.getFEELDialect(), (l, r) -> l.compareTo(r) > 0),
+ * BooleanEvalHelper.isEqual(o_val, o_s, ctx.getFEELDialect()),
+ * ctx);
+ */ // do not use Java || to avoid potential NPE due to FEEL 3vl.
+Object gte = GteExecutor.instance().evaluate(o_val, o_s, ctx);
Review Comment:
Sure @gitgabrio Let me give it a try. We’ll remove it slowly if it’s not
being used anywhere
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] [Incubator kie issues#2173] Decouple FEELDialect from BooleanEvalHelper [incubator-kie-drools]
gitgabrio commented on code in PR #6537:
URL:
https://github.com/apache/incubator-kie-drools/pull/6537#discussion_r2571189753
##
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/util/BooleanEvalHelper.java:
##
@@ -62,9 +60,13 @@ public static Boolean getBooleanOrNull(Object value) {
* @param op
* @return
*/
-public static Boolean compare(Object left, Object right, FEELDialect
feelDialect, BiPredicate op) {
+public static Boolean compare(Object left, Object right,
BiPredicate op, Supplier nullFallback,
Review Comment:
Hi @ChinchuAjith
I'm not 100% sure of this approach.
Those two `Supplier`s basically depend on the FEELDialect, and this
compareMethod - IINW, is invoked only by the FEELHandlers. I've the impression
that, as the code currently is, this method could be completely remove - if
actually invoked only by DialectHandlers and test, and managed directly inside
handler: wdyt ? (I may be miss something, of course)
##
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/util/BooleanEvalHelper.java:
##
@@ -96,19 +98,69 @@ public static Boolean compare(Object left, Object right,
FEELDialect feelDialect
Comparable r = (Comparable) right;
return op.test(l, r);
}
-return getBooleanOrDialectDefault(null, feelDialect);
+return defaultFallback.get();
}
+/**
+ * Compares left and right operands using the given predicate and returns
TRUE/FALSE accordingly
+ *
+ * @param left
+ * @param right
+ * @param op
+ * @return
+ */
+/*
+ * public static Boolean compare(Object left, Object right, FEELDialect
feelDialect, BiPredicate op) {
+ * if (left == null || right == null) {
+ * return getBooleanOrDialectDefault(null, feelDialect);
+ * }
+ * if (left instanceof ChronoPeriod && right instanceof ChronoPeriod) {
+ * // periods have special compare semantics in FEEL as it ignores "days".
Only months and years are compared
+ * Long l = ComparablePeriod.toTotalMonths((ChronoPeriod) left);
+ * Long r = ComparablePeriod.toTotalMonths((ChronoPeriod) right);
+ * return op.test(l, r);
+ * }
+ * if (left instanceof TemporalAccessor && right instanceof
TemporalAccessor) {
+ * // Handle specific cases when both time / datetime
+ * TemporalAccessor l = (TemporalAccessor) left;
+ * TemporalAccessor r = (TemporalAccessor) right;
+ * if (BuiltInTypeUtils.determineTypeFromInstance(left) ==
BuiltInType.TIME && BuiltInTypeUtils.determineTypeFromInstance(right) ==
BuiltInType.TIME) {
+ * return op.test(valuet(l), valuet(r));
+ * } else if (BuiltInTypeUtils.determineTypeFromInstance(left) ==
BuiltInType.DATE_TIME && BuiltInTypeUtils.determineTypeFromInstance(right) ==
BuiltInType.DATE_TIME) {
+ * return op.test(valuedt(l, r.query(TemporalQueries.zone())), valuedt(r,
l.query(TemporalQueries.zone(;
+ * }
+ * }
+ * if (left instanceof Number && right instanceof Number) {
+ * // Handle specific cases when both are Number, converting both to
BigDecimal
+ * BigDecimal l = getBigDecimalOrNull(left);
+ * BigDecimal r = getBigDecimalOrNull(right);
+ * return op.test(l, r);
+ * }
+ * // last fallback:
+ * if ((left instanceof String && right instanceof String) ||
+ * (left instanceof Boolean && right instanceof Boolean) ||
+ * (left instanceof Comparable &&
left.getClass().isAssignableFrom(right.getClass( {
+ * Comparable l = (Comparable) left;
+ * Comparable r = (Comparable) right;
+ * return op.test(l, r);
+ * }
+ * return getBooleanOrDialectDefault(null, feelDialect);
+ * }
+ */
+
/**
* Compares left and right for equality applying FEEL semantics to
specific data types
*
* @param left
* @param right
* @return
*/
-public static Boolean isEqual(Object left, Object right, FEELDialect
feelDialect) {
+public static Boolean isEqual(Object left, Object right, Supplier
nullFallback, Supplier defaultFallback) {
Review Comment:
Same comment as above
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] [Incubator kie issues#2173] Decouple FEELDialect from BooleanEvalHelper [incubator-kie-drools]
gitgabrio commented on code in PR #6537:
URL:
https://github.com/apache/incubator-kie-drools/pull/6537#discussion_r2571169963
##
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/runtime/impl/RangeImpl.java:
##
@@ -151,41 +148,58 @@ private Boolean posInfRangeIncludes(FEELDialect
feelDialect, Object param) {
private Boolean negInfRangeIncludes(FEELDialect feelDialect, Object param)
{
if (highBoundary == RangeBoundary.OPEN) {
-return compare(feelDialect, highEndPoint, param, (l, r) ->
l.compareTo(r) > 0);
+return compare(feelDialect, highEndPoint, param, (l, r) ->
l.compareTo(r) > 0);
} else {
-return compare(feelDialect, highEndPoint, param, (l, r) ->
l.compareTo(r) >= 0);
+return compare(feelDialect, highEndPoint, param, (l, r) ->
l.compareTo(r) >= 0);
}
}
-
+
private Boolean bothOrThrow(Boolean left, Boolean right, Object param) {
if (left == null || right == null) {
-throw new
IllegalArgumentException("Range.include("+classOf(param)+") not comparable with
"+classOf(lowEndPoint)+", "+classOf(highEndPoint));
+throw new IllegalArgumentException("Range.include(" +
classOf(param) + ") not comparable with " + classOf(lowEndPoint) + ", " +
classOf(highEndPoint));
}
return left && right;
}
-
+
private static String classOf(Object p) {
return p != null ? p.getClass().toString() : "null";
}
-
+
private static Boolean compare(FEELDialect feelDialect, Comparable left,
Object right, BiPredicate op) {
if (left.getClass().isAssignableFrom(right.getClass())) { // short path
-return op.test(left, (Comparable) right);
+return op.test(left, (Comparable) right);
+}
+// TODO to be removed
+//return BooleanEvalHelper.compare(left, right, feelDialect, op); //
defer to full DMN/FEEL semantic
+if (feelDialect == FEELDialect.BFEEL) {
Review Comment:
I'm not sure if this is stil WIP, but since there is an FEELDialect
dependant `if`, I think this block should be managed inside the FEELHandlers:
wdyt ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] [Incubator kie issues#2173] Decouple FEELDialect from BooleanEvalHelper [incubator-kie-drools]
gitgabrio commented on code in PR #6537:
URL:
https://github.com/apache/incubator-kie-drools/pull/6537#discussion_r2571145614
##
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/lang/ast/BetweenNode.java:
##
@@ -107,18 +109,27 @@ public Object evaluate(EvaluationContext ctx) {
ctx.notifyEvt(astEvent(severity, Msg.createMessage(Msg.IS_NULL,
"end")));
problem = true;
}
-if (problem && ctx.getFEELDialect() != FEELDialect.BFEEL) return null;
+if (problem && ctx.getFEELDialect() != FEELDialect.BFEEL)
+return null;
+
+/*
+ * Object gte = InfixExecutorUtils.or(BooleanEvalHelper.compare(o_val,
o_s, ctx.getFEELDialect(), (l, r) -> l.compareTo(r) > 0),
+ * BooleanEvalHelper.isEqual(o_val, o_s, ctx.getFEELDialect()),
+ * ctx);
+ */ // do not use Java || to avoid potential NPE due to FEEL 3vl.
+Object gte = GteExecutor.instance().evaluate(o_val, o_s, ctx);
Review Comment:
Hi @ChinchuAjith : great spot!
If you're sure that the behaviour is the same, then I think we may remove
altogether the usage of "GteExecutor".
The current path is:
`BetweenNode -> GteExecutor -> DialectHandler.executeGte`
but we may invoke DialectHandler directly from BewteenNode.
Please note that I created all those *Executor a couple of years ago, but
looking at this PR it seem we may slowly dismiss them: am I clear ? Does this
make sense ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] [Incubator kie issues#2173] Decouple FEELDialect from BooleanEvalHelper [incubator-kie-drools]
gitgabrio commented on code in PR #6537:
URL:
https://github.com/apache/incubator-kie-drools/pull/6537#discussion_r2571146079
##
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/lang/ast/BetweenNode.java:
##
@@ -107,18 +109,27 @@ public Object evaluate(EvaluationContext ctx) {
ctx.notifyEvt(astEvent(severity, Msg.createMessage(Msg.IS_NULL,
"end")));
problem = true;
}
-if (problem && ctx.getFEELDialect() != FEELDialect.BFEEL) return null;
+if (problem && ctx.getFEELDialect() != FEELDialect.BFEEL)
+return null;
+
+/*
+ * Object gte = InfixExecutorUtils.or(BooleanEvalHelper.compare(o_val,
o_s, ctx.getFEELDialect(), (l, r) -> l.compareTo(r) > 0),
+ * BooleanEvalHelper.isEqual(o_val, o_s, ctx.getFEELDialect()),
+ * ctx);
+ */ // do not use Java || to avoid potential NPE due to FEEL 3vl.
+Object gte = GteExecutor.instance().evaluate(o_val, o_s, ctx);
-Object gte = InfixExecutorUtils.or(BooleanEvalHelper.compare(o_val,
o_s, ctx.getFEELDialect(), (l, r) -> l.compareTo(r) > 0),
- BooleanEvalHelper.isEqual(o_val,
o_s, ctx.getFEELDialect()),
- ctx); // do not use Java || to
avoid potential NPE due to FEEL 3vl.
if (gte == null) {
ctx.notifyEvt(astEvent(Severity.ERROR,
Msg.createMessage(Msg.X_TYPE_INCOMPATIBLE_WITH_Y_TYPE, "value", "start")));
}
-Object lte = InfixExecutorUtils.or(BooleanEvalHelper.compare(o_val,
o_e, ctx.getFEELDialect(), (l, r) -> l.compareTo(r) < 0),
-BooleanEvalHelper.isEqual(o_val, o_e, ctx.getFEELDialect()),
-ctx); // do not use Java || to avoid potential NPE due to FEEL
3vl.
+/*
+ * Object lte = InfixExecutorUtils.or(BooleanEvalHelper.compare(o_val,
o_e, ctx.getFEELDialect(), (l, r) -> l.compareTo(r) < 0),
+ * BooleanEvalHelper.isEqual(o_val, o_e, ctx.getFEELDialect()),
+ * ctx); // do not use Java || to avoid potential NPE due to FEEL 3vl.
+ */
+Object lte = LteExecutor.instance().evaluate(o_val, o_e, ctx);
Review Comment:
Same comment as above
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
