From 7218af6ccb87c52937cecf842b8c98590a22c54a Mon Sep 17 00:00:00 2001 From: Toshiya Kobayashi Date: Wed, 20 Dec 2023 18:27:26 +0900 Subject: [PATCH] [KIE-766] exists and not don't work correctly with multiple constraints with BigDecimal (#5623) --- .../org/drools/core/util/index/IndexUtil.java | 2 +- .../drools/core/util/index/IndexUtilTest.java | 22 ++- .../operators/ExistsTest.java | 184 ++++++++++++++++++ 3 files changed, 205 insertions(+), 3 deletions(-) diff --git a/drools-core/src/main/java/org/drools/core/util/index/IndexUtil.java b/drools-core/src/main/java/org/drools/core/util/index/IndexUtil.java index b48d47a9c07..67e74d06b7d 100644 --- a/drools-core/src/main/java/org/drools/core/util/index/IndexUtil.java +++ b/drools-core/src/main/java/org/drools/core/util/index/IndexUtil.java @@ -66,7 +66,7 @@ public static boolean compositeAllowed(BetaNodeFieldConstraint[] constraints, sh } public static boolean isIndexable(BetaNodeFieldConstraint constraint, short nodeType, RuleBaseConfiguration config) { - return constraint instanceof IndexableConstraint && ((IndexableConstraint)constraint).isIndexable(nodeType, config); + return constraint instanceof IndexableConstraint && ((IndexableConstraint)constraint).isIndexable(nodeType, config) && !isBigDecimalEqualityConstraint((IndexableConstraint)constraint); } private static boolean canHaveRangeIndex(short nodeType, IndexableConstraint constraint, RuleBaseConfiguration config) { diff --git a/drools-core/src/test/java/org/drools/core/util/index/IndexUtilTest.java b/drools-core/src/test/java/org/drools/core/util/index/IndexUtilTest.java index f700b1dcdc8..1edb34d1569 100644 --- a/drools-core/src/test/java/org/drools/core/util/index/IndexUtilTest.java +++ b/drools-core/src/test/java/org/drools/core/util/index/IndexUtilTest.java @@ -125,7 +125,7 @@ public void isIndexableForNodeWithIntAndBigDecimalAndString() { } @Test - public void isIndexableForNodeWithBigDecimal() { + public void isIndexableForJoinNodeWithBigDecimal() { RuleBaseConfiguration config = new RuleBaseConfiguration(); FakeBetaNodeFieldConstraint bigDecimalEqualsConstraint = new FakeBetaNodeFieldConstraint(IndexUtil.ConstraintType.EQUAL, new FakeInternalReadAccessor(ValueType.BIG_DECIMAL_TYPE)); BetaNodeFieldConstraint[] constraints = new FakeBetaNodeFieldConstraint[]{bigDecimalEqualsConstraint}; @@ -133,6 +133,24 @@ public void isIndexableForNodeWithBigDecimal() { assertThat(indexed).as("BigDecimal is not indexed").containsExactly(false); } + @Test + public void isIndexableForExistsNodeWithBigDecimal() { + RuleBaseConfiguration config = new RuleBaseConfiguration(); + FakeBetaNodeFieldConstraint bigDecimalEqualsConstraint = new FakeBetaNodeFieldConstraint(IndexUtil.ConstraintType.EQUAL, new FakeInternalReadAccessor(ValueType.BIG_DECIMAL_TYPE)); + BetaNodeFieldConstraint[] constraints = new FakeBetaNodeFieldConstraint[]{bigDecimalEqualsConstraint}; + boolean[] indexed = IndexUtil.isIndexableForNode(IndexPrecedenceOption.EQUALITY_PRIORITY, NodeTypeEnums.ExistsNode, config.getCompositeKeyDepth(), constraints, config); + assertThat(indexed).as("BigDecimal is not indexed").containsExactly(false); + } + + @Test + public void isIndexableForNotNodeWithBigDecimal() { + RuleBaseConfiguration config = new RuleBaseConfiguration(); + FakeBetaNodeFieldConstraint bigDecimalEqualsConstraint = new FakeBetaNodeFieldConstraint(IndexUtil.ConstraintType.EQUAL, new FakeInternalReadAccessor(ValueType.BIG_DECIMAL_TYPE)); + BetaNodeFieldConstraint[] constraints = new FakeBetaNodeFieldConstraint[]{bigDecimalEqualsConstraint}; + boolean[] indexed = IndexUtil.isIndexableForNode(IndexPrecedenceOption.EQUALITY_PRIORITY, NodeTypeEnums.NotNode, config.getCompositeKeyDepth(), constraints, config); + assertThat(indexed).as("BigDecimal is not indexed").containsExactly(false); + } + static class FakeBetaNodeFieldConstraint implements BetaNodeFieldConstraint, IndexableConstraint { @@ -202,7 +220,7 @@ public boolean isUnification() { @Override public boolean isIndexable(short nodeType, RuleBaseConfiguration config) { - return false; + return constraintType.isIndexableForNode(nodeType, this, config); } @Override diff --git a/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/operators/ExistsTest.java b/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/operators/ExistsTest.java index 293674df95f..c57beef8392 100644 --- a/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/operators/ExistsTest.java +++ b/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/operators/ExistsTest.java @@ -16,6 +16,7 @@ package org.drools.compiler.integrationtests.operators; +import java.math.BigDecimal; import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -285,4 +286,187 @@ public void testSharedExistsWithNot() { ksession.dispose(); } } + + @Test + public void existsAndNotWithSingleCoercion_shouldNotMatchExists() { + // KIE-766 + final String drl = + "package org.drools.compiler.integrationtests.operators;\n" + + "import " + StringHolder.class.getCanonicalName() + "\n" + + "import " + BDHolder.class.getCanonicalName() + "\n" + + "global java.util.List list \n" + + "rule R1\n" + + " when\n" + + " $stringHolder : StringHolder( $value1 : value1 )\n" + + " exists BDHolder( $value1 == value1 )\n" + + " then\n" + + " list.add(\"R1\");\n" + + "end\n" + + "rule R2\n" + + " when\n" + + " $stringHolder : StringHolder( $value1 : value1 )\n" + + " not BDHolder( $value1 == value1 )\n" + + " then\n" + + " list.add(\"R2\");\n" + + "end\n"; + + final KieBase kbase = KieBaseUtil.getKieBaseFromKieModuleFromDrl("exists-test", + kieBaseTestConfiguration, + drl); + final KieSession ksession = kbase.newKieSession(); + try { + final List list = new ArrayList<>(); + ksession.setGlobal("list", list); + + ksession.insert(new StringHolder("9999", "567")); + ksession.insert(new BDHolder(new BigDecimal("0"), new BigDecimal("567"))); + ksession.fireAllRules(); + + assertThat(list).hasSize(1); + assertThat(list).as("BDHolder shouldn't match the exists constraints").containsExactly("R2"); + } finally { + ksession.dispose(); + } + } + + @Test + public void existsAndNotWithMultipleCoercion_shouldNotMatchExists() { + // KIE-766 + final String drl = + "package org.drools.compiler.integrationtests.operators;\n" + + "import " + StringHolder.class.getCanonicalName() + "\n" + + "import " + BDHolder.class.getCanonicalName() + "\n" + + "global java.util.List list \n" + + "rule R1\n" + + " when\n" + + " $stringHolder : StringHolder( $value1 : value1, $value2 : value2 )\n" + + " exists BDHolder( $value1 == value1, $value2 == value2 )\n" + + " then\n" + + " list.add(\"R1\");\n" + + "end\n" + + "rule R2\n" + + " when\n" + + " $stringHolder : StringHolder( $value1 : value1, $value2 : value2 )\n" + + " not BDHolder( $value1 == value1, $value2 == value2 )\n" + + " then\n" + + " list.add(\"R2\");\n" + + "end\n"; + + final KieBase kbase = KieBaseUtil.getKieBaseFromKieModuleFromDrl("exists-test", + kieBaseTestConfiguration, + drl); + final KieSession ksession = kbase.newKieSession(); + try { + final List list = new ArrayList<>(); + ksession.setGlobal("list", list); + + ksession.insert(new StringHolder("9999", "567")); + ksession.insert(new BDHolder(new BigDecimal("0"), new BigDecimal("567"))); + ksession.fireAllRules(); + + assertThat(list).hasSize(1); + assertThat(list).as("BDHolder shouldn't match the exists constraints").containsExactly("R2"); + } finally { + ksession.dispose(); + } + } + + @Test + public void existsAndNotWithBigDecimals_shouldNotMatchExists() { + // KIE-766 + final String drl = + "package org.drools.compiler.integrationtests.operators;\n" + + "import " + BDHolder.class.getCanonicalName() + "\n" + + "global java.util.List list \n" + + "rule R1\n" + + " when\n" + + " $bdHolder1 : BDHolder( $value1 : value1, $value2 : value2 )\n" + + " exists BDHolder( this != $bdHolder1, $value1 == value1, $value2 == value2 )\n" + + " then\n" + + " list.add(\"R1\");\n" + + "end\n" + + "rule R2\n" + + " when\n" + + " $bdHolder1 : BDHolder( $value1 : value1, $value2 : value2 )\n" + + " not BDHolder( this != $bdHolder1, $value1 == value1, $value2 == value2 )\n" + + " then\n" + + " list.add(\"R2\");\n" + + "end\n"; + + final KieBase kbase = KieBaseUtil.getKieBaseFromKieModuleFromDrl("exists-test", + kieBaseTestConfiguration, + drl); + final KieSession ksession = kbase.newKieSession(); + try { + final List list = new ArrayList<>(); + ksession.setGlobal("list", list); + + ksession.insert(new BDHolder(new BigDecimal("9999"), new BigDecimal("567"))); + ksession.insert(new BDHolder(new BigDecimal("0"), new BigDecimal("567"))); + ksession.fireAllRules(); + + assertThat(list).hasSize(2); + assertThat(list).as("BDHolder shouldn't match the exists constraints. R2 fires twice because 2 objects can match as $bdHolder1") + .containsExactly("R2", "R2"); + } finally { + ksession.dispose(); + } + } + + public static class StringHolder { + + private String value1; + private String value2; + + public StringHolder(String value1, String value2) { + this.value1 = value1; + this.value2 = value2; + } + + public String getValue1() { + return value1; + } + + public void setValue1(String value1) { + this.value1 = value1; + } + + public String getValue2() { + return value2; + } + + public void setValue2(String value2) { + this.value2 = value2; + } + + } + + public static class BDHolder { + + private BigDecimal value1; + private BigDecimal value2; + + public BDHolder(BigDecimal value1, BigDecimal value2) { + super(); + this.value1 = value1; + this.value2 = value2; + } + + public BigDecimal getValue1() { + return value1; + } + + public void setValue1(BigDecimal value1) { + this.value1 = value1; + } + + public BigDecimal getValue2() { + return value2; + } + + public void setValue2(BigDecimal value2) { + this.value2 = value2; + } + + } }