From a42b44c9a6680929cb1f260571526ac101ccdba8 Mon Sep 17 00:00:00 2001
From: Henning Baldersheim <balder@yahoo-inc.com>
Date: Wed, 21 Sep 2022 06:56:26 +0200
Subject: Revert "Revert "Short circuit boolean expressions""

---
 .../rankingexpression/rule/ArithmeticNode.java     | 27 +++++++++++++---------
 .../transform/ExpressionTransformer.java           |  6 ++++-
 2 files changed, 21 insertions(+), 12 deletions(-)

(limited to 'searchlib/src/main/java')

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);
 
     /**
-- 
cgit v1.2.3