Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
DROOLS-1404 Alpha node not shared with static function evaluation (#1063
)

If MVELConstraint hashable (==) 
and MVELConstraint.FieldValue != null
then 
the right part of the expression is constant/memoized,
hence
only the left part of the expression shall be compared.
  • Loading branch information
tarilabs authored and mariofusco committed Jan 13, 2017
1 parent 09b4926 commit ef58f94
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 31 deletions.
Expand Up @@ -59,6 +59,7 @@
import org.drools.core.reteoo.ObjectTypeNode;
import org.drools.core.reteoo.Rete;
import org.drools.core.reteoo.ReteComparator;
import org.drools.core.reteoo.ReteDumper;
import org.drools.core.reteoo.RightTuple;
import org.drools.core.reteoo.SegmentMemory;
import org.drools.core.spi.KnowledgeHelper;
Expand Down Expand Up @@ -8937,6 +8938,8 @@ public void test1187() {

kieSession.fireAllRules();
}



public class Shift1187 {

Expand Down Expand Up @@ -9067,4 +9070,46 @@ public void testNpeInLessThanComparison() {
assertFalse( list.contains( "GreaterThanCompare:5" ) );
assertFalse( list.contains( "GreaterThanCompare:null" ) );
}

public static class TestStaticUtils {
public static int return1() {
return 1;
}
}

@Test
public void testShouldAlphaShareBecauseSameConstantDespiteDifferentSyntax() {
// DROOLS-1404
String drl1 = "package c;\n" +
"import " + TestObject.class.getCanonicalName() + "\n" +
"rule fileArule1 when\n" +
" TestObject(value == 1)\n" +
"then\n" +
"end\n" +
"";
String drl2 = "package iTzXzx;\n" + // <<- keep the different package
"import " + TestObject.class.getCanonicalName() + "\n" +
"import " + TestStaticUtils.class.getCanonicalName() + "\n" +
"rule fileBrule1 when\n" +
" TestObject(value == TestStaticUtils.return1() )\n" +
"then\n" +
"end\n" +
"rule fileBrule2 when\n" + // <<- keep this rule
" TestObject(value == 0 )\n" +
"then\n" +
"end\n" +
"";

KieSession kieSession = new KieHelper()
.addContent(drl1, ResourceType.DRL)
.addContent(drl2, ResourceType.DRL)
.build().newKieSession();

ReteDumper.dumpRete(kieSession);

kieSession.insert(new TestObject(1));

assertEquals(2, kieSession.fireAllRules() );
}

}
Expand Up @@ -561,13 +561,13 @@ public MvelConstraint clone() {

public int hashCode() {
if (isAlphaHashable()) {
return 29 * getLeftForEqualExpression().hashCode() + 31 * fieldValue.hashCode();
return 29 * getLeftInExpression(IndexUtil.ConstraintType.EQUAL).hashCode() + 31 * fieldValue.hashCode();
}
return expression.hashCode();
}

private String getLeftForEqualExpression() {
return expression.substring(0, expression.indexOf("==")).trim();
private String getLeftInExpression(IndexUtil.ConstraintType constraint) {
return expression.substring(0, expression.indexOf(constraint.getOperator())).trim();
}

private boolean isAlphaHashable() {
Expand All @@ -584,7 +584,7 @@ public boolean equals(final Object object) {
MvelConstraint other = (MvelConstraint) object;
if (isAlphaHashable()) {
if ( !other.isAlphaHashable() ||
!getLeftForEqualExpression().equals(other.getLeftForEqualExpression()) ||
!getLeftInExpression(IndexUtil.ConstraintType.EQUAL).equals(other.getLeftInExpression(IndexUtil.ConstraintType.EQUAL)) ||
!fieldValue.equals(other.fieldValue) ) {
return false;
}
Expand Down Expand Up @@ -617,12 +617,19 @@ public boolean equals(Object object, InternalKnowledgeBase kbase) {
Map<String, Object> thisImports = ((MVELDialectRuntimeData) kbase.getPackage( thisPkg ).getDialectRuntimeRegistry().getDialectData("mvel")).getImports();
Map<String, Object> otherImports = ((MVELDialectRuntimeData) kbase.getPackage( otherPkg ).getDialectRuntimeRegistry().getDialectData("mvel")).getImports();

if (fieldValue != null) {
return equalsExpressionTokensInBothImports(getLeftInExpression(IndexUtil.ConstraintType.EQUAL), thisImports, otherImports);
} else {
return equalsExpressionTokensInBothImports(expression, thisImports, otherImports);
}
}

private boolean equalsExpressionTokensInBothImports(String expression, Map<String, Object> thisImports, Map<String, Object> otherImports) {
for (String token : splitExpression(expression)) {
if ( !areNullSafeEquals(thisImports.get(token), otherImports.get(token)) ) {
return false;
}
}

return true;
}

Expand Down
49 changes: 23 additions & 26 deletions drools-core/src/main/java/org/drools/core/util/index/IndexUtil.java
Expand Up @@ -188,19 +188,21 @@ private static void swap(BetaNodeFieldConstraint[] constraints, int p1, int p2)
}

public enum ConstraintType {
EQUAL(true),
NOT_EQUAL(false),
GREATER_THAN(true),
GREATER_OR_EQUAL(true),
LESS_THAN(true),
LESS_OR_EQUAL(true),
RANGE(true),
UNKNOWN(false);
EQUAL(true, "=="),
NOT_EQUAL(false, "!="),
GREATER_THAN(true, ">"),
GREATER_OR_EQUAL(true, ">="),
LESS_THAN(true, "<"),
LESS_OR_EQUAL(true, "<="),
RANGE(true, null),
UNKNOWN(false, null);

private final boolean indexable;
private final String operator;

private ConstraintType(boolean indexable) {
private ConstraintType(boolean indexable, String operator) {
this.indexable = indexable;
this.operator = operator;
}

public boolean isComparison() {
Expand All @@ -222,6 +224,14 @@ public boolean isDescending() {
public boolean isIndexable() {
return indexable;
}

/**
* May be null.
* @return the operator string representation if does exists, null otherwise.
*/
public String getOperator() {
return this.operator;
}

public boolean isIndexableForNode(short nodeType) {
switch (this) {
Expand All @@ -236,23 +246,10 @@ public boolean isIndexableForNode(short nodeType) {
}

public static ConstraintType decode(String operator) {
if (operator.equals("==")) {
return EQUAL;
}
if (operator.equals("!=")) {
return NOT_EQUAL;
}
if (operator.equals(">")) {
return GREATER_THAN;
}
if (operator.equals(">=")) {
return GREATER_OR_EQUAL;
}
if (operator.equals("<")) {
return LESS_THAN;
}
if (operator.equals("<=")) {
return LESS_OR_EQUAL;
for ( ConstraintType c : ConstraintType.values() ) {
if ( c.getOperator() != null && c.getOperator().equals(operator) ) {
return c;
}
}
return UNKNOWN;
}
Expand Down

0 comments on commit ef58f94

Please sign in to comment.