summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorArne Juul <arnej@yahooinc.com>2023-03-17 10:32:08 +0000
committerArne Juul <arnej@yahooinc.com>2023-03-17 13:43:26 +0000
commit09c6802922df60c9ddea8fbe044006f2c645a9c7 (patch)
tree69b20547b1568078f15163d161d6fa2e07ee5497
parent0ee0a814b61bb356a8ee526189bcdb57cda003f2 (diff)
avoid making OperationNode with no operators
-rw-r--r--config-model/src/main/java/com/yahoo/schema/expressiontransforms/TokenTransformer.java3
-rwxr-xr-xsearchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/OperationNode.java18
-rw-r--r--searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/transform/Simplifier.java5
3 files changed, 22 insertions, 4 deletions
diff --git a/config-model/src/main/java/com/yahoo/schema/expressiontransforms/TokenTransformer.java b/config-model/src/main/java/com/yahoo/schema/expressiontransforms/TokenTransformer.java
index cf354a05a93..de12de9b747 100644
--- a/config-model/src/main/java/com/yahoo/schema/expressiontransforms/TokenTransformer.java
+++ b/config-model/src/main/java/com/yahoo/schema/expressiontransforms/TokenTransformer.java
@@ -289,6 +289,9 @@ public class TokenTransformer extends ExpressionTransformer<RankProfileTransform
operators.add(Operator.plus);
}
}
+ if (operators.isEmpty() && factors.size() == 1) {
+ return factors.get(0);
+ }
return new OperationNode(factors, operators);
}
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)) {