Merge pull request #21571 from vespa-engine/arnej/handle-function-replacement

allow function replacement

7 files changed, 52 insertions, 24 deletions

diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java
index e419b8c93a7..e7b7c92ca2f 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java
@@ -745,8 +745,8 @@ public class RankProfile implements Cloneable {
     public RankingExpressionFunction addFunction(ExpressionFunction function, boolean inline) {
         RankingExpressionFunction rankingExpressionFunction = new RankingExpressionFunction(function, inline);
         if (functions.containsKey(function.getName())) {
-            deployLogger.log(Level.WARNING, "Function '" + function.getName() + "' replaces a previous function " +
-                    "with the same name in rank profile '" + this.name + "'");
+            deployLogger.log(Level.WARNING, "Function '" + function.getName() + "' is defined twice " +
+                    "in rank profile '" + this.name + "'");
         }
         functions.put(function.getName(), rankingExpressionFunction);
         allFunctionsCached = null;
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedField.java b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedField.java
index 5ee73abc28d..5bcfa841bae 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedField.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedField.java
@@ -121,10 +121,8 @@ class ParsedField extends ParsedBlock {
     void setStemming(Stemming stemming) { this.stemming = stemming; }
     void setWeight(int weight) { this.weight = weight; }
 
-    void addAttribute(ParsedAttribute attribute) {
-        String attrName = attribute.name();
-        verifyThat(! attributes.containsKey(attrName), "already has attribute", attrName);
-        attributes.put(attrName, attribute);
+    ParsedAttribute attributeFor(String attrName) {
+        return attributes.computeIfAbsent(attrName, n -> new ParsedAttribute(n));
     }
 
     void setIndexingOperation(ParsedIndexingOp idxOp) {
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedRankProfile.java b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedRankProfile.java
index 0801b613530..f028685b71a 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedRankProfile.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedRankProfile.java
@@ -113,9 +113,10 @@ class ParsedRankProfile extends ParsedBlock {
         fieldsRankWeight.put(field, weight);
     }
 
-    void addFunction(ParsedRankFunction func) {
-        verifyThat(! functions.containsKey(func.name()), "already has function", func.name());
-        functions.put(func.name(), func);
+    ParsedRankFunction addOrReplaceFunction(ParsedRankFunction func) {
+        // allowed with warning
+        // verifyThat(! functions.containsKey(func.name()), "already has function", func.name());
+        return functions.put(func.name(), func);
     }
 
     void addMutateOperation(MutateOperation.Phase phase, String attrName, String operation) {
diff --git a/config-model/src/main/javacc/IntermediateParser.jj b/config-model/src/main/javacc/IntermediateParser.jj
index 90260cb9895..b57719ec72d 100644
--- a/config-model/src/main/javacc/IntermediateParser.jj
+++ b/config-model/src/main/javacc/IntermediateParser.jj
@@ -1050,13 +1050,10 @@ void attribute(ParsedField field) :
 {
     <ATTRIBUTE> [name = identifier()]
     {
-        ParsedAttribute attr = new ParsedAttribute(name);
+        ParsedAttribute attr = field.attributeFor(name);
     }
          ( (<COLON> attributeSetting(attr))
            | (lbrace() (attributeSetting(attr) (<NL>)*)* <RBRACE>) )
-    {
-        field.addAttribute(attr);
-    }
 }
 
 /* pick up sorting in field block */
@@ -1910,7 +1907,13 @@ void function(ParsedRankProfile profile) :
     {
         func.setExpression(expression);
         func.setInline(inline);
-        profile.addFunction(func);
+        var old = profile.addOrReplaceFunction(func);
+        if (old != null) {
+            /* TODO disallow this for Vespa 8 */
+            deployLogger.logApplicationPackage(Level.WARNING, "Function '" + func.name()
+                                               + "' is defined twice in rank profile '"
+                                               + profile.name() + "'");
+        }
     }
 }
 
diff --git a/config-model/src/test/examples/attributesettings.sd b/config-model/src/test/examples/attributesettings.sd
index 2b83f137b96..e7bb2062227 100644
--- a/config-model/src/test/examples/attributesettings.sd
+++ b/config-model/src/test/examples/attributesettings.sd
@@ -33,12 +33,17 @@ search attributesettings {
       indexing: attribute
       weightedset: remove-if-zero
       weightedset: create-if-nonexistent
+      attribute: fast-search
+      attribute: fast-access
+      attribute: paged
     }
 
     field f6 type weightedset<string> {
       weightedset: remove-if-zero
       indexing: attribute
       weightedset: create-if-nonexistent
+      attribute: enable-bit-vectors
+      attribute: enable-only-bit-vector
     }
 
     field f7 type weightedset<string> {
diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/AttributeSettingsTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/AttributeSettingsTestCase.java
index 7b2ed2d7a7f..2a58618e11f 100644
--- a/config-model/src/test/java/com/yahoo/searchdefinition/AttributeSettingsTestCase.java
+++ b/config-model/src/test/java/com/yahoo/searchdefinition/AttributeSettingsTestCase.java
@@ -74,6 +74,14 @@ public class AttributeSettingsTestCase extends AbstractSchemaTestCase {
         assertWeightedSet(schema, "f8", true, false);
         assertWeightedSet(schema, "f9", false, true);
         assertWeightedSet(schema, "f10", false, true);
+
+        assertAttrSettings(schema, "f4", false, false, false);
+        assertAttrSettings(schema, "f5", true, true, true);
+        assertAttrSettings(schema, "f6", false, false, false);
+        assertAttrSettings(schema, "f7", false, false, false);
+        assertAttrSettings(schema, "f8", false, false, false);
+        assertAttrSettings(schema, "f9", false, false, false);
+        assertAttrSettings(schema, "f10", false, false, false);
     }
 
     private void assertWeightedSet(Schema schema, String name, boolean createIfNonExistent, boolean removeIfZero) {
@@ -82,13 +90,21 @@ public class AttributeSettingsTestCase extends AbstractSchemaTestCase {
         Attribute a4 = f4.getAttributes().get(f4.getName());
         assertEquals(Attribute.Type.STRING, a4.getType());
         assertEquals(Attribute.CollectionType.WEIGHTEDSET, a4.getCollectionType());
-        assertFalse(a4.isHuge());
-        assertFalse(a4.isFastSearch());
-        assertFalse(a4.isFastAccess());
         assertEquals(a4.isRemoveIfZero(), removeIfZero);
         assertEquals(a4.isCreateIfNonExistent(), createIfNonExistent);
     }
 
+    private void assertAttrSettings(Schema schema, String name, boolean fastAccess, boolean fastSearch, boolean paged) {
+        SDField f4 = (SDField) schema.getDocument().getField(name);
+        assertEquals(1, f4.getAttributes().size());
+        Attribute a4 = f4.getAttributes().get(f4.getName());
+        assertEquals(Attribute.Type.STRING, a4.getType());
+        assertEquals(Attribute.CollectionType.WEIGHTEDSET, a4.getCollectionType());
+        assertEquals(a4.isFastSearch(), fastSearch);
+        assertEquals(a4.isFastAccess(), fastAccess);
+        assertEquals(a4.isPaged(), paged);
+    }
+
     @Test
     public void requireThatFastAccessCanBeSet() throws IOException, ParseException {
         Schema schema = ApplicationBuilder.buildFromFile("src/test/examples/attributesettings.sd");
diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/RankingExpressionInliningTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/RankingExpressionInliningTestCase.java
index 94d25deb16a..e297beef026 100644
--- a/config-model/src/test/java/com/yahoo/searchdefinition/RankingExpressionInliningTestCase.java
+++ b/config-model/src/test/java/com/yahoo/searchdefinition/RankingExpressionInliningTestCase.java
@@ -192,14 +192,19 @@ public class RankingExpressionInliningTestCase extends AbstractSchemaTestCase {
 
     @Test
     public void testFunctionInliningWithReplacement() throws ParseException {
+        checkFunctionReplacement(false);
+        checkFunctionReplacement(true);
+    }
+
+    public void checkFunctionReplacement(boolean useXPP) throws ParseException {
         RankProfileRegistry rankProfileRegistry = new RankProfileRegistry();
         MockDeployLogger deployLogger = new MockDeployLogger();
         ApplicationBuilder builder = new ApplicationBuilder(MockApplicationPackage.createEmpty(),
-                                                              new MockFileRegistry(),
-                                                              deployLogger,
-                                                              new TestProperties(),
-                                                              rankProfileRegistry,
-                                                              new QueryProfileRegistry());
+                                                            new MockFileRegistry(),
+                                                            deployLogger,
+                                                            new TestProperties().setExperimentalSdParsing(useXPP),
+                                                            rankProfileRegistry,
+                                                            new QueryProfileRegistry());
         builder.addSchema(
                         "search test {\n" +
                         "    document test { }\n" +
@@ -219,8 +224,8 @@ public class RankingExpressionInliningTestCase extends AbstractSchemaTestCase {
         Schema s = builder.getSchema();
         RankProfile test = rankProfileRegistry.get(s, "test").compile(new QueryProfileRegistry(), new ImportedMlModels());
         assertEquals("foo(2)", test.getFirstPhaseRanking().getRoot().toString());
-        assertTrue("Does not contain expected warning", deployLogger.contains("Function 'foo' replaces " +
-                "a previous function with the same name in rank profile 'test'"));
+        assertTrue("Does not contain expected warning",
+                   deployLogger.contains("Function 'foo' is defined twice in rank profile 'test'"));
     }
 
     /**