Merge pull request #26270 from vespa-engine/arnej/ranking-expression-wrapping

Arnej/ranking expression wrapping

6 files changed, 201 insertions, 7 deletions

diff --git a/config-model/src/main/java/com/yahoo/schema/OnnxModel.java b/config-model/src/main/java/com/yahoo/schema/OnnxModel.java
index 6baaea6ea05..272b668b5fb 100644
--- a/config-model/src/main/java/com/yahoo/schema/OnnxModel.java
+++ b/config-model/src/main/java/com/yahoo/schema/OnnxModel.java
@@ -55,7 +55,7 @@ public class OnnxModel extends DistributableResource {
                 return ref.toString();
             }
             // or a function (evaluated by backend)
-            if (ref.isSimple() && "rankingExpression".equals(ref.name())) {
+            if (ref.isSimpleRankingExpressionWrapper()) {
                 var arg = ref.simpleArgument();
                 if (arg.isPresent()) {
                     return ref.toString();
diff --git a/config-model/src/main/java/com/yahoo/schema/RankProfile.java b/config-model/src/main/java/com/yahoo/schema/RankProfile.java
index 7cb0a088f5f..a00bbb682a8 100644
--- a/config-model/src/main/java/com/yahoo/schema/RankProfile.java
+++ b/config-model/src/main/java/com/yahoo/schema/RankProfile.java
@@ -1169,7 +1169,7 @@ public class RankProfile implements Cloneable {
             // Source is either a simple reference (query/attribute/constant/rankingExpression)...
             Optional<Reference> reference = Reference.simple(source);
             if (reference.isPresent()) {
-                if (reference.get().name().equals("rankingExpression") && reference.get().simpleArgument().isPresent()) {
+                if (reference.get().isSimpleRankingExpressionWrapper()) {
                     source = reference.get().simpleArgument().get();  // look up function below
                 } else {
                     return Optional.of(context.getType(reference.get()));
diff --git a/config-model/src/main/java/com/yahoo/schema/derived/RawRankProfile.java b/config-model/src/main/java/com/yahoo/schema/derived/RawRankProfile.java
index 31a38752bec..acb125197d2 100644
--- a/config-model/src/main/java/com/yahoo/schema/derived/RawRankProfile.java
+++ b/config-model/src/main/java/com/yahoo/schema/derived/RawRankProfile.java
@@ -20,6 +20,7 @@ import com.yahoo.searchlib.rankingexpression.parser.ParseException;
 import com.yahoo.searchlib.rankingexpression.rule.ReferenceNode;
 import com.yahoo.searchlib.rankingexpression.rule.SerializationContext;
 import com.yahoo.vespa.config.search.RankProfilesConfig;
+import static com.yahoo.searchlib.rankingexpression.Reference.wrapInRankingExpression;
 
 import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
@@ -273,9 +274,9 @@ public class RawRankProfile implements RankProfilesConfig.Producer {
                     String propertyName = RankingExpression.propertyName(referenceNode.getName());
                     String expressionString = function.getBody().getRoot().toString(context).toString();
                     context.addFunctionSerialization(propertyName, expressionString);
-                    ReferenceNode backendReferenceNode = new ReferenceNode("rankingExpression(" + referenceNode.getName() + ")",
-                                                                           referenceNode.getArguments().expressions(),
-                                                                           referenceNode.getOutput());
+                    var backendReferenceNode = new ReferenceNode(wrapInRankingExpression(referenceNode.getName()),
+                                                                 referenceNode.getArguments().expressions(),
+                                                                 referenceNode.getOutput());
                     // tell backend to map back to the name the user expects:
                     featureRenames.put(backendReferenceNode.toString(), referenceNode.toString());
                     functionFeatures.put(referenceNode.getName(), backendReferenceNode);
@@ -499,7 +500,7 @@ public class RawRankProfile implements RankProfilesConfig.Producer {
             if (expression.getRoot() instanceof ReferenceNode) {
                 properties.add(new Pair<>("vespa.rank." + phase, expression.getRoot().toString()));
             } else {
-                properties.add(new Pair<>("vespa.rank." + phase, "rankingExpression(" + name + ")"));
+                properties.add(new Pair<>("vespa.rank." + phase, wrapInRankingExpression(name)));
                 properties.add(new Pair<>(RankingExpression.propertyName(name), expression.getRoot().toString()));
             }
             return properties;
@@ -520,7 +521,7 @@ public class RawRankProfile implements RankProfilesConfig.Producer {
                 for (Map.Entry<String, String> mapping : onnxModel.getInputMap().entrySet()) {
                     String source = mapping.getValue();
                     if (functionNames.contains(source)) {
-                        onnxModel.addInputNameMapping(mapping.getKey(), "rankingExpression(" + source + ")");
+                        onnxModel.addInputNameMapping(mapping.getKey(), wrapInRankingExpression(source));
                     }
                 }
             }
diff --git a/config-model/src/test/derived/rankingmacros/rank-profiles.cfg b/config-model/src/test/derived/rankingmacros/rank-profiles.cfg
new file mode 100644
index 00000000000..71b7bc7166c
--- /dev/null
+++ b/config-model/src/test/derived/rankingmacros/rank-profiles.cfg
@@ -0,0 +1,83 @@
+rankprofile[].name "default"
+rankprofile[].name "unranked"
+rankprofile[].fef.property[].name "vespa.rank.firstphase"
+rankprofile[].fef.property[].value "value(0)"
+rankprofile[].fef.property[].name "vespa.hitcollector.heapsize"
+rankprofile[].fef.property[].value "0"
+rankprofile[].fef.property[].name "vespa.hitcollector.arraysize"
+rankprofile[].fef.property[].value "0"
+rankprofile[].fef.property[].name "vespa.dump.ignoredefaultfeatures"
+rankprofile[].fef.property[].value "true"
+rankprofile[].name "standalone"
+rankprofile[].fef.property[].name "rankingExpression(myfeature).rankingScript"
+rankprofile[].fef.property[].value "7 * attribute(num)"
+rankprofile[].fef.property[].name "rankingExpression(fourtimessum@2b1138e8965e7ff5.67f1e87166cfef86).rankingScript"
+rankprofile[].fef.property[].value "4 * (match + match)"
+rankprofile[].fef.property[].name "rankingExpression(macro_with_dollar$).rankingScript"
+rankprofile[].fef.property[].value "69"
+rankprofile[].fef.property[].name "rankingExpression(anotherfeature).rankingScript"
+rankprofile[].fef.property[].value "10 * rankingExpression(myfeature)"
+rankprofile[].fef.property[].name "rankingExpression(yetanotherfeature).rankingScript"
+rankprofile[].fef.property[].value "100 * rankingExpression(myfeature)"
+rankprofile[].fef.property[].name "rankingExpression(fourtimessum).rankingScript"
+rankprofile[].fef.property[].value "4 * (var1 + var2)"
+rankprofile[].fef.property[].name "vespa.rank.firstphase"
+rankprofile[].fef.property[].value "rankingExpression(firstphase)"
+rankprofile[].fef.property[].name "rankingExpression(firstphase).rankingScript"
+rankprofile[].fef.property[].value "match + fieldMatch(title) + rankingExpression(myfeature)"
+rankprofile[].fef.property[].name "vespa.rank.secondphase"
+rankprofile[].fef.property[].value "rankingExpression(secondphase)"
+rankprofile[].fef.property[].name "rankingExpression(secondphase).rankingScript"
+rankprofile[].fef.property[].value "rankingExpression(fourtimessum@2b1138e8965e7ff5.67f1e87166cfef86) + 0 * rankingExpression(macro_with_dollar$)"
+rankprofile[].fef.property[].name "vespa.summary.feature"
+rankprofile[].fef.property[].value "firstPhase"
+rankprofile[].fef.property[].name "vespa.summary.feature"
+rankprofile[].fef.property[].value "rankingExpression(myfeature)"
+rankprofile[].fef.property[].name "vespa.summary.feature"
+rankprofile[].fef.property[].value "rankingExpression(anotherfeature)"
+rankprofile[].fef.property[].name "vespa.summary.feature"
+rankprofile[].fef.property[].value "rankingExpression(yetanotherfeature)"
+rankprofile[].fef.property[].name "vespa.summary.feature"
+rankprofile[].fef.property[].value "rankingExpression(macro_with_dollar$)"
+rankprofile[].fef.property[].name "vespa.feature.rename"
+rankprofile[].fef.property[].value "rankingExpression(anotherfeature)"
+rankprofile[].fef.property[].name "vespa.feature.rename"
+rankprofile[].fef.property[].value "anotherfeature"
+rankprofile[].fef.property[].name "vespa.feature.rename"
+rankprofile[].fef.property[].value "rankingExpression(yetanotherfeature)"
+rankprofile[].fef.property[].name "vespa.feature.rename"
+rankprofile[].fef.property[].value "yetanotherfeature"
+rankprofile[].fef.property[].name "vespa.feature.rename"
+rankprofile[].fef.property[].value "rankingExpression(macro_with_dollar$)"
+rankprofile[].fef.property[].name "vespa.feature.rename"
+rankprofile[].fef.property[].value "macro_with_dollar$"
+rankprofile[].name "constantsAndMacro"
+rankprofile[].fef.property[].name "rankingExpression(c).rankingScript"
+rankprofile[].fef.property[].value "attribute(num)"
+rankprofile[].fef.property[].name "vespa.rank.firstphase"
+rankprofile[].fef.property[].value "rankingExpression(firstphase)"
+rankprofile[].fef.property[].name "rankingExpression(firstphase).rankingScript"
+rankprofile[].fef.property[].value "attribute(num) * 2.0 + 3.0"
+rankprofile[].fef.property[].name "vespa.summary.feature"
+rankprofile[].fef.property[].value "firstPhase"
+rankprofile[].name "doc"
+rankprofile[].fef.property[].name "rankingExpression(myfeature).rankingScript"
+rankprofile[].fef.property[].value "fieldMatch(title) + freshness(timestamp)"
+rankprofile[].fef.property[].name "rankingExpression(otherfeature@6b0a229a66fcaa04).rankingScript"
+rankprofile[].fef.property[].value "nativeRank(title,body)"
+rankprofile[].fef.property[].name "rankingExpression(otherfeature).rankingScript"
+rankprofile[].fef.property[].value "nativeRank(foo,body)"
+rankprofile[].fef.property[].name "vespa.rank.firstphase"
+rankprofile[].fef.property[].value "rankingExpression(firstphase)"
+rankprofile[].fef.property[].name "rankingExpression(firstphase).rankingScript"
+rankprofile[].fef.property[].value "rankingExpression(myfeature) * 10"
+rankprofile[].fef.property[].name "vespa.rank.secondphase"
+rankprofile[].fef.property[].value "rankingExpression(secondphase)"
+rankprofile[].fef.property[].name "rankingExpression(secondphase).rankingScript"
+rankprofile[].fef.property[].value "rankingExpression(otherfeature@6b0a229a66fcaa04) * rankingExpression(myfeature)"
+rankprofile[].fef.property[].name "vespa.summary.feature"
+rankprofile[].fef.property[].value "rankingExpression(myfeature)"
+rankprofile[].fef.property[].name "vespa.feature.rename"
+rankprofile[].fef.property[].value "rankingExpression(myfeature)"
+rankprofile[].fef.property[].name "vespa.feature.rename"
+rankprofile[].fef.property[].value "myfeature"
diff --git a/config-model/src/test/derived/rankingmacros/rankingmacros.sd b/config-model/src/test/derived/rankingmacros/rankingmacros.sd
new file mode 100644
index 00000000000..84598cb483a
--- /dev/null
+++ b/config-model/src/test/derived/rankingmacros/rankingmacros.sd
@@ -0,0 +1,105 @@
+# Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+schema rankingmacros {
+
+  document rankingmacros {
+    field title type string {
+      indexing: index
+    }
+    field timestamp type long {
+      indexing: attribute
+    }
+    field description type string {
+      indexing: index
+    }
+    field num type int {
+      indexing: attribute
+    }
+    field abstract type string {
+      indexing: index
+    }
+    field body type string {
+      indexing: index
+    }
+    field usstaticrank type string {
+      indexing: attribute
+    }
+    field boostmax type string {
+      indexing: index
+    }
+    field entitytitle type string {
+      indexing: index
+    }
+  }
+
+  rank-profile standalone {
+    macro fourtimessum(var1, var2) {
+      expression: 4*(var1+var2)
+    }
+    macro myfeature() {
+       expression {
+         7 * attribute(num)
+       }
+    }
+    macro anotherfeature() {
+      expression: 10*myfeature
+    }
+    macro yetanotherfeature() {
+      expression: 100*rankingExpression(myfeature) # legacy form
+    }
+    macro macro_with_dollar$() { # Not allowed
+      expression: 69
+    }
+    first-phase {
+      expression: match + fieldMatch(title) + myfeature
+    }
+    second-phase {
+      expression: fourtimessum(match,match) + 0 * macro_with_dollar$
+    }
+    summary-features {
+      firstPhase
+      rankingExpression(myfeature)
+      anotherfeature
+      yetanotherfeature
+      macro_with_dollar$
+    }
+  }
+
+  # Profile with macro and constants
+  rank-profile constantsAndMacro {
+    macro c() {
+      expression: attribute(num)
+    }
+
+    constants {
+      a: 2
+      b: 3
+    }
+
+    first-phase {
+      expression: attribute(num) * a + b
+    }
+
+    summary-features {
+      firstPhase
+    }
+  }
+
+  # The example in the docs
+  rank-profile doc inherits default {
+    macro myfeature() {
+      expression: fieldMatch(title) + freshness(timestamp)
+    }
+    macro otherfeature(foo) {
+      expression{ nativeRank(foo, body) }
+    }
+
+    first-phase {
+      expression: myfeature * 10
+    }
+    second-phase {
+      expression: otherfeature(title) * myfeature
+    }
+    summary-features: myfeature
+  }
+
+}
diff --git a/config-model/src/test/java/com/yahoo/schema/derived/RankProfilesTestCase.java b/config-model/src/test/java/com/yahoo/schema/derived/RankProfilesTestCase.java
index 5c24b32e275..a4c03291ce2 100644
--- a/config-model/src/test/java/com/yahoo/schema/derived/RankProfilesTestCase.java
+++ b/config-model/src/test/java/com/yahoo/schema/derived/RankProfilesTestCase.java
@@ -17,4 +17,9 @@ public class RankProfilesTestCase extends AbstractExportingTestCase {
     void testRankProfiles() throws IOException, ParseException {
         assertCorrectDeriving("rankprofiles", null, new TestProperties(), new TestableDeployLogger());
     }
+
+    @Test
+    void testMacrosInRankProfiles() throws IOException, ParseException {
+        assertCorrectDeriving("rankingmacros", null, new TestProperties(), new TestableDeployLogger());
+    }
 }