diff options
author | Jon Bratseth <bratseth@gmail.com> | 2023-01-26 14:52:03 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-01-26 14:52:03 +0100 |
commit | 48b902d4852bc63db8ec835a9dfccc2b1f8f89ec (patch) | |
tree | 71dfe3245177c464dc18113b1866b5b1e9fde80b | |
parent | d25bb106ee6d4cf30efb1299297baa071503da54 (diff) | |
parent | 8509fd404dbfe90349072bfbe5e7cc7164023725 (diff) |
Merge pull request #25735 from vespa-engine/bratseth/validate-rank-profiles-early
Validate rank profiles early
9 files changed, 69 insertions, 35 deletions
diff --git a/container-search/abi-spec.json b/container-search/abi-spec.json index ae4d0e713f8..785475ae77e 100644 --- a/container-search/abi-spec.json +++ b/container-search/abi-spec.json @@ -8185,6 +8185,7 @@ ], "methods" : [ "public java.lang.String name()", + "public com.yahoo.search.schema.Schema schema()", "public boolean hasSummaryFeatures()", "public boolean hasRankFeatures()", "public java.util.Map inputs()", diff --git a/container-search/src/main/java/com/yahoo/search/schema/RankProfile.java b/container-search/src/main/java/com/yahoo/search/schema/RankProfile.java index 39d4a389e6f..85bb3915975 100644 --- a/container-search/src/main/java/com/yahoo/search/schema/RankProfile.java +++ b/container-search/src/main/java/com/yahoo/search/schema/RankProfile.java @@ -21,6 +21,9 @@ public class RankProfile { private final boolean hasRankFeatures; private final Map<String, TensorType> inputs; + // Assigned when this is added to a schema + private Schema schema = null; + private RankProfile(Builder builder) { this.name = builder.name; this.hasSummaryFeatures = builder.hasSummaryFeatures; @@ -30,6 +33,16 @@ public class RankProfile { public String name() { return name; } + /** Returns the schema owning this, or null if this is not added to a schema */ + public Schema schema() { return schema; } + + void setSchema(Schema schema) { + if ( this.schema != null) + throw new IllegalStateException("Cannot add rank profile '" + name + "' to schema '" + schema.name() + + "' as it is already added to schema '" + this.schema.name() + "'"); + this.schema = schema; + } + /** Returns true if this rank profile has summary features. */ public boolean hasSummaryFeatures() { return hasSummaryFeatures; } @@ -42,8 +55,7 @@ public class RankProfile { @Override public boolean equals(Object o) { if (o == this) return true; - if ( ! (o instanceof RankProfile)) return false; - RankProfile other = (RankProfile)o; + if ( ! (o instanceof RankProfile other)) return false; if ( ! other.name.equals(this.name)) return false; if ( other.hasSummaryFeatures != this.hasSummaryFeatures) return false; if ( other.hasRankFeatures != this.hasRankFeatures) return false; @@ -58,7 +70,7 @@ public class RankProfile { @Override public String toString() { - return "rank profile '" + name + "'"; + return "rank profile '" + name + "'" + (schema == null ? "" : " in " + schema); } public static class Builder { diff --git a/container-search/src/main/java/com/yahoo/search/schema/Schema.java b/container-search/src/main/java/com/yahoo/search/schema/Schema.java index 2ab5a30fbd7..c20aa1e81bd 100644 --- a/container-search/src/main/java/com/yahoo/search/schema/Schema.java +++ b/container-search/src/main/java/com/yahoo/search/schema/Schema.java @@ -27,6 +27,7 @@ public class Schema { this.name = builder.name; this.rankProfiles = Collections.unmodifiableMap(builder.rankProfiles); this.documentSummaries = Collections.unmodifiableMap(builder.documentSummaries); + rankProfiles.values().forEach(rankProfile -> rankProfile.setSchema(this)); } public String name() { return name; } @@ -36,8 +37,7 @@ public class Schema { @Override public boolean equals(Object o) { if (o == this) return true; - if ( ! (o instanceof Schema)) return false; - Schema other = (Schema)o; + if ( ! (o instanceof Schema other)) return false; if ( ! other.name.equals(this.name)) return false; if ( ! other.rankProfiles.equals(this.rankProfiles)) return false; if ( ! other.documentSummaries.equals(this.documentSummaries)) return false; diff --git a/container-search/src/main/java/com/yahoo/search/schema/SchemaInfo.java b/container-search/src/main/java/com/yahoo/search/schema/SchemaInfo.java index 6482070bc03..d29964ea9c5 100644 --- a/container-search/src/main/java/com/yahoo/search/schema/SchemaInfo.java +++ b/container-search/src/main/java/com/yahoo/search/schema/SchemaInfo.java @@ -9,6 +9,7 @@ import com.yahoo.search.config.IndexInfoConfig; import com.yahoo.search.config.SchemaInfoConfig; import com.yahoo.tensor.TensorType; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashSet; @@ -17,6 +18,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.stream.Collectors; /** * Information about all the schemas configured in the application this container is a part of. @@ -124,6 +126,13 @@ public class SchemaInfo { return schemas.stream().filter(schema -> names.contains(schema.name())).toList(); } + private List<RankProfile> profilesNamed(String name) { + return schemas.stream() + .filter(schema -> schema.rankProfiles().containsKey(name)) + .map(schema -> schema.rankProfiles().get(name)) + .toList(); + } + /** * Returns the type of the given rank feature name in the given profile, * if it can be uniquely determined. @@ -131,23 +140,27 @@ public class SchemaInfo { * @param rankFeature the rank feature name, a string on the form "query(name)" * @param rankProfile the name of the rank profile in which to locate the input declaration * @return the type of the declared input, or null if it is not declared or the rank profile is not found - * @throws IllegalArgumentException if the feature is declared in this rank profile in multiple schemas + * @throws IllegalArgumentException if the given rank profile does not exist in any schema, or the + * feature is declared in this rank profile in multiple schemas * of this session with conflicting types */ public TensorType rankProfileInput(String rankFeature, String rankProfile) { + if (schemas.isEmpty()) return null; // no matching schemas - validated elsewhere + List<RankProfile> profiles = profilesNamed(rankProfile); + if (profiles.isEmpty()) + throw new IllegalArgumentException("No profile named '" + rankProfile + "' exists in schemas [" + + schemas.stream().map(Schema::name).collect(Collectors.joining(", ")) + "]"); TensorType foundType = null; - Schema declaringSchema = null; - for (Schema schema : schemas) { - RankProfile profile = schema.rankProfiles().get(rankProfile); - if (profile == null) continue; + RankProfile declaringProfile = null; + for (RankProfile profile : profiles) { TensorType newlyFoundType = profile.inputs().get(rankFeature); if (newlyFoundType == null) continue; if (foundType != null && ! newlyFoundType.equals(foundType)) throw new IllegalArgumentException("Conflicting input type declarations for '" + rankFeature + "': " + - "Declared as " + foundType + " in " + profile + " in " + declaringSchema + - ", and as " + newlyFoundType + " in " + profile + " in " + schema); + "Declared as " + foundType + " in " + declaringProfile + + ", and as " + newlyFoundType + " in " + profile); foundType = newlyFoundType; - declaringSchema = schema; + declaringProfile = profile; } return foundType; } diff --git a/container-search/src/test/java/com/yahoo/search/query/RankProfileInputTest.java b/container-search/src/test/java/com/yahoo/search/query/RankProfileInputTest.java index da1d09fec1e..cbe4ddcbc63 100644 --- a/container-search/src/test/java/com/yahoo/search/query/RankProfileInputTest.java +++ b/container-search/src/test/java/com/yahoo/search/query/RankProfileInputTest.java @@ -50,10 +50,12 @@ public class RankProfileInputTest { assertEquals(Tensor.from(tensorString), query.getRanking().getFeatures().getTensor("query(myTensor1)").get()); } - { // Resolution is limited to the correct sources - Query query = createTensor1Query(tensorString, "bOnly", "sources=a"); - assertEquals(0, query.errors().size()); - assertEquals(tensorString, query.properties().get("ranking.features.query(myTensor1)"), "Not converted to tensor"); + try { + createTensor1Query(tensorString, "bOnly", "sources=a"); + fail("Expected exception"); + } + catch (IllegalArgumentException e) { + assertEquals("No profile named 'bOnly' exists in schemas [a]", Exceptions.toMessageString(e)); } } @@ -237,20 +239,19 @@ public class RankProfileInputTest { private SchemaInfo createSchemaInfo() { List<Schema> schemas = new ArrayList<>(); - RankProfile common = new RankProfile.Builder("commonProfile") + RankProfile.Builder common = new RankProfile.Builder("commonProfile") .addInput("query(myTensor1)", TensorType.fromSpec("tensor(a{},b{})")) .addInput("query(myTensor2)", TensorType.fromSpec("tensor(x[2],y[2])")) .addInput("query(myTensor3)", TensorType.fromSpec("tensor(x[2],y[2])")) - .addInput("query(myTensor4)", TensorType.fromSpec("tensor<float>(x[5])")) - .build(); + .addInput("query(myTensor4)", TensorType.fromSpec("tensor<float>(x[5])")); schemas.add(new Schema.Builder("a") - .add(common) + .add(common.build()) .add(new RankProfile.Builder("inconsistent") .addInput("query(myTensor1)", TensorType.fromSpec("tensor(a{},b{})")) .build()) .build()); schemas.add(new Schema.Builder("b") - .add(common) + .add(common.build()) .add(new RankProfile.Builder("inconsistent") .addInput("query(myTensor1)", TensorType.fromSpec("tensor(x[10])")) .build()) diff --git a/container-search/src/test/java/com/yahoo/search/query/profile/config/test/query-profile-variants2.cfg b/container-search/src/test/java/com/yahoo/search/query/profile/config/test/query-profile-variants2.cfg index ec091ecf2ea..cf17bc6ffcf 100644 --- a/container-search/src/test/java/com/yahoo/search/query/profile/config/test/query-profile-variants2.cfg +++ b/container-search/src/test/java/com/yahoo/search/query/profile/config/test/query-profile-variants2.cfg @@ -1,16 +1,14 @@ queryprofile[4] queryprofile[0].id "default" -queryprofile[0].property[5] +queryprofile[0].property[4] queryprofile[0].property[0].name "hits" queryprofile[0].property[0].value "5" queryprofile[0].property[1].name "model.defaultIndex" queryprofile[0].property[1].value "title" queryprofile[0].property[2].name "ranking.features.query(scorelimit)" queryprofile[0].property[2].value "-20" -queryprofile[0].property[3].name "ranking.profile" -queryprofile[0].property[3].value "production1" -queryprofile[0].property[4].name "ranking.properties.dotProduct.X" -queryprofile[0].property[4].value "(a:1,b:2)" +queryprofile[0].property[3].name "ranking.properties.dotProduct.X" +queryprofile[0].property[3].value "(a:1,b:2)" queryprofile[1].id "multi" queryprofile[1].inherit[1] queryprofile[1].inherit[0] "default" diff --git a/container-search/src/test/java/com/yahoo/search/schema/SchemaInfoTest.java b/container-search/src/test/java/com/yahoo/search/schema/SchemaInfoTest.java index 40d6f19c275..ba0c3f87900 100644 --- a/container-search/src/test/java/com/yahoo/search/schema/SchemaInfoTest.java +++ b/container-search/src/test/java/com/yahoo/search/schema/SchemaInfoTest.java @@ -2,9 +2,11 @@ package com.yahoo.search.schema; import com.yahoo.tensor.TensorType; +import com.yahoo.yolean.Exceptions; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.fail; /** * @author bratseth @@ -40,10 +42,17 @@ public class SchemaInfoTest { "a", "", "inconsistent", "query(myTensor1)"); tester.assertInput(TensorType.fromSpec("tensor(x[10])"), "b", "", "inconsistent", "query(myTensor1)"); - tester.assertInput(null, - "a", "", "bOnly", "query(myTensor1)"); tester.assertInput(TensorType.fromSpec("tensor(a{},b{})"), "ab", "", "bOnly", "query(myTensor1)"); + try { + tester.assertInput(null, + "a", "", "bOnly", "query(myTensor1)"); + fail("Expected exception since bOnly is not in a"); + } + catch (IllegalArgumentException e) { + assertEquals("No profile named 'bOnly' exists in schemas [a]", + Exceptions.toMessageString(e)); + } } } diff --git a/container-search/src/test/java/com/yahoo/search/schema/SchemaInfoTester.java b/container-search/src/test/java/com/yahoo/search/schema/SchemaInfoTester.java index 4e7dc27e73c..a46f3480d50 100644 --- a/container-search/src/test/java/com/yahoo/search/schema/SchemaInfoTester.java +++ b/container-search/src/test/java/com/yahoo/search/schema/SchemaInfoTester.java @@ -58,14 +58,13 @@ public class SchemaInfoTester { static SchemaInfo createSchemaInfo() { List<Schema> schemas = new ArrayList<>(); - RankProfile common = new RankProfile.Builder("commonProfile") + RankProfile.Builder common = new RankProfile.Builder("commonProfile") .addInput("query(myTensor1)", TensorType.fromSpec("tensor(a{},b{})")) .addInput("query(myTensor2)", TensorType.fromSpec("tensor(x[2],y[2])")) .addInput("query(myTensor3)", TensorType.fromSpec("tensor(x[2],y[2])")) - .addInput("query(myTensor4)", TensorType.fromSpec("tensor<float>(x[5])")) - .build(); + .addInput("query(myTensor4)", TensorType.fromSpec("tensor<float>(x[5])")); schemas.add(new Schema.Builder("a") - .add(common) + .add(common.build()) .add(new RankProfile.Builder("inconsistent") .addInput("query(myTensor1)", TensorType.fromSpec("tensor(a{},b{})")) .build()) @@ -76,7 +75,7 @@ public class SchemaInfoTester { .build()) .build()); schemas.add(new Schema.Builder("b") - .add(common) + .add(common.build()) .add(new RankProfile.Builder("inconsistent") .addInput("query(myTensor1)", TensorType.fromSpec("tensor(x[10])")) .build()) @@ -92,6 +91,7 @@ public class SchemaInfoTester { /** Creates the same schema info as createSchemaInfo from config objects. */ static SchemaInfo createSchemaInfoFromConfig() { + var indexInfoConfig = new IndexInfoConfig.Builder(); var rankProfileCommon = new SchemaInfoConfig.Schema.Rankprofile.Builder(); diff --git a/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/ScriptTestCase.java b/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/ScriptTestCase.java index 3a32b0049fe..e6744c010f4 100644 --- a/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/ScriptTestCase.java +++ b/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/ScriptTestCase.java @@ -68,7 +68,7 @@ public class ScriptTestCase { exp.verify(input); fail(); } catch (VerificationException e) { - assertTrue(e.getExpressionType().equals(ScriptExpression.class)); + assertEquals(e.getExpressionType(), ScriptExpression.class); assertEquals("Expected any input, got null.", e.getMessage()); } } |