From eabb1d3195a1bd6d34def333854ad713bd6ac886 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Mon, 6 Jun 2022 12:49:32 +0200 Subject: Validate rank profiles --- .../com/yahoo/prelude/cluster/ClusterSearcher.java | 68 ------------------- .../prelude/fastsearch/VespaBackEndSearcher.java | 8 ++- .../prelude/cluster/ClusterSearcherTestCase.java | 76 ---------------------- .../fastsearch/test/FastSearcherTestCase.java | 54 +++++++++++---- 4 files changed, 48 insertions(+), 158 deletions(-) (limited to 'container-search') diff --git a/container-search/src/main/java/com/yahoo/prelude/cluster/ClusterSearcher.java b/container-search/src/main/java/com/yahoo/prelude/cluster/ClusterSearcher.java index a1cad6de419..496e057a243 100644 --- a/container-search/src/main/java/com/yahoo/prelude/cluster/ClusterSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/cluster/ClusterSearcher.java @@ -38,7 +38,6 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.Executor; import java.util.concurrent.FutureTask; import java.util.concurrent.RejectedExecutionException; -import java.util.stream.Collectors; import static com.yahoo.container.QrSearchersConfig.Searchcluster.Indexingmode.STREAMING; @@ -166,63 +165,6 @@ public class ClusterSearcher extends Searcher { this(schemas, null, null); } - /** - * Returns an error if the document types do not have the requested rank - * profile. For the case of multiple document types, only returns an - * error if we have restricted the set of documents somehow. This is - * because when searching over all doc types, common ancestors might - * not have the requested rank profile and failing on that basis is - * probably not reasonable. - * - * @param query query - * @param schemas set of requested schemas for this query - * @return null if requested rank profile is ok for the requested - * schemas, a result with error message if not. - */ - // TODO: This should be in a separate searcher - // TODO Vespa 8: This should simply fail if the specified profile isn't present in all schemas - private Result checkValidRankProfiles(Query query, Set schemas, Execution.Context context) { - String rankProfile = query.getRanking().getProfile(); - Set invalidInSchemas = null; - Set schemasHavingProfile = schemasHavingProfile(rankProfile, context); - - if (schemasHavingProfile.isEmpty()) { - invalidInSchemas = schemas; - } - else if (schemas.size() == 1) { - if ( ! schemasHavingProfile.containsAll(schemas)) - invalidInSchemas = schemas; - } - else { - // multiple schemas, only fail when restricting doc types - Set restrict = query.getModel().getRestrict(); - Set sources = query.getModel().getSources(); - boolean validate = restrict != null && !restrict.isEmpty(); - validate = validate || sources != null && !sources.isEmpty(); - if (validate && !schemasHavingProfile.containsAll(schemas)) { - invalidInSchemas = new HashSet<>(schemas); - invalidInSchemas.removeAll(schemasHavingProfile); - } - } - - if (invalidInSchemas != null && !invalidInSchemas.isEmpty()) { - String plural = invalidInSchemas.size() > 1 ? "s" : ""; - return new Result(query, - ErrorMessage.createInvalidQueryParameter("Requested rank profile '" + rankProfile + - "' is undefined for document type" + plural + " '" + - String.join(", ", invalidInSchemas) + "'")); - } - - return null; - } - - private Set schemasHavingProfile(String profile, Execution.Context context) { - return context.schemaInfo().schemas().values().stream() - .filter(schema -> schema.rankProfiles().containsKey(profile)) - .map(schema -> schema.name()) - .collect(Collectors.toSet()); - } - @Override public void fill(com.yahoo.search.Result result, String summaryClass, Execution execution) { Query query = result.getQuery(); @@ -285,12 +227,6 @@ public class ClusterSearcher extends Searcher { return searchMultipleDocumentTypes(searcher, query, execution); } else { String docType = schemas.iterator().next(); - - Result invalidRankProfile = checkValidRankProfiles(query, schemas, execution.context()); - if (invalidRankProfile != null) { - return invalidRankProfile; - } - query.getModel().setRestrict(docType); return searcher.search(query, execution); } @@ -310,10 +246,6 @@ public class ClusterSearcher extends Searcher { private Result searchMultipleDocumentTypes(Searcher searcher, Query query, Execution execution) { Set schemas = resolveSchemas(query, execution.context().getIndexFacts()); - - Result invalidRankProfile = checkValidRankProfiles(query, schemas, execution.context()); - if (invalidRankProfile != null) return invalidRankProfile; - List queries = createQueries(query, schemas); if (queries.size() == 1) { return searcher.search(queries.get(0), execution); diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java index 21037be1a8b..584249fa1f3 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java @@ -14,7 +14,6 @@ import com.yahoo.protect.Validator; import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.cluster.PingableSearcher; -import com.yahoo.search.config.SchemaInfoConfig; import com.yahoo.search.schema.RankProfile; import com.yahoo.search.grouping.vespa.GroupingExecutor; import com.yahoo.search.result.ErrorMessage; @@ -168,6 +167,11 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { return new Result(query, ErrorMessage.createNullQuery(query.getHttpRequest().getUri().toString())); } + if ( ! getDocumentDatabase(query).schema().rankProfiles().containsKey(query.getRanking().getProfile())) + return new Result(query, ErrorMessage.createInvalidQueryParameter(getDocumentDatabase(query).schema() + + " does not contain requested rank profile '" + + query.getRanking().getProfile() + "'")); + QueryRewrite.optimizeByRestrict(query); QueryRewrite.optimizeAndNot(query); QueryRewrite.collapseSingleComposites(query); @@ -185,8 +189,6 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { return new Result(query); Result result = doSearch2(query, execution); - if (isLoggingFine()) - getLogger().fine("Result NOT retrieved from cache"); if (query.getTraceLevel() >= 1) query.trace(getName() + " dispatch response: " + result, false, 1); diff --git a/container-search/src/test/java/com/yahoo/prelude/cluster/ClusterSearcherTestCase.java b/container-search/src/test/java/com/yahoo/prelude/cluster/ClusterSearcherTestCase.java index 434f4bcc8e0..a09c2ff9b79 100644 --- a/container-search/src/test/java/com/yahoo/prelude/cluster/ClusterSearcherTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/cluster/ClusterSearcherTestCase.java @@ -8,7 +8,6 @@ import com.yahoo.concurrent.InThreadExecutorService; import com.yahoo.container.QrSearchersConfig; import com.yahoo.container.handler.ClustersStatus; import com.yahoo.container.handler.VipStatus; -import com.yahoo.container.protect.Error; import com.yahoo.prelude.IndexFacts; import com.yahoo.prelude.IndexModel; import com.yahoo.prelude.SearchDefinition; @@ -428,81 +427,6 @@ public class ClusterSearcherTestCase { assertResult(6, new ArrayList<>(), getResult(6, 2, extra, ex)); } - @Test - public void testRequireThatSearchFailsForUndefinedRankProfileWithOneDocType() { - Execution execution = createExecution(Arrays.asList("type1"), false); - - // "default" rank profile - Query query = new Query("?query=hello"); - com.yahoo.search.Result result = execution.search(query); - assertEquals(3, result.getTotalHitCount()); - - // specified "default" rank profile - query = new Query("?query=hello&ranking.profile=default"); - result = execution.search(query); - assertEquals(3, result.getTotalHitCount()); - - // empty rank profile, should fail - query = new Query("?query=hello&ranking.profile="); - result = execution.search(query); - assertEquals(0, result.getTotalHitCount()); - assertEquals(result.hits().getError().getCode(), Error.INVALID_QUERY_PARAMETER.code); - - // invalid rank profile - query = new Query("?query=hello&ranking.profile=undefined"); - result = execution.search(query); - assertEquals(0, result.getTotalHitCount()); - assertEquals(result.hits().getError().getCode(), Error.INVALID_QUERY_PARAMETER.code); - - // valid rank profile for type1 - query = new Query("?query=hello&ranking.profile=testprofile"); - result = execution.search(query); - assertEquals(3, result.getTotalHitCount()); - } - - @Test - public void testRequireThatSearchFailsForUndefinedRankProfileWithMultipleDocTypes() { - Execution execution = createExecution(Arrays.asList("type1", "type2", "type3"), false); - - // "default" rank profile - Query query = new Query("?query=hello"); - com.yahoo.search.Result result = execution.search(query); - assertEquals(9, result.getTotalHitCount()); - - // specified "default" rank profile - query = new Query("?query=hello&ranking.profile=default"); - result = execution.search(query); - assertEquals(9, result.getTotalHitCount()); - - // empty rank profile, should fail - query = new Query("?query=hello&ranking.profile="); - result = execution.search(query); - assertEquals(0, result.getTotalHitCount()); - assertEquals(result.hits().getError().getCode(), Error.INVALID_QUERY_PARAMETER.code); - - // invalid rank profile - query = new Query("?query=hello&ranking.profile=undefined"); - result = execution.search(query); - assertEquals(0, result.getTotalHitCount()); - assertEquals(result.hits().getError().getCode(), Error.INVALID_QUERY_PARAMETER.code); - - // testprofile is only defined for type1, but should pass as it exists in at least one document type - query = new Query("?query=hello&ranking.profile=testprofile"); - result = execution.search(query); - assertEquals(9, result.getTotalHitCount()); - - // testprofile is only defined for type1, but should fail when restricting doc types - query = new Query("?query=hello&ranking.profile=testprofile&restrict=type1,type3"); - result = execution.search(query); - assertEquals(0, result.getTotalHitCount()); - assertEquals(result.hits().getError().getCode(), Error.INVALID_QUERY_PARAMETER.code); - - // testprofile is only defined for type1, ok if restricted to type1 - query = new Query("?query=hello&ranking.profile=testprofile&restrict=type1"); - result = execution.search(query); - assertEquals(3, result.getTotalHitCount()); - } - private static ClusterSearcher createSearcher(String clusterName, Double maxQueryTimeout, Double maxQueryCacheTimeout, boolean streamingMode, VipStatus vipStatus) { QrSearchersConfig.Builder qrSearchersConfig = new QrSearchersConfig.Builder(); diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java index c9193d08381..271b932dff5 100644 --- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java @@ -16,6 +16,7 @@ import com.yahoo.prelude.fastsearch.VespaBackEndSearcher; import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.Searcher; +import com.yahoo.search.config.SchemaInfoConfig; import com.yahoo.search.dispatch.rpc.RpcResourcePool; import com.yahoo.search.dispatch.searchcluster.Node; import com.yahoo.search.grouping.GroupingRequest; @@ -41,6 +42,7 @@ import java.util.logging.Logger; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -51,9 +53,6 @@ import static org.junit.Assert.assertTrue; */ public class FastSearcherTestCase { - private final static DocumentdbInfoConfig documentdbInfoConfig = new DocumentdbInfoConfig(new DocumentdbInfoConfig.Builder()); - - @Test public void testNullQuery() { Logger.getLogger(FastSearcher.class.getName()).setLevel(Level.ALL); @@ -61,11 +60,11 @@ public class FastSearcherTestCase { MockDispatcher.create(Collections.emptyList()), new SummaryParameters(null), new ClusterParams("testhittype"), - documentdbInfoConfig, - SchemaInfo.empty()); + documentdbInfoConfig("test"), + schemaInfo("test")); String query = "?junkparam=ignored"; - Result result = doSearch(fastSearcher,new Query(query), 0, 10); + Result result = doSearch(fastSearcher, new Query(query), 0, 10); ErrorMessage message = result.hits().getError(); assertNotNull("Got error", message); @@ -93,11 +92,11 @@ public class FastSearcherTestCase { @Test public void testSinglePassGroupingIsForcedWithSingleNodeGroups() { FastSearcher fastSearcher = new FastSearcher("container.0", - MockDispatcher.create(Collections.singletonList(new Node(0, "host0", 0))), + MockDispatcher.create(List.of(new Node(0, "host0", 0))), new SummaryParameters(null), new ClusterParams("testhittype"), - documentdbInfoConfig, - SchemaInfo.empty()); + documentdbInfoConfig("test"), + schemaInfo("test")); Query q = new Query("?query=foo"); GroupingRequest request1 = GroupingRequest.newInstance(q); request1.setRootOperation(new AllOperation()); @@ -113,6 +112,19 @@ public class FastSearcherTestCase { assertForceSinglePassIs(true, q); } + @Test + public void testRankProfileValidation() { + FastSearcher fastSearcher = new FastSearcher("container.0", + MockDispatcher.create(List.of(new Node(0, "host0", 0))), + new SummaryParameters(null), + new ClusterParams("testhittype"), + documentdbInfoConfig("test"), + schemaInfo("test")); + assertFalse(searchError("?query=q", fastSearcher).contains("does not contain requested rank profile")); + assertFalse(searchError("?query=q&ranking.profile=default", fastSearcher).contains("does not contain requested rank profile")); + assertTrue(searchError("?query=q&ranking.profile=nosuch", fastSearcher).contains("does not contain requested rank profile")); + } + @Test public void testSummaryNeedsQuery() { var documentDb = new DocumentdbInfoConfig(new DocumentdbInfoConfig.Builder().documentdb(new DocumentdbInfoConfig.Documentdb.Builder().name("test"))); @@ -145,8 +157,8 @@ public class FastSearcherTestCase { dispatcher, new SummaryParameters(null), new ClusterParams("testhittype"), - documentdbInfoConfig, - SchemaInfo.empty()); + documentdbInfoConfig("test"), + schemaInfo("test")); Query q = new Query("?query=foo"); GroupingRequest request1 = GroupingRequest.newInstance(q); request1.setRootOperation(new AllOperation()); @@ -192,4 +204,24 @@ public class FastSearcherTestCase { assertTrue(vipStatus.isInRotation()); //Verify that deconstruct does not touch vipstatus } + private String searchError(String query, Searcher searcher) { + return search(query, searcher).hits().getError().getDetailedMessage(); + } + + private Result search(String query, Searcher searcher) { + return searcher.search(new Query(query), new Execution(Execution.Context.createContextStub())); + } + + private DocumentdbInfoConfig documentdbInfoConfig(String schemaName) { + var db = new DocumentdbInfoConfig.Documentdb.Builder().name(schemaName); + db.rankprofile(new DocumentdbInfoConfig.Documentdb.Rankprofile.Builder().name("default")); + return new DocumentdbInfoConfig.Builder().documentdb(db).build(); + } + + private SchemaInfo schemaInfo(String schemaName) { + var schema = new Schema.Builder(schemaName); + schema.add(new RankProfile.Builder("default").build()); + return new SchemaInfo(List.of(schema.build()), Map.of()); + } + } -- cgit v1.2.3