From 81ab3e0be448c1f456c7256ccc37e451f478b2c6 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Fri, 31 Mar 2023 20:11:50 +0200 Subject: [PATCH 1/2] Replace reflection by visitor --- .../schema/processing/IndexingValidation.java | 2 +- .../test/derived/indexswitches/ilscripts.cfg | 2 +- .../indexinglanguage/ExpressionConverter.java | 121 ++---------------- .../ValueTransformProvider.java | 2 +- .../expressions/ArithmeticExpression.java | 7 + .../expressions/CatExpression.java | 6 + .../expressions/ChoiceExpression.java | 6 + .../expressions/CompositeExpression.java | 4 + .../expressions/Expression.java | 6 + .../expressions/ExpressionList.java | 6 + .../expressions/ForEachExpression.java | 6 + .../expressions/GuardExpression.java | 6 + .../expressions/IfThenExpression.java | 10 ++ .../expressions/ParenthesisExpression.java | 6 + .../expressions/ScriptExpression.java | 10 ++ .../expressions/SelectInputExpression.java | 9 ++ .../expressions/StatementExpression.java | 10 ++ .../expressions/SwitchExpression.java | 12 ++ .../ExpressionConverterTestCase.java | 35 ----- 19 files changed, 115 insertions(+), 151 deletions(-) diff --git a/config-model/src/main/java/com/yahoo/schema/processing/IndexingValidation.java b/config-model/src/main/java/com/yahoo/schema/processing/IndexingValidation.java index d8c1fb3125f0..3c7e9b4066ff 100644 --- a/config-model/src/main/java/com/yahoo/schema/processing/IndexingValidation.java +++ b/config-model/src/main/java/com/yahoo/schema/processing/IndexingValidation.java @@ -62,7 +62,7 @@ private static class MyConverter extends ExpressionConverter { final Set prevNames = new HashSet<>(); @Override - protected ExpressionConverter branch() { + public ExpressionConverter branch() { MyConverter ret = new MyConverter(); ret.outputs.addAll(outputs); ret.prevNames.addAll(prevNames); diff --git a/config-model/src/test/derived/indexswitches/ilscripts.cfg b/config-model/src/test/derived/indexswitches/ilscripts.cfg index 77ac18e3261c..5cda0a9fdc7f 100644 --- a/config-model/src/test/derived/indexswitches/ilscripts.cfg +++ b/config-model/src/test/derived/indexswitches/ilscripts.cfg @@ -4,7 +4,7 @@ ilscript[].doctype "indexswitches" ilscript[].docfield[] "title" ilscript[].docfield[] "descr" ilscript[].docfield[] "source_src" -ilscript[].content[] "clear_state | guard { input source_src | switch { case \"theweb\": input source_src | tokenize normalize | summary source | index source; case \"amg\": input source_src | tokenize normalize | summary source; default: input source_src . \" partner\" | tokenize normalize | summary source | index source; }; }" +ilscript[].content[] "clear_state | guard { input source_src | switch { case \"amg\": input source_src | tokenize normalize | summary source; case \"theweb\": input source_src | tokenize normalize | summary source | index source; default: input source_src . \" partner\" | tokenize normalize | summary source | index source; }; }" ilscript[].content[] "clear_state | guard { input title | tokenize normalize stem:\"BEST\" | summary title | index title; }" ilscript[].content[] "clear_state | guard { input descr | tokenize normalize stem:\"BEST\" | summary descr | index descr; }" ilscript[].content[] "input source_src | passthrough source_src" diff --git a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/ExpressionConverter.java b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/ExpressionConverter.java index ccad9d6d08b9..231f6fb7598e 100644 --- a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/ExpressionConverter.java +++ b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/ExpressionConverter.java @@ -1,127 +1,22 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.indexinglanguage; -import com.yahoo.collections.Pair; -import com.yahoo.vespa.indexinglanguage.expressions.*; - -import java.lang.reflect.InvocationTargetException; -import java.util.HashMap; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; +import com.yahoo.vespa.indexinglanguage.expressions.Expression; /** * @author Simon Thoresen Hult */ -@SuppressWarnings({ "UnusedDeclaration" }) public abstract class ExpressionConverter implements Cloneable { - public final Expression convert(Expression exp) { - if (exp == null) { - return null; - } - if (shouldConvert(exp)) { - return doConvert(exp); - } - if (!(exp instanceof CompositeExpression)) { - return exp; - } - try { - // The class.getMethod here takes 8% of the cpu time in reading the SSBE application package - // TODO: Implement double dispatch through visitor instead? - return (Expression)ExpressionConverter.class.getMethod("innerConvert", exp.getClass()).invoke(this, exp); - } catch (IllegalAccessException | NoSuchMethodException e) { - throw new UnsupportedOperationException(exp.getClass().getName(), e); - } catch (InvocationTargetException e) { - Throwable t = e.getTargetException(); - throw t instanceof RuntimeException ? (RuntimeException)t : new RuntimeException(t); - } - } - - public Expression innerConvert(ArithmeticExpression exp) { - return new ArithmeticExpression(convert(exp.getLeftHandSide()), - exp.getOperator(), - convert(exp.getRightHandSide())); - } - - public Expression innerConvert(CatExpression exp) { - List lst = new LinkedList<>(); - for (Expression innerExp : exp) { - Expression next = convert(innerExp); - if (next != null) { - lst.add(next); - } - } - return new CatExpression(lst); - } - - public Expression innerConvert(ChoiceExpression exp) { - var convertedInnerExpressions = exp.asList().stream().map(inner -> convert(inner)).toList(); - return new ChoiceExpression(convertedInnerExpressions); - } - - public Expression innerConvert(ForEachExpression exp) { - return new ForEachExpression(convert(exp.getInnerExpression())); - } - - public Expression innerConvert(GuardExpression exp) { - return new GuardExpression(convert(exp.getInnerExpression())); - } - - public Expression innerConvert(IfThenExpression exp) { - return new IfThenExpression(branch().convert(exp.getLeftHandSide()), - exp.getComparator(), - branch().convert(exp.getRightHandSide()), - branch().convert(exp.getIfTrueExpression()), - branch().convert(exp.getIfFalseExpression())); - } - - public Expression innerConvert(ParenthesisExpression exp) { - return new ParenthesisExpression(convert(exp.getInnerExpression())); - } - - public Expression innerConvert(ScriptExpression exp) { - List lst = new LinkedList<>(); - for (Expression innerExp : exp) { - StatementExpression next = (StatementExpression)branch().convert(innerExp); - if (next != null) { - lst.add(next); - } - } - return new ScriptExpression(lst); - } - - public Expression innerConvert(SelectInputExpression exp) { - List> cases = new LinkedList<>(); - for (Pair pair : exp.getCases()) { - cases.add(new Pair<>(pair.getFirst(), branch().convert(pair.getSecond()))); - } - return new SelectInputExpression(cases); - } - - public Expression innerConvert(StatementExpression exp) { - List lst = new LinkedList<>(); - for (Expression innerExp : exp) { - Expression next = convert(innerExp); - if (next != null) { - lst.add(next); - } - } - return new StatementExpression(lst); - } - - public Expression innerConvert(SwitchExpression exp) { - Map cases = new HashMap<>(); - for (Map.Entry entry : exp.getCases().entrySet()) { - Expression next = branch().convert(entry.getValue()); - if (next != null) { - cases.put(entry.getKey(), next); - } - } - return new SwitchExpression(cases, branch().convert(exp.getDefaultExpression())); + public final Expression convert(Expression expression) { + if (expression == null) return null; + if (shouldConvert(expression)) + return doConvert(expression); + else + return expression.convertChildren(this); } - protected ExpressionConverter branch() { + public ExpressionConverter branch() { return this; } diff --git a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/ValueTransformProvider.java b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/ValueTransformProvider.java index bbd8c5ebcb84..623f940b06bb 100644 --- a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/ValueTransformProvider.java +++ b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/ValueTransformProvider.java @@ -20,7 +20,7 @@ public ValueTransformProvider(Class transformClass) { } @Override - protected final ExpressionConverter branch() { + public final ExpressionConverter branch() { return clone(); } diff --git a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ArithmeticExpression.java b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ArithmeticExpression.java index e4bc2dae965e..b7ee444975fe 100644 --- a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ArithmeticExpression.java +++ b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ArithmeticExpression.java @@ -4,6 +4,7 @@ import com.yahoo.document.DataType; import com.yahoo.document.NumericDataType; import com.yahoo.document.datatypes.*; +import com.yahoo.vespa.indexinglanguage.ExpressionConverter; import com.yahoo.vespa.objects.ObjectOperation; import com.yahoo.vespa.objects.ObjectPredicate; @@ -55,6 +56,12 @@ public ArithmeticExpression(Expression lhs, Operator op, Expression rhs) { this.rhs = rhs; } + @Override + public ArithmeticExpression convertChildren(ExpressionConverter converter) { + // TODO: branch()? + return new ArithmeticExpression(converter.convert(lhs), op, converter.convert(rhs)); + } + public Expression getLeftHandSide() { return lhs; } diff --git a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/CatExpression.java b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/CatExpression.java index 4c14c633fbf6..564ab015e10e 100644 --- a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/CatExpression.java +++ b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/CatExpression.java @@ -9,6 +9,7 @@ import com.yahoo.document.datatypes.FieldValue; import com.yahoo.document.datatypes.StringFieldValue; import com.yahoo.document.datatypes.WeightedSet; +import com.yahoo.vespa.indexinglanguage.ExpressionConverter; import java.util.*; @@ -25,6 +26,11 @@ public CatExpression(Collection lst) { super(lst, resolveInputType(lst)); } + @Override + public CatExpression convertChildren(ExpressionConverter converter) { + return new CatExpression(convertChildList(converter)); + } + @Override protected void doExecute(ExecutionContext context) { FieldValue input = context.getValue(); diff --git a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ChoiceExpression.java b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ChoiceExpression.java index 4f83cbfdd8c8..86826770828f 100644 --- a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ChoiceExpression.java +++ b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ChoiceExpression.java @@ -3,6 +3,7 @@ import com.yahoo.document.DataType; import com.yahoo.document.datatypes.FieldValue; +import com.yahoo.vespa.indexinglanguage.ExpressionConverter; import java.util.Arrays; import java.util.Collection; @@ -31,6 +32,11 @@ public ChoiceExpression(Collection choices) { super(choices, resolveInputType(choices)); } + @Override + public ChoiceExpression convertChildren(ExpressionConverter converter) { + return new ChoiceExpression(asList().stream().map(choice -> converter.branch().convert(choice)).toList()); + } + @Override protected void doExecute(ExecutionContext context) { FieldValue input = context.getValue(); diff --git a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/CompositeExpression.java b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/CompositeExpression.java index 27e5524f4ad0..8c00aad6bb04 100644 --- a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/CompositeExpression.java +++ b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/CompositeExpression.java @@ -2,12 +2,16 @@ package com.yahoo.vespa.indexinglanguage.expressions; import com.yahoo.document.DataType; +import com.yahoo.vespa.indexinglanguage.ExpressionConverter; /** * @author Simon Thoresen Hult */ public abstract class CompositeExpression extends Expression { + @Override + public abstract CompositeExpression convertChildren(ExpressionConverter converter); + protected CompositeExpression(DataType inputType) { super(inputType); } diff --git a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/Expression.java b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/Expression.java index bf8201ee7ee3..e5c60c953ef6 100644 --- a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/Expression.java +++ b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/Expression.java @@ -35,6 +35,12 @@ protected Expression(DataType inputType) { this.inputType = inputType; } + /** + * Returns an expression where the children of this has been converted using the given converter. + * This defauløt implementation returns this as it has no children. + */ + public Expression convertChildren(ExpressionConverter converter) { return this; } + /** Sets the document type and field the statement this expression is part of will write to */ public void setStatementOutput(DocumentType documentType, Field field) {} diff --git a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ExpressionList.java b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ExpressionList.java index e2ff1de7126c..57de66f80a0f 100644 --- a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ExpressionList.java +++ b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ExpressionList.java @@ -4,6 +4,7 @@ import com.yahoo.document.DataType; import com.yahoo.document.DocumentType; import com.yahoo.document.Field; +import com.yahoo.vespa.indexinglanguage.ExpressionConverter; import com.yahoo.vespa.objects.ObjectOperation; import com.yahoo.vespa.objects.ObjectPredicate; @@ -11,6 +12,7 @@ import java.util.Iterator; import java.util.LinkedList; import java.util.List; +import java.util.Objects; /** * @author Simon Thoresen Hult @@ -26,6 +28,10 @@ protected ExpressionList(Iterable lst, DataType inputType) { } } + protected List convertChildList(ExpressionConverter converter) { + return asList().stream().map(converter::convert).filter(Objects::nonNull).toList(); + } + @Override public void setStatementOutput(DocumentType documentType, Field field) { for (Expression expression : expressions) diff --git a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ForEachExpression.java b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ForEachExpression.java index 0f3a445bcb95..3053a391823c 100644 --- a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ForEachExpression.java +++ b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ForEachExpression.java @@ -6,6 +6,7 @@ import com.yahoo.document.datatypes.FieldValue; import com.yahoo.document.datatypes.Struct; import com.yahoo.document.datatypes.WeightedSet; +import com.yahoo.vespa.indexinglanguage.ExpressionConverter; import com.yahoo.vespa.indexinglanguage.FieldValueConverter; import com.yahoo.vespa.objects.ObjectOperation; import com.yahoo.vespa.objects.ObjectPredicate; @@ -26,6 +27,11 @@ public Expression getInnerExpression() { return exp; } + @Override + public ForEachExpression convertChildren(ExpressionConverter converter) { + return new ForEachExpression(converter.convert(exp)); + } + @Override public void setStatementOutput(DocumentType documentType, Field field) { exp.setStatementOutput(documentType, field); diff --git a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/GuardExpression.java b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/GuardExpression.java index da7cfcdcaee2..38a05c3056c7 100644 --- a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/GuardExpression.java +++ b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/GuardExpression.java @@ -4,6 +4,7 @@ import com.yahoo.document.DataType; import com.yahoo.document.DocumentType; import com.yahoo.document.Field; +import com.yahoo.vespa.indexinglanguage.ExpressionConverter; import com.yahoo.vespa.indexinglanguage.ExpressionVisitor; import com.yahoo.vespa.indexinglanguage.UpdateAdapter; import com.yahoo.vespa.objects.ObjectOperation; @@ -27,6 +28,11 @@ public Expression getInnerExpression() { return exp; } + @Override + public GuardExpression convertChildren(ExpressionConverter converter) { + return new GuardExpression(converter.convert(exp)); + } + @Override public void setStatementOutput(DocumentType documentType, Field field) { exp.setStatementOutput(documentType, field); diff --git a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/IfThenExpression.java b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/IfThenExpression.java index 8a29c8e8645f..f05795aa2347 100644 --- a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/IfThenExpression.java +++ b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/IfThenExpression.java @@ -6,6 +6,7 @@ import com.yahoo.document.Field; import com.yahoo.document.datatypes.FieldValue; import com.yahoo.document.datatypes.NumericFieldValue; +import com.yahoo.vespa.indexinglanguage.ExpressionConverter; import com.yahoo.vespa.objects.ObjectOperation; import com.yahoo.vespa.objects.ObjectPredicate; @@ -56,6 +57,15 @@ public IfThenExpression(Expression lhs, Comparator cmp, Expression rhs, Expressi this.ifFalse = ifFalse; } + @Override + public IfThenExpression convertChildren(ExpressionConverter converter) { + return new IfThenExpression(converter.branch().convert(lhs), + cmp, + converter.branch().convert(rhs), + converter.branch().convert(ifTrue), + converter.branch().convert(ifFalse)); + } + @Override public void setStatementOutput(DocumentType documentType, Field field) { lhs.setStatementOutput(documentType, field); diff --git a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ParenthesisExpression.java b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ParenthesisExpression.java index 60b059f3ef18..6e476f5f7e49 100644 --- a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ParenthesisExpression.java +++ b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ParenthesisExpression.java @@ -4,6 +4,7 @@ import com.yahoo.document.DataType; import com.yahoo.document.DocumentType; import com.yahoo.document.Field; +import com.yahoo.vespa.indexinglanguage.ExpressionConverter; import com.yahoo.vespa.objects.ObjectOperation; import com.yahoo.vespa.objects.ObjectPredicate; @@ -23,6 +24,11 @@ public Expression getInnerExpression() { return innerExp; } + @Override + public ParenthesisExpression convertChildren(ExpressionConverter converter) { + return new ParenthesisExpression(converter.convert(innerExp)); + } + @Override public void setStatementOutput(DocumentType documentType, Field field) { innerExp.setStatementOutput(documentType, field); diff --git a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ScriptExpression.java b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ScriptExpression.java index f0c37960a993..1a640c9924ef 100644 --- a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ScriptExpression.java +++ b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ScriptExpression.java @@ -6,6 +6,7 @@ import com.yahoo.language.Linguistics; import com.yahoo.language.process.Embedder; import com.yahoo.language.simple.SimpleLinguistics; +import com.yahoo.vespa.indexinglanguage.ExpressionConverter; import com.yahoo.vespa.indexinglanguage.ScriptParser; import com.yahoo.vespa.indexinglanguage.ScriptParserContext; import com.yahoo.vespa.indexinglanguage.parser.IndexingInput; @@ -17,6 +18,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Objects; /** * @author Simon Thoresen Hult @@ -35,6 +37,14 @@ public ScriptExpression(Collection lst) { super(lst, resolveInputType(lst)); } + @Override + public ScriptExpression convertChildren(ExpressionConverter converter) { + return new ScriptExpression(asList().stream() + .map(child -> (StatementExpression)converter.branch().convert(child)) + .filter(Objects::nonNull) + .toList()); + } + @Override protected void doExecute(ExecutionContext context) { FieldValue input = context.getValue(); diff --git a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/SelectInputExpression.java b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/SelectInputExpression.java index 212b60525f9d..bb8111f358e3 100644 --- a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/SelectInputExpression.java +++ b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/SelectInputExpression.java @@ -6,10 +6,12 @@ import com.yahoo.document.DocumentType; import com.yahoo.document.Field; import com.yahoo.document.datatypes.FieldValue; +import com.yahoo.vespa.indexinglanguage.ExpressionConverter; import com.yahoo.vespa.objects.ObjectOperation; import com.yahoo.vespa.objects.ObjectPredicate; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.List; @@ -31,6 +33,13 @@ public SelectInputExpression(List> cases) { this.cases = cases; } + @Override + public SelectInputExpression convertChildren(ExpressionConverter converter) { + return new SelectInputExpression(cases.stream() + .map(c -> new Pair<>(c.getFirst(), converter.branch().convert(c.getSecond()))) + .toList()); + } + @Override public void setStatementOutput(DocumentType documentType, Field field) { for (var casePair : cases) diff --git a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/StatementExpression.java b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/StatementExpression.java index 8516ddb883dc..75f206ef47d9 100644 --- a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/StatementExpression.java +++ b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/StatementExpression.java @@ -5,6 +5,7 @@ import com.yahoo.language.Linguistics; import com.yahoo.language.process.Embedder; import com.yahoo.language.simple.SimpleLinguistics; +import com.yahoo.vespa.indexinglanguage.ExpressionConverter; import com.yahoo.vespa.indexinglanguage.ScriptParser; import com.yahoo.vespa.indexinglanguage.ScriptParserContext; import com.yahoo.vespa.indexinglanguage.parser.IndexingInput; @@ -15,6 +16,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.stream.Collectors; /** @@ -44,6 +46,14 @@ private StatementExpression(Iterable list, Object unused) { /** Returns the input fields which are (perhaps optionally) consumed by some expression in this statement. */ public List getInputFields() { return inputFields; } + @Override + public StatementExpression convertChildren(ExpressionConverter converter) { + return new StatementExpression(asList().stream() + .map(child -> converter.convert(child)) + .filter(Objects::nonNull) + .toList()); + } + @Override protected void doExecute(ExecutionContext context) { for (Expression expression : this) { diff --git a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/SwitchExpression.java b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/SwitchExpression.java index 86913d8c1ba9..c7cf70664838 100644 --- a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/SwitchExpression.java +++ b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/SwitchExpression.java @@ -7,6 +7,7 @@ import com.yahoo.document.datatypes.FieldValue; import com.yahoo.document.datatypes.StringFieldValue; import com.yahoo.text.StringUtilities; +import com.yahoo.vespa.indexinglanguage.ExpressionConverter; import com.yahoo.vespa.objects.ObjectOperation; import com.yahoo.vespa.objects.ObjectPredicate; @@ -32,6 +33,17 @@ public SwitchExpression(Map cases, Expression this.cases.putAll(cases); } + @Override + public SwitchExpression convertChildren(ExpressionConverter converter) { + var convertedCases = new LinkedHashMap(); + for (var entry : cases.entrySet()) { + var converted = converter.branch().convert(entry.getValue()); + if (converted != null) + convertedCases.put(entry.getKey(), converted); + } + return new SwitchExpression(convertedCases, converter.branch().convert(defaultExp)); + } + public boolean isEmpty() { return defaultExp == null && cases.isEmpty(); } diff --git a/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/ExpressionConverterTestCase.java b/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/ExpressionConverterTestCase.java index f1e1be0ae413..8aeaa084e1ba 100644 --- a/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/ExpressionConverterTestCase.java +++ b/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/ExpressionConverterTestCase.java @@ -2,7 +2,6 @@ package com.yahoo.vespa.indexinglanguage; import com.yahoo.collections.Pair; -import com.yahoo.document.DataType; import com.yahoo.document.datatypes.IntegerFieldValue; import com.yahoo.language.simple.SimpleLinguistics; import com.yahoo.vespa.indexinglanguage.expressions.ArithmeticExpression; @@ -11,9 +10,7 @@ import com.yahoo.vespa.indexinglanguage.expressions.Base64EncodeExpression; import com.yahoo.vespa.indexinglanguage.expressions.CatExpression; import com.yahoo.vespa.indexinglanguage.expressions.ClearStateExpression; -import com.yahoo.vespa.indexinglanguage.expressions.CompositeExpression; import com.yahoo.vespa.indexinglanguage.expressions.EchoExpression; -import com.yahoo.vespa.indexinglanguage.expressions.ExecutionContext; import com.yahoo.vespa.indexinglanguage.expressions.Expression; import com.yahoo.vespa.indexinglanguage.expressions.ForEachExpression; import com.yahoo.vespa.indexinglanguage.expressions.GetFieldExpression; @@ -54,7 +51,6 @@ import com.yahoo.vespa.indexinglanguage.expressions.ToWsetExpression; import com.yahoo.vespa.indexinglanguage.expressions.TokenizeExpression; import com.yahoo.vespa.indexinglanguage.expressions.TrimExpression; -import com.yahoo.vespa.indexinglanguage.expressions.VerificationContext; import com.yahoo.vespa.indexinglanguage.expressions.ZCurveExpression; import com.yahoo.vespa.indexinglanguage.linguistics.AnnotatorConfig; import org.junit.Test; @@ -73,7 +69,6 @@ */ public class ExpressionConverterTestCase { - @SuppressWarnings("unchecked") @Test public void requireThatAllExpressionTypesCanBeTraversed() { assertConvertable(new ArithmeticExpression(new InputExpression("foo"), ArithmeticExpression.Operator.ADD, @@ -166,16 +161,6 @@ public void requireThatSwitchElementsCanBeRemoved() { assertEquals(1, ((SwitchExpression)after).getCases().size()); } - @Test - public void requireThatUnknownCompositeThrows() { - try { - new MyTraverser().convert(new MyComposite()); - fail(); - } catch (UnsupportedOperationException e) { - assertEquals(NoSuchMethodException.class, e.getCause().getClass()); - } - } - @Test public void requireThatConversionExceptionCanBeThrown() { final RuntimeException expectedCause = new RuntimeException(); @@ -254,24 +239,4 @@ protected Expression doConvert(Expression exp) { } } - private static class MyComposite extends CompositeExpression { - - MyComposite() { - super(null); - } - @Override - protected void doExecute(ExecutionContext context) { - - } - - @Override - protected void doVerify(VerificationContext context) { - - } - - @Override - public DataType createdOutputType() { - return null; - } - } } From 878a918b39185800b7720510336e589deb13c301 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Wed, 12 Apr 2023 21:03:48 +0200 Subject: [PATCH 2/2] Update indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/Expression.java Co-authored-by: Geir Storli --- .../yahoo/vespa/indexinglanguage/expressions/Expression.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/Expression.java b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/Expression.java index e5c60c953ef6..f498b871096b 100644 --- a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/Expression.java +++ b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/Expression.java @@ -37,7 +37,7 @@ protected Expression(DataType inputType) { /** * Returns an expression where the children of this has been converted using the given converter. - * This defauløt implementation returns this as it has no children. + * This default implementation returns this as it has no children. */ public Expression convertChildren(ExpressionConverter converter) { return this; }