diff options
author | Arne Juul <arnej@yahooinc.com> | 2023-03-17 10:32:08 +0000 |
---|---|---|
committer | Arne Juul <arnej@yahooinc.com> | 2023-03-17 13:43:26 +0000 |
commit | 09c6802922df60c9ddea8fbe044006f2c645a9c7 (patch) | |
tree | 69b20547b1568078f15163d161d6fa2e07ee5497 /searchlib | |
parent | 0ee0a814b61bb356a8ee526189bcdb57cda003f2 (diff) |
avoid making OperationNode with no operators
Diffstat (limited to 'searchlib')
-rwxr-xr-x | searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/OperationNode.java | 18 | ||||
-rw-r--r-- | searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/transform/Simplifier.java | 5 |
2 files changed, 19 insertions, 4 deletions
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)) { |