From 09c6802922df60c9ddea8fbe044006f2c645a9c7 Mon Sep 17 00:00:00 2001
From: Arne Juul <arnej@yahooinc.com>
Date: Fri, 17 Mar 2023 10:32:08 +0000
Subject: avoid making OperationNode with no operators

---
 .../rankingexpression/rule/OperationNode.java          | 18 +++++++++++++++---
 .../rankingexpression/transform/Simplifier.java        |  5 ++++-
 2 files changed, 19 insertions(+), 4 deletions(-)

(limited to 'searchlib')

diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/OperationNode.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/OperationNode.java
index 1c66686a9fe..7ebbf0d582b 100755
--- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/OperationNode.java
+++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/OperationNode.java
@@ -14,6 +14,7 @@ import java.util.Deque;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Objects;
+import java.util.logging.Logger;
 
 /**
  * A sequence of binary operations.
@@ -22,12 +23,21 @@ import java.util.Objects;
  */
 public final class OperationNode extends CompositeNode {
 
+    private static final Logger logger = Logger.getLogger(OperationNode.class.getName());
+
     private final List<ExpressionNode> children;
     private final List<Operator> operators;
 
     public OperationNode(List<ExpressionNode> children, List<Operator> operators) {
         this.children = List.copyOf(children);
         this.operators = List.copyOf(operators);
+        if (operators.isEmpty()) {
+            logger.warning("Strange: no operators for OperationNode");
+        }
+        int needChildren = operators.size() + 1;
+        if (needChildren != children.size()) {
+            throw new IllegalArgumentException("Need " + needChildren + " children, but got " + children.size());
+        }
     }
 
     public OperationNode(ExpressionNode leftExpression, Operator operator, ExpressionNode rightExpression) {
@@ -70,12 +80,14 @@ public final class OperationNode extends CompositeNode {
         if ( parent == null) return false;
         if ( ! (parent instanceof OperationNode operationParent)) return false;
 
-        // The line below can only be correct in both only have one operator.
+        // The last line below can only be correct if 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
         // extra parenthesis just in case....
-        return operationParent.operators.get(0).hasPrecedenceOver(this.operators.get(0))
-                || ((operationParent.operators.size() > 1) || (operators.size() > 1));
+        if ((operationParent.operators.size() != 1) || (operators.size() != 1)) {
+            return true;
+        }
+        return operationParent.operators.get(0).hasPrecedenceOver(this.operators.get(0));
     }
 
     @Override
diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/transform/Simplifier.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/transform/Simplifier.java
index 1c1f7509ce8..4293ff29d0b 100644
--- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/transform/Simplifier.java
+++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/transform/Simplifier.java
@@ -53,6 +53,9 @@ public class Simplifier extends ExpressionTransformer<TransformContext> {
             List<Operator> operators = new ArrayList<>(node.operators());
             for (Operator operator : Operator.operatorsByPrecedence)
                 transform(operator, children, operators);
+            if (operators.isEmpty() && children.size() == 1) {
+                return children.get(0);
+            }
             node = new OperationNode(children, operators);
         }
 
@@ -69,7 +72,7 @@ public class Simplifier extends ExpressionTransformer<TransformContext> {
         int i = 0;
         while (i < children.size()-1) {
             boolean transformed = false;
-            if ( operators.get(i).equals(operatorToTransform)) {
+            if (operators.get(i).equals(operatorToTransform)) {
                 ExpressionNode child1 = children.get(i);
                 ExpressionNode child2 = children.get(i + 1);
                 if (isConstant(child1) && isConstant(child2) && hasPrecedence(operators, i)) {
-- 
cgit v1.2.3