Merge pull request #24144 from vespa-engine/revert-24135-bratseth/boo…

…lean-shortcircuit

config-model/src/main/java/com/yahoo/schema/expressiontransforms/ExpressionTransforms.java

@@ -35,8 +35,7 @@ public ExpressionTransforms() {
                                  new FunctionInliner(),
                                  new FunctionShadower(),
                                  new TensorMaxMinTransformer(),
-                                 new Simplifier(),
-                                 new BooleanExpressionTransformer());
+                                 new Simplifier());
     }
 
     public RankingExpression transform(RankingExpression expression, RankProfileTransformContext context) {

config-model/src/test/java/com/yahoo/schema/RankingExpressionInliningTestCase.java

@@ -163,7 +163,7 @@ void testNonTopLevelInlining() throws ParseException {
                         "    \n" +
                         "    rank-profile test {\n" +
                         "        first-phase {\n" +
-                        "            expression: A + C - D\n" +
+                        "            expression: A + C + D\n" +
                         "        }\n" +
                         "        function inline D() {\n" +
                         "            expression: B + 1\n" +
@@ -184,7 +184,7 @@ void testNonTopLevelInlining() throws ParseException {
         Schema s = builder.getSchema();
 
         RankProfile test = rankProfileRegistry.get(s, "test").compile(new QueryProfileRegistry(), new ImportedMlModels());
-        assertEquals("attribute(a) + C - (attribute(b) + 1)", test.getFirstPhaseRanking().getRoot().toString());
+        assertEquals("attribute(a) + C + (attribute(b) + 1)", test.getFirstPhaseRanking().getRoot().toString());
         assertEquals("attribute(a) + attribute(b)", getRankingExpression("C", test, s));
         assertEquals("attribute(b) + 1", getRankingExpression("D", test, s));
     }

documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/GetDocumentReply.java

@@ -25,8 +25,7 @@ public class GetDocumentReply extends DocumentAcceptedReply {
 
     /**
      * Constructs a new reply to lazily deserialize from a byte buffer.
-     *
-     * @param decoder The decoder to use for deserialization.
+     *  @param decoder The decoder to use for deserialization.
      * @param buf     A byte buffer that contains a serialized reply.
      */
     GetDocumentReply(LazyDecoder decoder, DocumentDeserializer buf) {

indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ArithmeticExpression.java

@@ -166,13 +166,19 @@ private FieldValue evaluate(FieldValue lhs, FieldValue rhs) {
         }
         BigDecimal lhsVal = asBigDecimal((NumericFieldValue)lhs);
         BigDecimal rhsVal = asBigDecimal((NumericFieldValue)rhs);
-        return switch (op) {
-            case ADD -> createFieldValue(lhs, rhs, lhsVal.add(rhsVal));
-            case SUB -> createFieldValue(lhs, rhs, lhsVal.subtract(rhsVal));
-            case MUL -> createFieldValue(lhs, rhs, lhsVal.multiply(rhsVal));
-            case DIV -> createFieldValue(lhs, rhs, lhsVal.divide(rhsVal, MathContext.DECIMAL64));
-            case MOD -> createFieldValue(lhs, rhs, lhsVal.remainder(rhsVal));
-        };
+        switch (op) {
+        case ADD:
+            return createFieldValue(lhs, rhs, lhsVal.add(rhsVal));
+        case SUB:
+            return createFieldValue(lhs, rhs, lhsVal.subtract(rhsVal));
+        case MUL:
+            return createFieldValue(lhs, rhs, lhsVal.multiply(rhsVal));
+        case DIV:
+            return createFieldValue(lhs, rhs, lhsVal.divide(rhsVal, MathContext.DECIMAL64));
+        case MOD:
+            return createFieldValue(lhs, rhs, lhsVal.remainder(rhsVal));
+        }
+        throw new IllegalStateException("Unsupported operation: " + op);
     }
 
     private FieldValue createFieldValue(FieldValue lhs, FieldValue rhs, BigDecimal val) {

searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/ArithmeticNode.java

@@ -47,9 +47,7 @@ public StringBuilder toString(StringBuilder string, SerializationContext context
             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 StringBuilder toString(StringBuilder string, SerializationContext context
      * (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 Value evaluate(Context context) {
         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 CompositeNode setChildren(List<ExpressionNode> newChildren) {
     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 ValueItem(ArithmeticOperator op, Value value) {
             this.op = op;
             this.value = value;
         }
-
-        @Override
-        public String toString() {
-            return value.toString();
-        }
-
     }
 
 }

searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/transform/ExpressionTransformer.java

@@ -20,11 +20,7 @@ public RankingExpression transform(RankingExpression expression, CONTEXT context
         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);
 
     /**

searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/evaluation/EvaluationTestCase.java

@@ -55,15 +55,6 @@ public void testStringValueToString() {
         verifyStringValueToString("\\\\x10081977");
     }
 
-    @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();
@@ -87,7 +78,6 @@ public void testEvaluation() {
         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 void testEvaluation() {
 
         // 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");