diff options
author | Arne H Juul <arnej27959@users.noreply.github.com> | 2022-03-07 11:35:01 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-03-07 11:35:01 +0100 |
commit | b4cecb502139513c39b6903e7d2844e511eb27b1 (patch) | |
tree | b995d222eb24d6dc5afbdc9b6958bbc21a9fb55e | |
parent | 0b87a80fef3135e19522e92fa886a48148f650c0 (diff) | |
parent | 597689db11bce4732a0055c3f05d9347fcb7b38b (diff) |
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'")); } /** |