diff options
16 files changed, 125 insertions, 35 deletions
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/derived/RawRankProfile.java b/config-model/src/main/java/com/yahoo/searchdefinition/derived/RawRankProfile.java index b075bce7eaf..1a22b98fd9f 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/derived/RawRankProfile.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/derived/RawRankProfile.java @@ -406,7 +406,7 @@ public class RawRankProfile implements RankProfilesConfig.Producer { return properties; } - private List<Pair<String, String>> deriveRankingPhaseRankProperties(RankingExpression expression, String phase) { + private List<Pair<String, String>> deriveRankingPhaseRankProperties(RankingExpression expression, String phase) { List<Pair<String, String>> properties = new ArrayList<>(); if (expression == null) return properties; diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/FunctionShadower.java b/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/FunctionShadower.java index c3ee7d5fc3d..bb2e20a4f05 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/FunctionShadower.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/FunctionShadower.java @@ -44,9 +44,8 @@ public class FunctionShadower extends ExpressionTransformer<RankProfileTransform private ExpressionNode transformFunctionNode(FunctionNode function, RankProfileTransformContext context) { String name = function.getFunction().toString(); RankProfile.RankingExpressionFunction rankingExpressionFunction = context.rankProfile().findFunction(name); - if (rankingExpressionFunction == null) { + if (rankingExpressionFunction == null) return transformChildren(function, context); - } int functionArity = function.getFunction().arity(); if (functionArity != rankingExpressionFunction.function().arguments().size()) diff --git a/config-model/src/test/derived/tensor/rank-profiles.cfg b/config-model/src/test/derived/tensor/rank-profiles.cfg index e7b946e4929..554a36aef86 100644 --- a/config-model/src/test/derived/tensor/rank-profiles.cfg +++ b/config-model/src/test/derived/tensor/rank-profiles.cfg @@ -133,7 +133,7 @@ rankprofile[].fef.property[].value "3" rankprofile[].fef.property[].name "vespa.rank.firstphase" rankprofile[].fef.property[].value "rankingExpression(firstphase)" rankprofile[].fef.property[].name "rankingExpression(firstphase).rankingScript" -rankprofile[].fef.property[].value "reduce(tensor(d0[1])(attribute{x:(functionNotLabel)}), sum)" +rankprofile[].fef.property[].value "reduce(tensor(d0[1])(attribute{x:(rankingExpression(functionNotLabel))}), sum)" rankprofile[].fef.property[].name "vespa.type.attribute.f2" rankprofile[].fef.property[].value "tensor<float>(x[2],y[1])" rankprofile[].fef.property[].name "vespa.type.attribute.f3" @@ -142,3 +142,19 @@ rankprofile[].fef.property[].name "vespa.type.attribute.f4" rankprofile[].fef.property[].value "tensor(x[10],y[20])" rankprofile[].fef.property[].name "vespa.type.attribute.f5" rankprofile[].fef.property[].value "tensor<float>(x[10])" +rankprofile[].name "profile9" +rankprofile[].fef.property[].name "rankingExpression(shadow).rankingScript" +rankprofile[].fef.property[].value "3" +rankprofile[].fef.property[].name "vespa.rank.firstphase" +rankprofile[].fef.property[].value "rankingExpression(firstphase)" +rankprofile[].fef.property[].name "rankingExpression(firstphase).rankingScript" +rankprofile[].fef.property[].value "reduce(tensor(shadow[1])(attribute{x:shadow + rankingExpression(shadow)}), sum)" +rankprofile[].fef.property[].name "vespa.type.attribute.f2" +rankprofile[].fef.property[].value "tensor<float>(x[2],y[1])" +rankprofile[].fef.property[].name "vespa.type.attribute.f3" +rankprofile[].fef.property[].value "tensor(x{})" +rankprofile[].fef.property[].name "vespa.type.attribute.f4" +rankprofile[].fef.property[].value "tensor(x[10],y[20])" +rankprofile[].fef.property[].name "vespa.type.attribute.f5" +rankprofile[].fef.property[].value "tensor<float>(x[10])" + diff --git a/config-model/src/test/derived/tensor/tensor.sd b/config-model/src/test/derived/tensor/tensor.sd index aac17668c01..c3380bed19c 100644 --- a/config-model/src/test/derived/tensor/tensor.sd +++ b/config-model/src/test/derived/tensor/tensor.sd @@ -102,4 +102,17 @@ search tensor { } + rank-profile profile9 { + + # shadow refers to the generate index and shadow() to the function + first-phase { + expression: sum(tensor(shadow[1])(attribute{x: shadow + shadow() })) + } + + function shadow() { + expression: 3 + } + + } + } diff --git a/searchlib/abi-spec.json b/searchlib/abi-spec.json index abfae426ad0..db9c2d46534 100644 --- a/searchlib/abi-spec.json +++ b/searchlib/abi-spec.json @@ -348,6 +348,7 @@ "public" ], "methods": [ + "public static com.yahoo.searchlib.rankingexpression.Reference fromIdentifier(java.lang.String)", "public void <init>(java.lang.String, com.yahoo.searchlib.rankingexpression.rule.Arguments, java.lang.String)", "public com.yahoo.searchlib.rankingexpression.rule.Arguments arguments()", "public java.lang.String output()", diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/RankingExpression.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/RankingExpression.java index 722520fea08..18f6c6f2ca2 100755 --- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/RankingExpression.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/RankingExpression.java @@ -265,8 +265,8 @@ public class RankingExpression implements Serializable { /** * Returns the rank-property name for a given expression name. * - * @param expressionName The expression name to mangle. - * @return The property name. + * @param expressionName the expression name to mangle. + * @return the property name. */ public static String propertyName(String expressionName) { return "rankingExpression(" + expressionName + ").rankingScript"; diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/Reference.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/Reference.java index fa2d0f1ee45..3c537b53e9d 100644 --- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/Reference.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/Reference.java @@ -27,15 +27,28 @@ public class Reference extends Name { /** The output, or null if none */ private final String output; + /** True if this was created by the "fromIdentifier" method. This lets us separate 'foo()' and 'foo' */ + private final boolean isIdentifier; + + public static Reference fromIdentifier(String identifier) { + return new Reference(identifier, new Arguments(), null, true); + } + public Reference(String name, Arguments arguments, String output) { + this(name, arguments, output, false); + } + + private Reference(String name, Arguments arguments, String output, boolean isIdentifier) { super(name); Objects.requireNonNull(name, "name cannot be null"); Objects.requireNonNull(arguments, "arguments cannot be null"); this.arguments = arguments; this.output = output; - this.hashCode = Objects.hash(name(), arguments, output); + this.hashCode = Objects.hash(name(), arguments, output, isIdentifier); + this.isIdentifier = isIdentifier; } + public Arguments arguments() { return arguments; } public String output() { return output; } @@ -66,12 +79,8 @@ public class Reference extends Name { return Optional.of(simple(featureName, argument)); } - /** - * Returns whether this is a simple identifier - no arguments or output - */ - public boolean isIdentifier() { - return this.arguments.expressions().size() == 0 && output == null; - } + /** Returns true if this was created by fromIdentifier. Identifiers have no arguments or outputs. */ + public boolean isIdentifier() { return isIdentifier; } /** * A <i>simple feature reference</i> is a reference with a single identifier argument @@ -105,11 +114,11 @@ public class Reference extends Name { } public Reference withArguments(Arguments arguments) { - return new Reference(name(), arguments, output); + return new Reference(name(), arguments, output, isIdentifier && arguments.isEmpty()); } public Reference withOutput(String output) { - return new Reference(name(), arguments, output); + return new Reference(name(), arguments, output, isIdentifier && output == null); } @Override @@ -121,6 +130,7 @@ public class Reference extends Name { if (!Objects.equals(other.name(), this.name())) return false; if (!Objects.equals(other.arguments, this.arguments)) return false; if (!Objects.equals(other.output, this.output)) return false; + if (!Objects.equals(other.isIdentifier, this.isIdentifier)) return false; return true; } diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/Value.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/Value.java index 39e408d27ca..382cbb7ce9a 100644 --- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/Value.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/Value.java @@ -76,7 +76,7 @@ public abstract class Value { * @return this for convenience */ public Value freeze() { - frozen=true; + frozen = true; return this; } diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/ReferenceNode.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/ReferenceNode.java index 7312863fa26..62b3379f635 100755 --- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/ReferenceNode.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/ReferenceNode.java @@ -13,6 +13,7 @@ import com.yahoo.tensor.evaluation.TypeContext; import java.util.ArrayDeque; import java.util.Deque; import java.util.List; +import java.util.Optional; /** * A node referring either to a value in the context or to a named ranking expression function. @@ -25,7 +26,7 @@ public final class ReferenceNode extends CompositeNode { /* Creates a node with a simple identifier reference */ public ReferenceNode(String name) { - this(name, null, null); + this.reference = Reference.fromIdentifier(name); } public ReferenceNode(String name, List<? extends ExpressionNode> arguments, String output) { @@ -88,7 +89,6 @@ public final class ReferenceNode extends CompositeNode { context.addFunctionSerialization(RankingExpression.propertyName(instance.getName()), instance.getExpressionString()); return string.append("rankingExpression(").append(instance.getName()).append(')'); } - // Not resolved in this context: output as-is return reference.toString(string, context, path, parent); } diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/TensorFunctionNode.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/TensorFunctionNode.java index cec8837abcd..a0c261ae9d3 100644 --- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/TensorFunctionNode.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/TensorFunctionNode.java @@ -119,7 +119,7 @@ public class TensorFunctionNode extends CompositeNode { public String toString(ToStringContext c) { if (c instanceof ExpressionToStringContext) { ExpressionToStringContext context = (ExpressionToStringContext) c; - return expression.toString(new StringBuilder(),context.context, context.path, context.parent).toString(); + return expression.toString(new StringBuilder(), context.context, context.path, context.parent).toString(); } else { return expression.toString(); @@ -182,7 +182,7 @@ public class TensorFunctionNode extends CompositeNode { public String toString(ToStringContext c) { if (c instanceof ExpressionToStringContext) { ExpressionToStringContext context = (ExpressionToStringContext) c; - return expression.toString(new StringBuilder(),context.context, context.path, context.parent).toString(); + return expression.toString(new StringBuilder(), context.context, context.path, context.parent).toString(); } else { return expression.toString(); diff --git a/searchlib/src/main/javacc/RankingExpressionParser.jj b/searchlib/src/main/javacc/RankingExpressionParser.jj index c7870182939..fdad824cd1b 100755 --- a/searchlib/src/main/javacc/RankingExpressionParser.jj +++ b/searchlib/src/main/javacc/RankingExpressionParser.jj @@ -275,7 +275,12 @@ ReferenceNode feature() : } { ( name = identifier() [ <LBRACE> args = args() <RBRACE> ] [ <DOT> out = outs() ] ) - { return new ReferenceNode(name, args, out); } + { + if (args == null && out == null) // know the difference between "foo" and "foo()" + return new ReferenceNode(name); + else + return new ReferenceNode(name, args, out); + } } // Rank properties are referenced by $propertyname diff --git a/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/RankingExpressionTestCase.java b/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/RankingExpressionTestCase.java index 274cb0b567e..7eb1fecc0cb 100755 --- a/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/RankingExpressionTestCase.java +++ b/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/RankingExpressionTestCase.java @@ -159,6 +159,48 @@ public class RankingExpressionTestCase { } @Test + public void testFunctionInTensorSerialization() throws ParseException { + List<ExpressionFunction> functions = new ArrayList<>(); + functions.add(new ExpressionFunction("scalarFunction", List.of(), new RankingExpression("5"))); + functions.add(new ExpressionFunction("tensorFunction", List.of(), new RankingExpression("tensor(x[3]):[1, 2, 3]"))); + + // Getting a value from a tensor supplied by a function, inside a tensor generate function + assertSerialization(List.of("tensor(x[3])(rankingExpression(tensorFunction)[x])"), + "tensor(x[3])(tensorFunction[x])", + functions, false); + + // Getting a value from a tensor supplied by a function, where the value index is supplied by a function, inside a tensor generate function, short form + assertSerialization(List.of("tensor(x[3])(rankingExpression(tensorFunction)[rankingExpression(scalarFunction)])"), + "tensor(x[3])(tensorFunction[scalarFunction()])", + functions, false); + + // 'scalarFunction' is interpreted as a label here since it is the short form of a mapped dimension + assertSerialization(List.of("tensor(x[3])(rankingExpression(tensorFunction){scalarFunction})"), + "tensor(x[3])(tensorFunction{scalarFunction})", + functions, false); + + // Getting a value from a tensor supplied by a function, where the value index is supplied by a function, inside a tensor generate function, long form + assertSerialization(List.of("tensor(x[3])(rankingExpression(tensorFunction){x:rankingExpression(scalarFunction)})"), + "tensor(x[3])(tensorFunction{x:scalarFunction()})", + functions, false); + + // 'scalarFunction' without parentheses is interpreted as a label instead of a reference to the function + assertSerialization(List.of("tensor(x[3])(rankingExpression(tensorFunction){x:scalarFunction})"), + "tensor(x[3])(tensorFunction{x:scalarFunction})", + functions, false); + + // Accessing a function in a dynamic tensor, short form + assertSerialization(List.of("tensor(x[2]):{{x:0}:rankingExpression(scalarFunction),{x:1}:rankingExpression(scalarFunction)}"), + "tensor(x[2]):[scalarFunction(), scalarFunction()]]", + functions, false); + + // Accessing a function in a dynamic tensor, long form + assertSerialization(List.of("tensor(x{}):{{x:foo}:rankingExpression(scalarFunction),{x:bar}:rankingExpression(scalarFunction)}"), + "tensor(x{}):{{x:foo}:scalarFunction(), {x:bar}:scalarFunction()}", + functions, false); + } + + @Test public void testBug3464208() throws ParseException { List<ExpressionFunction> functions = new ArrayList<>(); functions.add(new ExpressionFunction("log10tweetage", null, new RankingExpression("69"))); @@ -323,7 +365,8 @@ public class RankingExpressionTestCase { } } - private void assertSerialization(List<String> expectedSerialization, String expressionString, + private void assertSerialization(List<String> expectedSerialization, + String expressionString, List<ExpressionFunction> functions) { assertSerialization(expectedSerialization, expressionString, functions, false); } @@ -331,13 +374,13 @@ public class RankingExpressionTestCase { List<ExpressionFunction> functions, boolean print) { try { if (print) - System.out.println("Parsing expression '" + expressionString + "'."); + System.out.println("Parsing expression '" + expressionString + "':"); RankingExpression expression = new RankingExpression(expressionString); Map<String, String> rankProperties = expression.getRankProperties(functions); if (print) { for (String key : rankProperties.keySet()) - System.out.println("Property '" + key + "': " + rankProperties.get(key)); + System.out.println(key + ": " + rankProperties.get(key)); } for (int i = 0; i < expectedSerialization.size();) { String val = expectedSerialization.get(i++); diff --git a/vespajlib/abi-spec.json b/vespajlib/abi-spec.json index cefdbb09b7a..8aca75ac1be 100644 --- a/vespajlib/abi-spec.json +++ b/vespajlib/abi-spec.json @@ -2526,7 +2526,8 @@ "public java.util.Optional dimension()", "public java.util.Optional label()", "public java.util.Optional index()", - "public java.lang.String toString()" + "public java.lang.String toString()", + "public java.lang.String toString(com.yahoo.tensor.functions.ToStringContext)" ], "fields": [] }, diff --git a/vespajlib/src/main/java/com/yahoo/tensor/functions/DynamicTensor.java b/vespajlib/src/main/java/com/yahoo/tensor/functions/DynamicTensor.java index 416940a60eb..0a496cda5d9 100644 --- a/vespajlib/src/main/java/com/yahoo/tensor/functions/DynamicTensor.java +++ b/vespajlib/src/main/java/com/yahoo/tensor/functions/DynamicTensor.java @@ -83,7 +83,7 @@ public abstract class DynamicTensor<NAMETYPE extends Name> extends PrimitiveTens String contentToString(ToStringContext context) { if (type().dimensions().isEmpty()) { if (cells.isEmpty()) return "{}"; - return "{" + cells.values().iterator().next() + "}"; + return "{" + cells.values().iterator().next().toString(context) + "}"; } StringBuilder b = new StringBuilder("{"); @@ -124,7 +124,7 @@ public abstract class DynamicTensor<NAMETYPE extends Name> extends PrimitiveTens String contentToString(ToStringContext context) { if (type().dimensions().isEmpty()) { if (cells.isEmpty()) return "{}"; - return "{" + cells.get(0) + "}"; + return "{" + cells.get(0).toString(context) + "}"; } IndexedTensor.Indexes indexes = IndexedTensor.Indexes.of(type()); diff --git a/vespajlib/src/main/java/com/yahoo/tensor/functions/ScalarFunction.java b/vespajlib/src/main/java/com/yahoo/tensor/functions/ScalarFunction.java index 07b3658fb58..ec579a90e4f 100644 --- a/vespajlib/src/main/java/com/yahoo/tensor/functions/ScalarFunction.java +++ b/vespajlib/src/main/java/com/yahoo/tensor/functions/ScalarFunction.java @@ -16,8 +16,6 @@ public interface ScalarFunction<NAMETYPE extends Name> extends Function<Evaluati @Override Double apply(EvaluationContext<NAMETYPE> context); - default String toString(ToStringContext context) { - return toString(); - } + default String toString(ToStringContext context) { return toString(); } } diff --git a/vespajlib/src/main/java/com/yahoo/tensor/functions/Value.java b/vespajlib/src/main/java/com/yahoo/tensor/functions/Value.java index fff9862279c..37a54807673 100644 --- a/vespajlib/src/main/java/com/yahoo/tensor/functions/Value.java +++ b/vespajlib/src/main/java/com/yahoo/tensor/functions/Value.java @@ -78,15 +78,15 @@ public class Value<NAMETYPE extends Name> extends PrimitiveTensorFunction<NAMETY @Override public String toString(ToStringContext context) { - StringBuilder b = new StringBuilder(argument.toString()); + StringBuilder b = new StringBuilder(argument.toString(context)); if (cellAddress.size() == 1 && cellAddress.get(0).dimension().isEmpty()) { if (cellAddress.get(0).index().isPresent()) - b.append("[").append(cellAddress.get(0).index().get()).append("]"); + b.append("[").append(cellAddress.get(0).index().get().toString(context)).append("]"); else - b.append("{").append(cellAddress.get(0).label()).append("}"); + b.append("{").append(cellAddress.get(0).label().get()).append("}"); } else { - b.append("{").append(cellAddress.stream().map(i -> i.toString()).collect(Collectors.joining(", "))).append("}"); + b.append("{").append(cellAddress.stream().map(i -> i.toString(context)).collect(Collectors.joining(", "))).append("}"); } return b.toString(); } @@ -153,12 +153,16 @@ public class Value<NAMETYPE extends Name> extends PrimitiveTensorFunction<NAMETY @Override public String toString() { + return toString(null); + } + + public String toString(ToStringContext context) { StringBuilder b = new StringBuilder(); dimension.ifPresent(d -> b.append(d).append(":")); if (label != null) b.append(label); else - b.append(index); + b.append(index.toString(context)); return b.toString(); } |