diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2022-09-20 13:24:38 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-09-20 13:24:38 +0200 |
commit | 7890a7db63968c49c033c79c98e6581868b5909d (patch) | |
tree | 1a952c4e8cea9efb159d00f037f18f09e33cdf5d /searchlib/src | |
parent | 6f9ebcf9d3cdb2897406eeb7704cef84d5df66ac (diff) | |
parent | 3e49b3bd652b162a65cf8df7564ecab9ac4f3414 (diff) |
Merge pull request #24135 from vespa-engine/bratseth/boolean-shortcircuit
Short circuit boolean expressions
Diffstat (limited to 'searchlib/src')
3 files changed, 32 insertions, 13 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 580f42e67cb..ce5853155d4 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,7 +47,9 @@ public final class ArithmeticNode extends CompositeNode { string.append("("); Iterator<ExpressionNode> child = children.iterator(); - child.next().toString(string, context, path, this).append(" "); + child.next().toString(string, context, path, this); + if (child.hasNext()) + string.append(" "); for (Iterator<ArithmeticOperator> op = operators.iterator(); op.hasNext() && child.hasNext();) { string.append(op.next().toString()).append(" "); child.next().toString(string, context, path, this); @@ -65,16 +67,15 @@ 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)) return false; + if ( parent == null) return false; + if ( ! (parent instanceof ArithmeticNode arithmeticParent)) 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 now we only handle the simple case correctly, and use a safe approach by adding + // So for now we only handle the simple case correctly, and use a safe approach by adding // extra parenthesis just in case.... - return arithParent.operators.get(0).hasPrecedenceOver(this.operators.get(0)) - || ((arithParent.operators.size() > 1) || (operators.size() > 1)); + return arithmeticParent.operators.get(0).hasPrecedenceOver(this.operators.get(0)) + || ((arithmeticParent.operators.size() > 1) || (operators.size() > 1)); } @Override @@ -98,7 +99,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.peek().op.hasPrecedenceOver(op)) { + while (stack.size() > 1 && ! op.hasPrecedenceOver(stack.peek().op)) { popStack(stack); } } @@ -127,9 +128,7 @@ 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)) return new ArithmeticNode(left, op, right); - - ArithmeticNode leftArithmetic = (ArithmeticNode)left; + if ( ! (left instanceof ArithmeticNode leftArithmetic)) return new ArithmeticNode(left, op, right); List<ExpressionNode> newChildren = new ArrayList<>(leftArithmetic.children()); newChildren.add(right); @@ -149,6 +148,12 @@ 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 4aee3268111..e23c6ec5dd5 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,7 +20,11 @@ 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 */ + /** + * 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. + */ 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 19e32c23234..ad50a423eb9 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,6 +56,15 @@ 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"); @@ -78,6 +87,7 @@ 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"); @@ -106,7 +116,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, (double)((IfNode) e.getRoot()).getTrueProbability(), tolerance); + assertEquals(0.3d, ((IfNode) e.getRoot()).getTrueProbability(), tolerance); // Conditionals as expressions tester.assertEvaluates(new BooleanValue(true), "2<3"); |