Skip to content

Commit

Permalink
[KIE-766] exists and not don't work correctly with multiple constrain…
Browse files Browse the repository at this point in the history 
…ts with BigDecimal (apache#5623) (#3) (#8)

Co-authored-by: Toshiya Kobayashi <[email protected]>
  • Loading branch information
@github-actions @tkobayas
github-actions[bot] and tkobayas authored Jan 22, 2024
1 parent 31d7865 commit 7bc8b1b
Show file tree
Hide file tree
Showing 3 changed files with 205 additions and 3 deletions.
2 changes: 1 addition & 1 deletion drools-core/src/main/java/org/drools/core/util/index/IndexUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
22 changes: 20 additions & 2 deletions drools-core/src/test/java/org/drools/core/util/index/IndexUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,14 +125,32 @@ 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};
boolean[] indexed = IndexUtil.isIndexableForNode(IndexPrecedenceOption.EQUALITY_PRIORITY, NodeTypeEnums.JoinNode, config.getCompositeKeyDepth(), constraints, config);
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 {

Expand Down Expand Up @@ -202,7 +220,7 @@ public boolean isUnification() {

@Override
public boolean isIndexable(short nodeType, RuleBaseConfiguration config) {
return false;
return constraintType.isIndexableForNode(nodeType, this, config);
}

@Override
Expand Down
184 changes: 184 additions & 0 deletions ...-integration/src/test/java/org/drools/compiler/integrationtests/operators/ExistsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<String> 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<String> 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<String> 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;
}

}
}

0 comments on commit 7bc8b1b

Please sign in to comment.