summaryrefslogtreecommitdiffstats
path: root/searchlib
diff options
context:
space:
mode:
authorArnstein Ressem <aressem@gmail.com>2022-09-20 22:54:04 +0200
committerGitHub <noreply@github.com>2022-09-20 22:54:04 +0200
commit530593d97fa55ec3044f5b6baff3e40e8b37342a (patch)
treefa20ede0378f568ad48a4053e9875d289f83d233 /searchlib
parent5f308520b4030c1670c394a62bcb4cd1284476ef (diff)
Revert "Short circuit boolean expressions"
Diffstat (limited to 'searchlib')
-rwxr-xr-xsearchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/ArithmeticNode.java27
-rw-r--r--searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/transform/ExpressionTransformer.java6
-rw-r--r--searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/evaluation/EvaluationTestCase.java12
3 files changed, 13 insertions, 32 deletions
diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/ArithmeticNode.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/ArithmeticNode.java
index ce5853155d4..580f42e67cb 100755
--- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/ArithmeticNode.java
+++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/ArithmeticNode.java
@@ -47,9 +47,7 @@ public final class ArithmeticNode extends CompositeNode {
string.append("(");
Iterator<ExpressionNode> child = children.iterator();
- child.next().toString(string, context, path, this);
- if (child.hasNext())
- string.append(" ");
+ child.next().toString(string, context, path, this).append(" ");
for (Iterator<ArithmeticOperator> op = operators.iterator(); op.hasNext() && child.hasNext();) {
string.append(op.next().toString()).append(" ");
child.next().toString(string, context, path, this);
@@ -67,15 +65,16 @@ public final class ArithmeticNode extends CompositeNode {
* (even though by virtue of being a node it will be calculated before the parent).
*/
private boolean nonDefaultPrecedence(CompositeNode parent) {
- if ( parent == null) return false;
- if ( ! (parent instanceof ArithmeticNode arithmeticParent)) return false;
+ if ( parent==null) return false;
+ if ( ! (parent instanceof ArithmeticNode)) return false;
+ ArithmeticNode arithParent = (ArithmeticNode) parent;
// The line below can only be correct in both only have one operator.
// Getting this correct is impossible without more work.
- // So for now we only handle the simple case correctly, and use a safe approach by adding
+ // So for now now we only handle the simple case correctly, and use a safe approach by adding
// extra parenthesis just in case....
- return arithmeticParent.operators.get(0).hasPrecedenceOver(this.operators.get(0))
- || ((arithmeticParent.operators.size() > 1) || (operators.size() > 1));
+ return arithParent.operators.get(0).hasPrecedenceOver(this.operators.get(0))
+ || ((arithParent.operators.size() > 1) || (operators.size() > 1));
}
@Override
@@ -99,7 +98,7 @@ public final class ArithmeticNode extends CompositeNode {
for (Iterator<ArithmeticOperator> it = operators.iterator(); it.hasNext() && child.hasNext();) {
ArithmeticOperator op = it.next();
if ( ! stack.isEmpty()) {
- while (stack.size() > 1 && ! op.hasPrecedenceOver(stack.peek().op)) {
+ while (stack.peek().op.hasPrecedenceOver(op)) {
popStack(stack);
}
}
@@ -128,7 +127,9 @@ public final class ArithmeticNode extends CompositeNode {
public int hashCode() { return Objects.hash(children, operators); }
public static ArithmeticNode resolve(ExpressionNode left, ArithmeticOperator op, ExpressionNode right) {
- if ( ! (left instanceof ArithmeticNode leftArithmetic)) return new ArithmeticNode(left, op, right);
+ if ( ! (left instanceof ArithmeticNode)) return new ArithmeticNode(left, op, right);
+
+ ArithmeticNode leftArithmetic = (ArithmeticNode)left;
List<ExpressionNode> newChildren = new ArrayList<>(leftArithmetic.children());
newChildren.add(right);
@@ -148,12 +149,6 @@ public final class ArithmeticNode extends CompositeNode {
this.op = op;
this.value = value;
}
-
- @Override
- public String toString() {
- return value.toString();
- }
-
}
}
diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/transform/ExpressionTransformer.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/transform/ExpressionTransformer.java
index e23c6ec5dd5..4aee3268111 100644
--- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/transform/ExpressionTransformer.java
+++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/transform/ExpressionTransformer.java
@@ -20,11 +20,7 @@ public abstract class ExpressionTransformer<CONTEXT extends TransformContext> {
return new RankingExpression(expression.getName(), transform(expression.getRoot(), context));
}
- /**
- * Transforms an expression node and returns the transformed node.
- * This ic called with the root node of an expression to transform by clients of transformers.
- * Transforming nested expression nodes are left to each transformer.
- */
+ /** Transforms an expression node and returns the transformed node */
public abstract ExpressionNode transform(ExpressionNode node, CONTEXT context);
/**
diff --git a/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/evaluation/EvaluationTestCase.java b/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/evaluation/EvaluationTestCase.java
index ad50a423eb9..19e32c23234 100644
--- a/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/evaluation/EvaluationTestCase.java
+++ b/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/evaluation/EvaluationTestCase.java
@@ -56,15 +56,6 @@ public class EvaluationTestCase {
}
@Test
- public void testEvaluationOrder() {
- EvaluationTester tester = new EvaluationTester();
- tester.assertEvaluates(-4, "1 + -2 + -3");
- tester.assertEvaluates(2, "1 - (2 - 3)");
- tester.assertEvaluates(-4, "(1 - 2) - 3");
- tester.assertEvaluates(-4, "1 - 2 - 3");
- }
-
- @Test
public void testEvaluation() {
EvaluationTester tester = new EvaluationTester();
tester.assertEvaluates(0.5, "0.5");
@@ -87,7 +78,6 @@ public class EvaluationTestCase {
tester.assertEvaluates(3, "1 + 10 % 6 / 2");
tester.assertEvaluates(10.0, "3 ^ 2 + 1");
tester.assertEvaluates(18.0, "2 * 3 ^ 2");
- tester.assertEvaluates(-4, "1 - 2 - 3"); // Means 1 + -2 + -3
// Conditionals
tester.assertEvaluates(2 * (3 * 4 + 3) * (4 * 5 - 4 * 200) / 10, "2*(3*4+3)*(4*5-4*200)/10");
@@ -116,7 +106,7 @@ public class EvaluationTestCase {
// Conditionals with branch probabilities
RankingExpression e = tester.assertEvaluates(3.5, "if(1.0-1.0, 2.5, 3.5, 0.3)");
- assertEquals(0.3d, ((IfNode) e.getRoot()).getTrueProbability(), tolerance);
+ assertEquals(0.3d, (double)((IfNode) e.getRoot()).getTrueProbability(), tolerance);
// Conditionals as expressions
tester.assertEvaluates(new BooleanValue(true), "2<3");