Merge pull request #18964 from vespa-engine/balder/multiple-registration-of-rank-functions

- Add test for large expressions and inheritance.

3 files changed, 120 insertions, 4 deletions

diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/LargeRankExpressions.java b/config-model/src/main/java/com/yahoo/searchdefinition/LargeRankExpressions.java
index 9d8e8b90ceb..90dc40e7b93 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/LargeRankExpressions.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/LargeRankExpressions.java
@@ -21,11 +21,14 @@ public class LargeRankExpressions {
         expression.register(fileRegistry);
         String name = expression.getName();
         if (expressions.containsKey(name)) {
-            throw new IllegalArgumentException("Rank expression '" + name +
-                    "' defined twice. Previous blob with " + expressions.get(name).getBlob().remaining() +
-                    " bytes, while current has " + expression.getBlob().remaining() + " bytes");
+            if ( ! expressions.get(name).getBlob().equals(expression.getBlob())) {
+                throw new IllegalArgumentException("Rank expression '" + name +
+                        "' defined twice. Previous blob with " + expressions.get(name).getBlob().remaining() +
+                        " bytes, while current has " + expression.getBlob().remaining() + " bytes");
+            }
+        } else {
+            expressions.put(name, expression);
         }
-        expressions.put(name, expression);
     }
 
     /** Returns the ranking constant with the given name, or null if not present */
diff --git a/config-model/src/test/examples/largerankingexpressions/rankexpression.sd b/config-model/src/test/examples/largerankingexpressions/rankexpression.sd
new file mode 100644
index 00000000000..55466b7a5d3
--- /dev/null
+++ b/config-model/src/test/examples/largerankingexpressions/rankexpression.sd
@@ -0,0 +1,64 @@
+# Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+search rankexpression {
+
+  document rankexpression {
+
+    field artist type string {
+      indexing: summary | index
+    }
+
+    field title type string {
+      indexing: summary | index
+    }
+
+    field surl type string {
+      indexing: summary
+    }
+
+    field year type int {
+      indexing: summary | attribute
+    }
+
+  }
+
+  rank-profile base {
+
+    function large_f() {
+      expression: 78+closeness(distance)*(attribute(year)+attribute(year))/attribute(year)
+    }
+    macro large_m() {
+      expression: 78+closeness(distance)*(attribute(year)+attribute(year))/attribute(year)
+    }
+  }
+
+  rank-profile child_a inherits base {
+
+    function large_local_f() {
+      expression: large_f() + large_m()
+    }
+
+    function large_local_m() {
+      expression: large_f() + large_m()
+    }
+
+    first-phase {
+      expression: 0.8+0.2*large_local_f() + large_local_m()
+    }
+  }
+
+  rank-profile child_b inherits base {
+
+    function large_local_f() {
+      expression: large_f() + large_m()
+    }
+
+    function large_local_m() {
+      expression: large_f() + large_m()
+    }
+
+    first-phase {
+      expression: 0.8+0.2*large_local_f() + large_local_m()
+    }
+  }
+
+}
diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionsTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionsTestCase.java
index 9dd569a2f64..af21b6f5036 100644
--- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionsTestCase.java
+++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionsTestCase.java
@@ -22,10 +22,12 @@ import ai.vespa.rankingexpression.importer.configmodelview.ImportedMlModels;
 import org.junit.Test;
 
 import java.io.IOException;
+import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 public class RankingExpressionsTestCase extends SchemaTestCase {
 
@@ -78,4 +80,51 @@ public class RankingExpressionsTestCase extends SchemaTestCase {
         new DerivedConfiguration(search, new BaseDeployLogger(), new TestProperties(), registry, new QueryProfileRegistry(), new ImportedMlModels()); // rank profile parsing happens during deriving
     }
 
+    private void verifyProfile(RankProfile profile, List<String> expectedFunctions, List<Pair<String, String>> rankProperties,
+                               LargeRankExpressions largeExpressions, QueryProfileRegistry queryProfiles, ImportedMlModels models,
+                               AttributeFields attributes, ModelContext.Properties properties) {
+        var functions = profile.getFunctions();
+        assertEquals(expectedFunctions.size(), functions.size());
+        for (String func : expectedFunctions) {
+            assertTrue(functions.containsKey(func));
+        }
+
+        RawRankProfile raw = new RawRankProfile(profile, largeExpressions, queryProfiles, models, attributes, properties);
+        assertEquals(rankProperties.size(), raw.configProperties().size());
+        for (int i = 0; i < rankProperties.size(); i++) {
+            assertEquals(rankProperties.get(i).getFirst(), raw.configProperties().get(i).getFirst());
+            assertEquals(rankProperties.get(i).getSecond(), raw.configProperties().get(i).getSecond());
+        }
+    }
+
+    private void verifySearch(Search search, RankProfileRegistry rankProfileRegistry, LargeRankExpressions largeExpressions,
+                              QueryProfileRegistry queryProfiles, ImportedMlModels models, ModelContext.Properties properties)
+    {
+        ;
+        AttributeFields attributes = new AttributeFields(search);
+
+        verifyProfile(rankProfileRegistry.get(search, "base"), Arrays.asList("large_f", "large_m"),
+                Arrays.asList(new Pair<>("rankingExpression(large_f).expressionName", "base.large_f"), new Pair<>("rankingExpression(large_m).expressionName", "base.large_m")),
+                largeExpressions, queryProfiles, models, attributes, properties);
+        for (String child : Arrays.asList("child_a", "child_b")) {
+            verifyProfile(rankProfileRegistry.get(search, child), Arrays.asList("large_f", "large_m", "large_local_f", "large_local_m"),
+                    Arrays.asList(new Pair<>("rankingExpression(large_f).expressionName", child + ".large_f"), new Pair<>("rankingExpression(large_m).expressionName", child + ".large_m"),
+                            new Pair<>("rankingExpression(large_local_f).expressionName", child + ".large_local_f"), new Pair<>("rankingExpression(large_local_m).expressionName", child + ".large_local_m"),
+                            new Pair<>("vespa.rank.firstphase", "rankingExpression(firstphase)"), new Pair<>("rankingExpression(firstphase).expressionName", child + ".firstphase")),
+                    largeExpressions, queryProfiles, models, attributes, properties);
+        }
+    }
+
+    @Test
+    public void testLargeInheritedFunctions() throws IOException, ParseException {
+        ModelContext.Properties properties = new TestProperties().useExternalRankExpression(true).largeRankExpressionLimit(50);
+        RankProfileRegistry rankProfileRegistry = new RankProfileRegistry();
+        LargeRankExpressions largeExpressions = new LargeRankExpressions(new MockFileRegistry());
+        QueryProfileRegistry queryProfiles = new QueryProfileRegistry();
+        ImportedMlModels models = new ImportedMlModels();
+        Search search = createSearch("src/test/examples/largerankingexpressions", properties, rankProfileRegistry);
+        verifySearch(search, rankProfileRegistry, largeExpressions, queryProfiles, models, properties);
+        // Need to verify that second derivation works as that will happen if same sd is used in multiple content clusters
+        verifySearch(search, rankProfileRegistry, largeExpressions, queryProfiles, models, properties);
+    }
 }