diff options
33 files changed, 534 insertions, 556 deletions
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 a8605ab1597..855a524473d 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 @@ -12,7 +12,6 @@ import com.yahoo.container.handler.VipStatus; import com.yahoo.prelude.fastsearch.ClusterParams; import com.yahoo.prelude.fastsearch.DocumentdbInfoConfig; import com.yahoo.prelude.fastsearch.IndexedBackend; -import com.yahoo.prelude.fastsearch.SummaryParameters; import com.yahoo.prelude.fastsearch.VespaBackend; import com.yahoo.search.Query; import com.yahoo.search.Result; @@ -92,23 +91,19 @@ public class ClusterSearcher extends Searcher { maxQueryTimeout = ParameterParser.asMilliSeconds(clusterConfig.maxQueryTimeout(), DEFAULT_MAX_QUERY_TIMEOUT); maxQueryCacheTimeout = ParameterParser.asMilliSeconds(clusterConfig.maxQueryCacheTimeout(), DEFAULT_MAX_QUERY_CACHE_TIMEOUT); - SummaryParameters docSumParams = new SummaryParameters(qrsConfig - .com().yahoo().prelude().fastsearch().IndexedBackend().docsum() - .defaultclass()); - - String uniqueServerId = UUID.randomUUID().toString(); VespaBackend streaming = null, indexed = null; + ClusterParams clusterParams = makeClusterParams(searchClusterIndex, qrsConfig + .com().yahoo().prelude().fastsearch().IndexedBackend().docsum() + .defaultclass(), documentDbConfig, schemaInfo); for (DocumentdbInfoConfig.Documentdb docDb : documentDbConfig.documentdb()) { if (docDb.mode() == DocumentdbInfoConfig.Documentdb.Mode.Enum.INDEX) { if (indexed == null) { - indexed = searchDispatch(searchClusterIndex, searchClusterName, uniqueServerId, - docSumParams, documentDbConfig, schemaInfo, dispatchers); + indexed = searchDispatch(clusterParams, searchClusterName, dispatchers); } schema2Searcher.put(docDb.name(), indexed); } else if (docDb.mode() == DocumentdbInfoConfig.Documentdb.Mode.Enum.STREAMING) { if (streaming == null) { - streaming = streamingCluster(uniqueServerId, searchClusterIndex, - searchClusterConfig, docSumParams, documentDbConfig, schemaInfo, access); + streaming = streamingCluster(clusterParams, searchClusterConfig, access); vipStatus.addToRotation(streaming.getName()); } schema2Searcher.put(docDb.name(), streaming); @@ -126,42 +121,30 @@ public class ClusterSearcher extends Searcher { config.searchcluster().stream().map(QrSearchersConfig.Searchcluster::name).toList()); } - private static ClusterParams makeClusterParams(int searchclusterIndex) { - return new ClusterParams("sc" + searchclusterIndex + ".num" + 0); + private static ClusterParams makeClusterParams(int searchclusterIndex, String defaultSummary, + DocumentdbInfoConfig documentDbConfig, SchemaInfo schemaInfo) + { + return new ClusterParams("sc" + searchclusterIndex + ".num" + 0, UUID.randomUUID().toString(), + defaultSummary, documentDbConfig, schemaInfo); } - private static IndexedBackend searchDispatch(int searchclusterIndex, + private static IndexedBackend searchDispatch(ClusterParams clusterParams, String searchClusterName, - String serverId, - SummaryParameters docSumParams, - DocumentdbInfoConfig documentdbInfoConfig, - SchemaInfo schemaInfo, - ComponentRegistry<Dispatcher> dispatchers) { - ClusterParams clusterParams = makeClusterParams(searchclusterIndex); + ComponentRegistry<Dispatcher> dispatchers) + { ComponentId dispatcherComponentId = new ComponentId("dispatcher." + searchClusterName); Dispatcher dispatcher = dispatchers.getComponent(dispatcherComponentId); if (dispatcher == null) - throw new IllegalArgumentException("Configuration error: No dispatcher " + dispatcherComponentId + - " is configured"); - return new IndexedBackend(serverId, dispatcher, docSumParams, clusterParams, documentdbInfoConfig, schemaInfo); + throw new IllegalArgumentException("Configuration error: No dispatcher " + dispatcherComponentId + " is configured"); + return new IndexedBackend(clusterParams, dispatcher); } - private static StreamingBackend streamingCluster(String serverId, - int searchclusterIndex, + private static StreamingBackend streamingCluster(ClusterParams clusterParams, QrSearchersConfig.Searchcluster searchClusterConfig, - SummaryParameters docSumParams, - DocumentdbInfoConfig documentdbInfoConfig, - SchemaInfo schemaInfo, - VespaDocumentAccess access) { - if (searchClusterConfig.searchdef().size() != 1) - throw new IllegalArgumentException("Streaming search clusters can only contain a single schema but got " + - searchClusterConfig.searchdef()); - ClusterParams clusterParams = makeClusterParams(searchclusterIndex); - StreamingBackend searcher = new StreamingBackend(access); - searcher.setSearchClusterName(searchClusterConfig.rankprofiles_configid()); - searcher.setStorageClusterRouteSpec(searchClusterConfig.storagecluster().routespec()); - searcher.init(serverId, docSumParams, clusterParams, documentdbInfoConfig, schemaInfo); - return searcher; + VespaDocumentAccess access) + { + return new StreamingBackend(clusterParams, searchClusterConfig.rankprofiles_configid(), + access, searchClusterConfig.storagecluster().routespec()); } /** Do not use, for internal testing purposes only. **/ diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/ClusterParams.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/ClusterParams.java index c34187e576b..2987500fdb3 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/ClusterParams.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/ClusterParams.java @@ -1,21 +1,40 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.prelude.fastsearch; +import com.yahoo.search.schema.SchemaInfo; + /** * Helper class for carrying around cluster-related - * config parameters to the FastSearcher class. + * config parameters to the VespaBackend class. * * @author arnej27959 */ public class ClusterParams { - public final String searcherName; + private final String searcherName; + private final String serverId; + private final String defaultSummary; + private final DocumentdbInfoConfig documentdbInfoConfig; + private final SchemaInfo schemaInfo; - /** - * Make up full ClusterParams - */ public ClusterParams(String name) { + this(name, "server.0", null, null, null); + } + public ClusterParams(String name, String serverId, String defaultSummary, + DocumentdbInfoConfig documentdbInfoConfig, SchemaInfo schemaInfo) { this.searcherName = name; + this.serverId = serverId; + if (defaultSummary != null && defaultSummary.isEmpty()) + this.defaultSummary = null; + else + this.defaultSummary = defaultSummary; + this.documentdbInfoConfig = documentdbInfoConfig; + this.schemaInfo = schemaInfo; } + public String getServerId() { return serverId; } + public String getSearcherName() { return searcherName; } + public String getDefaultSummary() { return defaultSummary; } + public DocumentdbInfoConfig getDocumentdbInfoConfig() { return documentdbInfoConfig; } + public SchemaInfo getSchemaInfo() { return schemaInfo; } } diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/IndexedBackend.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/IndexedBackend.java index 294aff8d78b..9836934acc1 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/IndexedBackend.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/IndexedBackend.java @@ -13,7 +13,6 @@ import com.yahoo.search.query.Ranking; import com.yahoo.search.result.ErrorMessage; import com.yahoo.search.result.Hit; import com.yahoo.search.result.HitGroup; -import com.yahoo.search.schema.SchemaInfo; import java.io.IOException; import java.util.Optional; @@ -38,23 +37,15 @@ public class IndexedBackend extends VespaBackend { /** * Creates a Fastsearcher. * - * @param serverId the resource pool used to create direct connections to the local search nodes when - * bypassing the dispatch node * @param dispatcher the dispatcher used (when enabled) to send summary requests over the rpc protocol. * Eventually we will move everything to this protocol and never use dispatch nodes. * At that point we won't need a cluster searcher above this to select and pass the right * backend. - * @param docSumParams document summary parameters * @param clusterParams the cluster number, and other cluster backend parameters - * @param documentdbInfoConfig document database parameters */ - public IndexedBackend(String serverId, - Dispatcher dispatcher, - SummaryParameters docSumParams, - ClusterParams clusterParams, - DocumentdbInfoConfig documentdbInfoConfig, - SchemaInfo schemaInfo) { - init(serverId, docSumParams, clusterParams, documentdbInfoConfig, schemaInfo); + public IndexedBackend(ClusterParams clusterParams, Dispatcher dispatcher) + { + super(clusterParams); this.dispatcher = dispatcher; } diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/SummaryParameters.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/SummaryParameters.java deleted file mode 100644 index 8751a730229..00000000000 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/SummaryParameters.java +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.prelude.fastsearch; - - -/** - * Wrapper for document summary parameters and configuration. - * - * @author Steinar Knutsen - */ -public class SummaryParameters { - - public final String defaultClass; - - public SummaryParameters(String defaultClass) { - if (defaultClass != null && defaultClass.isEmpty()) - this.defaultClass = null; - else - this.defaultClass = defaultClass; - } - -} diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackend.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackend.java index 5191bfc4f41..761cb22be57 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackend.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackend.java @@ -16,15 +16,14 @@ import com.yahoo.search.schema.RankProfile; import com.yahoo.search.grouping.vespa.GroupingExecutor; import com.yahoo.search.result.ErrorMessage; import com.yahoo.search.result.Hit; -import com.yahoo.search.schema.SchemaInfo; import com.yahoo.searchlib.aggregation.Grouping; import java.util.ArrayList; import java.util.Iterator; -import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.stream.Collectors; /** * Superclass for backend searchers. @@ -36,14 +35,14 @@ public abstract class VespaBackend { /** for vespa-internal use only; consider renaming the summary class */ public static final String SORTABLE_ATTRIBUTES_SUMMARY_CLASS = "attributeprefetch"; - private String serverId; + private final String serverId; /** The set of all document databases available in the backend handled by this searcher */ - private final Map<String, DocumentDatabase> documentDbs = new LinkedHashMap<>(); - private DocumentDatabase defaultDocumentDb = null; + private final Map<String, DocumentDatabase> documentDbs; + private final DocumentDatabase defaultDocumentDb; /** Default docsum class. null means "unset" and is the default value */ - private String defaultDocsumClass = null; + private final String defaultDocsumClass; /** Returns an iterator which returns all hits below this result **/ private static Iterator<Hit> hitIterator(Result result) { @@ -51,14 +50,29 @@ public abstract class VespaBackend { } /** The name of this source */ - private String name; + private final String name; + + protected VespaBackend(ClusterParams clusterParams) { + this.serverId = clusterParams.getServerId(); + this.name = clusterParams.getSearcherName(); + this.defaultDocsumClass = clusterParams.getDefaultSummary(); + + Validator.ensureNotNull("Name of Vespa backend integration", name); + + List<DocumentDatabase> dbs = new ArrayList<>(); + if (clusterParams.getDocumentdbInfoConfig() != null) { + for (DocumentdbInfoConfig.Documentdb docDb : clusterParams.getDocumentdbInfoConfig().documentdb()) { + DocumentDatabase db = new DocumentDatabase(clusterParams.getSchemaInfo().schemas().get(docDb.name())); + dbs.add(db); + } + } + this.defaultDocumentDb = dbs.isEmpty() ? null : dbs.get(0); + this.documentDbs = dbs.stream().collect(Collectors.toMap(db -> db.schema().name(), db -> db)); + } public final String getName() { return name; } protected final String getDefaultDocsumClass() { return defaultDocsumClass; } - /** Sets default document summary class. Default is null */ - private void setDefaultDocsumClass(String docsumClass) { defaultDocsumClass = docsumClass; } - /** * Searches a search cluster * This is an endpoint - searchers will never propagate the search to any nested searcher. @@ -127,25 +141,6 @@ public abstract class VespaBackend { } } - public final void init(String serverId, SummaryParameters docSumParams, ClusterParams clusterParams, - DocumentdbInfoConfig documentdbInfoConfig, SchemaInfo schemaInfo) { - this.serverId = serverId; - this.name = clusterParams.searcherName; - - Validator.ensureNotNull("Name of Vespa backend integration", getName()); - - setDefaultDocsumClass(docSumParams.defaultClass); - - if (documentdbInfoConfig != null) { - for (DocumentdbInfoConfig.Documentdb docDb : documentdbInfoConfig.documentdb()) { - DocumentDatabase db = new DocumentDatabase(schemaInfo.schemas().get(docDb.name())); - if (documentDbs.isEmpty()) - defaultDocumentDb = db; - documentDbs.put(docDb.name(), db); - } - } - } - protected void transformQuery(Query query) { } public Result search(String schema, Query query) { diff --git a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/StreamingBackend.java b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/StreamingBackend.java index 9953d76f50a..6b81ab0fa97 100644 --- a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/StreamingBackend.java +++ b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/StreamingBackend.java @@ -11,6 +11,7 @@ import com.yahoo.fs4.DocsumPacket; import com.yahoo.messagebus.routing.Route; import com.yahoo.prelude.Ping; import com.yahoo.prelude.Pong; +import com.yahoo.prelude.fastsearch.ClusterParams; import com.yahoo.prelude.fastsearch.FastHit; import com.yahoo.prelude.fastsearch.GroupingListHit; import com.yahoo.prelude.fastsearch.TimeoutException; @@ -46,6 +47,7 @@ import java.util.logging.Logger; */ public class StreamingBackend extends VespaBackend { + private static final Logger log = Logger.getLogger(StreamingBackend.class.getName()); private static final CompoundName streamingUserid = CompoundName.from("streaming.userid"); private static final CompoundName streamingGroupname = CompoundName.from("streaming.groupname"); private static final CompoundName streamingSelection = CompoundName.from("streaming.selection"); @@ -53,41 +55,35 @@ public class StreamingBackend extends VespaBackend { static final String STREAMING_STATISTICS = "streaming.statistics"; private final VisitorFactory visitorFactory; private final TracingOptions tracingOptions; - private static final Logger log = Logger.getLogger(StreamingBackend.class.getName()); - private Route route; + private final Route route; /** The configId used to access the searchcluster. */ - private String searchClusterName = null; + private final String searchClusterName; /** The route to the storage cluster. */ - private String storageClusterRouteSpec = null; + private final String storageClusterRouteSpec; - StreamingBackend(VisitorFactory visitorFactory) { - this.visitorFactory = visitorFactory; - tracingOptions = TracingOptions.DEFAULT; + StreamingBackend(ClusterParams clusterParams, String searchClusterName, VisitorFactory visitorFactory, String storageClusterRouteSpec) { + this(clusterParams, searchClusterName, visitorFactory, storageClusterRouteSpec, TracingOptions.DEFAULT); } - StreamingBackend(VisitorFactory visitorFactory, TracingOptions tracingOptions) { + StreamingBackend(ClusterParams clusterParams, String searchClusterName, VisitorFactory visitorFactory, String storageClusterRouteSpec, TracingOptions tracingOptions) { + super(clusterParams); this.visitorFactory = visitorFactory; this.tracingOptions = tracingOptions; + this.searchClusterName = searchClusterName; + this.storageClusterRouteSpec = storageClusterRouteSpec; + this.route = Route.parse(storageClusterRouteSpec); } - public StreamingBackend(VespaDocumentAccess access) { - this(new VespaVisitorFactory(access)); + public StreamingBackend(ClusterParams clusterParams, String searchClusterName, VespaDocumentAccess access, String storageClusterRouteSpec) { + this(clusterParams, searchClusterName, new VespaVisitorFactory(access), storageClusterRouteSpec); } private String getSearchClusterName() { return searchClusterName; } - private String getStorageClusterRouteSpec() { return storageClusterRouteSpec; } - public final void setSearchClusterName(String clusterName) { this.searchClusterName = clusterName; } - public final void setStorageClusterRouteSpec(String storageClusterRouteSpec) { - this.storageClusterRouteSpec = storageClusterRouteSpec; - } - - @Override - protected void doPartialFill(Result result, String summaryClass) { - } + @Override protected void doPartialFill(Result result, String summaryClass) { } private double durationInMillisFromNanoTime(long startTimeNanos) { return (tracingOptions.getClock().nanoTimeNow() - startTimeNanos) / (double)TimeUnit.MILLISECONDS.toNanos(1); @@ -165,11 +161,7 @@ public class StreamingBackend extends VespaBackend { } private void initializeMissingQueryFields(Query query) { - lazyTrace(query, 7, "Routing to storage cluster ", getStorageClusterRouteSpec()); - - if (route == null) { - route = Route.parse(getStorageClusterRouteSpec()); - } + lazyTrace(query, 7, "Routing to storage cluster ", storageClusterRouteSpec); lazyTrace(query, 8, "Route is ", route); lazyTrace(query, 7, "doSearch2(): query docsum class=", 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 ff91f346195..4adbce3add9 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 @@ -7,6 +7,7 @@ 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.prelude.fastsearch.ClusterParams; import com.yahoo.prelude.fastsearch.DocumentdbInfoConfig; import com.yahoo.prelude.fastsearch.FastHit; import com.yahoo.prelude.fastsearch.VespaBackend; @@ -213,6 +214,7 @@ public class ClusterSearcherTestCase { } MyMockBackend(boolean expectAttributePrefetch) { + super(new ClusterParams("container.0")); this.expectAttributePrefetch = expectAttributePrefetch; init(); } diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/IndexedBackendTestCase.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/IndexedBackendTestCase.java index 917206bf00c..58427bee30a 100644 --- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/IndexedBackendTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/IndexedBackendTestCase.java @@ -36,16 +36,13 @@ import static org.junit.jupiter.api.Assertions.*; public class IndexedBackendTestCase { private static final String SCHEMA = "test"; private static final String CLUSTER = "test"; + private static final ClusterParams CLUSTER_PARAMS = new ClusterParams("testhittype", "container.0", null, + documentdbInfoConfig(SCHEMA), schemaInfo(SCHEMA)); @Test void testNullQuery() { Logger.getLogger(IndexedBackend.class.getName()).setLevel(Level.ALL); - IndexedBackend fastSearcher = new IndexedBackend("container.0", - MockDispatcher.create(List.of()), - new SummaryParameters(null), - new ClusterParams("testhittype"), - documentdbInfoConfig(SCHEMA), - schemaInfo(SCHEMA)); + IndexedBackend fastSearcher = new IndexedBackend(CLUSTER_PARAMS, MockDispatcher.create(List.of())); String query = "?junkparam=ignored"; Result result = doSearch(fastSearcher, new Query(query), 0, 10); @@ -65,12 +62,8 @@ public class IndexedBackendTestCase { @Test void testSinglePassGroupingIsForcedWithSingleNodeGroups() { - IndexedBackend fastSearcher = new IndexedBackend("container.0", - MockDispatcher.create(List.of(new Node(CLUSTER, 0, "host0", 0))), - new SummaryParameters(null), - new ClusterParams("testhittype"), - documentdbInfoConfig(SCHEMA), - schemaInfo(SCHEMA)); + IndexedBackend fastSearcher = new IndexedBackend(CLUSTER_PARAMS, + MockDispatcher.create(List.of(new Node(CLUSTER, 0, "host0", 0)))); Query q = new Query("?query=foo"); GroupingRequest request1 = GroupingRequest.newInstance(q); request1.setRootOperation(new AllOperation()); @@ -88,12 +81,8 @@ public class IndexedBackendTestCase { @Test void testRankProfileValidation() { - IndexedBackend fastSearcher = new IndexedBackend("container.0", - MockDispatcher.create(List.of(new Node(CLUSTER, 0, "host0", 0))), - new SummaryParameters(null), - new ClusterParams("testhittype"), - documentdbInfoConfig(SCHEMA), - schemaInfo(SCHEMA)); + IndexedBackend fastSearcher = new IndexedBackend(CLUSTER_PARAMS, + MockDispatcher.create(List.of(new Node(CLUSTER, 0, "host0", 0)))); 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")); @@ -101,18 +90,15 @@ public class IndexedBackendTestCase { @Test void testSummaryNeedsQuery() { - var documentDb = new DocumentdbInfoConfig(new DocumentdbInfoConfig.Builder().documentdb(new DocumentdbInfoConfig.Documentdb.Builder().name(SCHEMA))); var schema = new Schema.Builder(SCHEMA) .add(new DocumentSummary.Builder("default").build()) .add(new RankProfile.Builder("default").setHasRankFeatures(false) .setHasSummaryFeatures(false) .build()); - IndexedBackend backend = new IndexedBackend("container.0", - MockDispatcher.create(Collections.singletonList(new Node(CLUSTER, 0, "host0", 0))), - new SummaryParameters(null), - new ClusterParams("testhittype"), - documentDb, - new SchemaInfo(List.of(schema.build()), List.of())); + var backend = new IndexedBackend(new ClusterParams(CLUSTER_PARAMS.getSearcherName(), CLUSTER_PARAMS.getServerId(), + CLUSTER_PARAMS.getDefaultSummary(), CLUSTER_PARAMS.getDocumentdbInfoConfig(), + new SchemaInfo(List.of(schema.build()), List.of())), + MockDispatcher.create(Collections.singletonList(new Node(CLUSTER, 0, "host0", 0)))); Query q = new Query("?query=foo"); Result result = doSearch(backend, q, 0, 10); assertFalse(backend.summaryNeedsQuery(q)); @@ -127,12 +113,7 @@ public class IndexedBackendTestCase { void testSinglePassGroupingIsNotForcedWithSingleNodeGroups() { MockDispatcher dispatcher = MockDispatcher.create(List.of(new Node(CLUSTER, 0, "host0", 0), new Node(CLUSTER, 2, "host1", 0))); - IndexedBackend fastSearcher = new IndexedBackend("container.0", - dispatcher, - new SummaryParameters(null), - new ClusterParams("testhittype"), - documentdbInfoConfig(SCHEMA), - schemaInfo(SCHEMA)); + IndexedBackend fastSearcher = new IndexedBackend(CLUSTER_PARAMS, dispatcher); Query q = new Query("?query=foo"); GroupingRequest request1 = GroupingRequest.newInstance(q); request1.setRootOperation(new AllOperation()); @@ -185,12 +166,12 @@ public class IndexedBackendTestCase { return searcher.search(SCHEMA, new Query(query)); } - private DocumentdbInfoConfig documentdbInfoConfig(String schemaName) { + private static DocumentdbInfoConfig documentdbInfoConfig(String schemaName) { var db = new DocumentdbInfoConfig.Documentdb.Builder().name(schemaName); return new DocumentdbInfoConfig.Builder().documentdb(db).build(); } - private SchemaInfo schemaInfo(String schemaName) { + private static SchemaInfo schemaInfo(String schemaName) { var schema = new Schema.Builder(schemaName); schema.add(new RankProfile.Builder("default").build()); return new SchemaInfo(List.of(schema.build()), List.of()); diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/PartialFillTestCase.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/PartialFillTestCase.java index 7760e204d4b..7e3509cbef9 100644 --- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/PartialFillTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/PartialFillTestCase.java @@ -17,9 +17,13 @@ import static org.junit.jupiter.api.Assertions.*; * @author havardpe */ public class PartialFillTestCase { + private static final ClusterParams CLUSTER_PARAMS = new ClusterParams("container.0"); public static class FS4 extends VespaBackend { public List<Result> history = new ArrayList<>(); + FS4() { + super(CLUSTER_PARAMS); + } protected Result doSearch2(String schema, Query query) { return new Result(query); } @@ -29,6 +33,9 @@ public class PartialFillTestCase { } public static class BadFS4 extends VespaBackend { + BadFS4() { + super(CLUSTER_PARAMS); + } protected Result doSearch2(String schema, Query query) { return new Result(query); } diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/rpc/RpcSearchInvokerTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/rpc/RpcSearchInvokerTest.java index ef8e0522337..b6fa385cfae 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/rpc/RpcSearchInvokerTest.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/rpc/RpcSearchInvokerTest.java @@ -5,6 +5,7 @@ package com.yahoo.search.dispatch.rpc; import ai.vespa.searchlib.searchprotocol.protobuf.SearchProtocol; import com.google.common.collect.ImmutableMap; import com.yahoo.compress.CompressionType; +import com.yahoo.prelude.fastsearch.ClusterParams; import com.yahoo.prelude.fastsearch.VespaBackend; import com.yahoo.search.Query; import com.yahoo.search.Result; @@ -118,7 +119,7 @@ public class RpcSearchInvokerTest { } private VespaBackend mockSearcher() { - return new VespaBackend() { + return new VespaBackend(new ClusterParams("container.0")) { @Override protected Result doSearch2(String schema, Query query) { fail("Unexpected call"); diff --git a/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/StreamingSearcherTestCase.java b/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/StreamingSearcherTestCase.java index b96dd97f76c..8d264f9860b 100644 --- a/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/StreamingSearcherTestCase.java +++ b/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/StreamingSearcherTestCase.java @@ -9,7 +9,6 @@ import com.yahoo.messagebus.routing.Route; import com.yahoo.prelude.fastsearch.ClusterParams; import com.yahoo.prelude.fastsearch.DocumentdbInfoConfig; import com.yahoo.document.select.parser.ParseException; -import com.yahoo.prelude.fastsearch.SummaryParameters; import com.yahoo.prelude.fastsearch.TimeoutException; import com.yahoo.search.Query; import com.yahoo.search.Result; @@ -46,6 +45,7 @@ public class StreamingSearcherTestCase { public static final String USERDOC_ID_PREFIX = "id:namespace:mytype:n=1:userspecific"; public static final String GROUPDOC_ID_PREFIX = "id:namespace:mytype:g=group1:userspecific"; + private static final ClusterParams CLUSTER_PARAMS = new ClusterParams("clusterName"); private static class MockVisitor implements Visitor { private final Query query; @@ -229,15 +229,12 @@ public class StreamingSearcherTestCase { @Test void testBasics() { MockVisitorFactory factory = new MockVisitorFactory(); - StreamingBackend searcher = new StreamingBackend(factory); - var schema = new Schema.Builder("test"); schema.add(new com.yahoo.search.schema.DocumentSummary.Builder("default").build()); - searcher.init("container.0", - new SummaryParameters("default"), - new ClusterParams("clusterName"), + ClusterParams clusterParams = new ClusterParams("clusterName", "server.0", "default", new DocumentdbInfoConfig.Builder().documentdb(new DocumentdbInfoConfig.Documentdb.Builder().name("test")).build(), new SchemaInfo(List.of(schema.build()), List.of())); + StreamingBackend searcher = new StreamingBackend(clusterParams, "search-cluster-A", factory, "content-cluster-A"); // Magic query values are used to trigger specific behaviors from mock visitor. checkError(searcher, "/?query=noselection", @@ -310,7 +307,7 @@ public class StreamingSearcherTestCase { clock = MockUtils.mockedClockReturning(firstTimestamp, additionalTimestamps); options = new TracingOptions(sampler, exporter, clock, 8, 2.0); factory = new MockVisitorFactory(); - searcher = new StreamingBackend(factory, options); + searcher = new StreamingBackend(CLUSTER_PARAMS, "search-cluster-A", factory, "content-cluster-A", options); } private TraceFixture() { diff --git a/dependency-versions/pom.xml b/dependency-versions/pom.xml index e06b8e0dd4b..77292dcea8c 100644 --- a/dependency-versions/pom.xml +++ b/dependency-versions/pom.xml @@ -65,7 +65,7 @@ <assertj.vespa.version>3.25.3</assertj.vespa.version> <!-- Athenz dependencies. Make sure these dependencies match those in Vespa's internal repositories --> - <aws-sdk.vespa.version>1.12.677</aws-sdk.vespa.version> + <aws-sdk.vespa.version>1.12.678</aws-sdk.vespa.version> <athenz.vespa.version>1.11.53</athenz.vespa.version> <!-- Athenz END --> diff --git a/document/src/vespa/document/select/branch.cpp b/document/src/vespa/document/select/branch.cpp index 02f767c96b5..6035b5fe9a0 100644 --- a/document/src/vespa/document/select/branch.cpp +++ b/document/src/vespa/document/select/branch.cpp @@ -39,6 +39,10 @@ namespace { ResultList traceAndValue(const T& value, std::ostream& out, const Node& leftNode, const Node& rightNode) { + out << "And (lhs):\n"; + (void)leftNode.trace(value, out); + out << "And (rhs):\n"; + (void)rightNode.trace(value, out); out << "And - Left branch returned " << leftNode.contains(value) << ".\n"; out << "And - Right branch returned " << rightNode.contains(value) << ".\n"; return leftNode.contains(value) && rightNode.contains(value); @@ -83,6 +87,10 @@ namespace { ResultList traceOrValue(const T& value, std::ostream& out, const Node& leftNode, const Node& rightNode) { + out << "Or (lhs):\n"; + (void)leftNode.trace(value, out); + out << "Or (rhs):\n"; + (void)rightNode.trace(value, out); out << "Or - Left branch returned " << leftNode.contains(value) << ".\n"; out << "Or - Right branch returned " << rightNode.contains(value) << ".\n"; return leftNode.contains(value) || rightNode.contains(value); @@ -122,6 +130,8 @@ namespace { template<typename T> ResultList traceNotValue(const T& value, std::ostream& out, const Node& node) { + out << "Not:\n"; + (void)node.trace(value, out); out << "Not - Child returned " << node.contains(value) << ". Returning opposite.\n"; return !node.contains(value); diff --git a/parent/pom.xml b/parent/pom.xml index cb6e4c1d433..0511b227211 100644 --- a/parent/pom.xml +++ b/parent/pom.xml @@ -317,7 +317,7 @@ --> <groupId>org.openrewrite.maven</groupId> <artifactId>rewrite-maven-plugin</artifactId> - <version>5.24.0</version> + <version>5.25.0</version> <configuration> <activeRecipes> <recipe>org.openrewrite.java.testing.junit5.JUnit5BestPractices</recipe> @@ -327,7 +327,7 @@ <dependency> <groupId>org.openrewrite.recipe</groupId> <artifactId>rewrite-testing-frameworks</artifactId> - <version>2.4.1</version> + <version>2.5.0</version> </dependency> </dependencies> </plugin> @@ -1187,7 +1187,7 @@ See pluginManagement of rewrite-maven-plugin for more details --> <groupId>org.openrewrite.recipe</groupId> <artifactId>rewrite-recipe-bom</artifactId> - <version>2.7.1</version> + <version>2.8.0</version> <type>pom</type> <scope>import</scope> </dependency> diff --git a/searchcore/src/tests/proton/common/cachedselect_test.cpp b/searchcore/src/tests/proton/common/cachedselect_test.cpp index 8c2913a3d1b..a0c8fef3b83 100644 --- a/searchcore/src/tests/proton/common/cachedselect_test.cpp +++ b/searchcore/src/tests/proton/common/cachedselect_test.cpp @@ -102,7 +102,8 @@ makeDocTypeRepo() addField("aa", DataType::T_INT). addField("aaa", Array(DataType::T_INT)). addField("aaw", Wset(DataType::T_INT)). - addField("ab", DataType::T_INT)); + addField("ab", DataType::T_INT)). + imported_field("my_imported_field"); builder.document(doc_type_id + 1, type_name_2, Struct(header_name_2), Struct(body_name_2). addField("ic", DataType::T_STRING). @@ -152,12 +153,17 @@ checkSelect(const NodeUP &sel, void checkSelect(const CachedSelect::SP &cs, + uint32_t docId, const Document &doc, const Result &exp) { + SelectContext ctx(*cs); + ctx._docId = docId; + ctx._doc = &doc; + ctx.getAttributeGuards(); bool expSessionContains = (cs->preDocOnlySelect() || (exp == Result::True)); - EXPECT_TRUE(checkSelect(cs->docSelect(), Context(doc), exp)); - EXPECT_EQUAL(expSessionContains, cs->createSession()->contains(doc)); + EXPECT_TRUE(checkSelect(cs->docSelect(), ctx, exp)); + EXPECT_EQUAL(expSessionContains, cs->createSession()->contains_doc(ctx)); } void @@ -170,7 +176,7 @@ checkSelect(const CachedSelect::SP &cs, ctx._docId = docId; ctx.getAttributeGuards(); EXPECT_TRUE(checkSelect((cs->preDocOnlySelect() ? cs->preDocOnlySelect() : cs->preDocSelect()), ctx, exp)); - EXPECT_EQUAL(expSessionContains, cs->createSession()->contains(ctx)); + EXPECT_EQUAL(expSessionContains, cs->createSession()->contains_pre_doc(ctx)); } void @@ -271,18 +277,22 @@ MyDB::addDoc(uint32_t lid, Document::UP doc(makeDoc(_repo, docId, ia, ib, aa, ab)); _docIdToLid[docId] = lid; - _lidToDocSP[lid] = Document::SP(doc.release()); - AttributeGuard::UP guard = _amgr.getAttribute("aa"); - AttributeVector &av = *guard->get(); - if (lid >= av.getNumDocs()) { - AttributeVector::DocId checkDocId(0u); - ASSERT_TRUE(av.addDoc(checkDocId)); - ASSERT_EQUAL(lid, checkDocId); - } - IntegerAttribute &iav(static_cast<IntegerAttribute &>(av)); - AttributeVector::largeint_t laa(aa); - EXPECT_TRUE(iav.update(lid, laa)); - av.commit(); + _lidToDocSP[lid] = std::move(doc); + auto add_attr_value = [lid, aa](auto guard) { + AttributeVector &av = *guard->get(); + if (lid >= av.getNumDocs()) { + AttributeVector::DocId checkDocId(0u); + ASSERT_TRUE(av.addDoc(checkDocId)); + ASSERT_EQUAL(lid, checkDocId); + } + auto &iav(dynamic_cast<IntegerAttribute &>(av)); + AttributeVector::largeint_t laa(aa); + EXPECT_TRUE(iav.update(lid, laa)); + av.commit(); + }; + + add_attr_value(_amgr.getAttribute("aa")); + add_attr_value(_amgr.getAttribute("my_imported_field")); } @@ -327,15 +337,14 @@ TestFixture::TestFixture() _amgr.addAttribute("aa"); _amgr.addAttribute("aaa", AttributeFactory::createAttribute("aaa", {BasicType::INT32, CollectionType::ARRAY})); _amgr.addAttribute("aaw", AttributeFactory::createAttribute("aaw", {BasicType::INT32, CollectionType::WSET})); + // "Faked" imported attribute, as in `selectpruner_test.cpp` + _amgr.addAttribute("my_imported_field", AttributeFactory::createAttribute("my_imported_field", { BasicType::INT32 })); - _db.reset(new MyDB(*_repoUP, _amgr)); + _db = std::make_unique<MyDB>(*_repoUP, _amgr); } -TestFixture::~TestFixture() -{ -} - +TestFixture::~TestFixture() = default; CachedSelect::SP TestFixture::testParse(const string &selection, @@ -475,45 +484,45 @@ TEST_F("Test that basic select works", TestFixture) cs = f.testParse("test.ia == \"hello\"", "test"); TEST_DO(assertEquals(Stats().fieldNodes(1).attrFieldNodes(0).svAttrFieldNodes(0), *cs)); - TEST_DO(checkSelect(cs, db.getDoc(1u), Result::True)); - TEST_DO(checkSelect(cs, db.getDoc(2u), Result::False)); - TEST_DO(checkSelect(cs, db.getDoc(3u), Result::False)); - TEST_DO(checkSelect(cs, db.getDoc(4u), Result::False)); + TEST_DO(checkSelect(cs, 1u, db.getDoc(1u), Result::True)); + TEST_DO(checkSelect(cs, 2u, db.getDoc(2u), Result::False)); + TEST_DO(checkSelect(cs, 3u, db.getDoc(3u), Result::False)); + TEST_DO(checkSelect(cs, 4u, db.getDoc(4u), Result::False)); cs = f.testParse("test.ia.foo == \"hello\"", "test"); TEST_DO(assertEquals(Stats().allInvalid(), *cs)); - TEST_DO(checkSelect(cs, db.getDoc(1u), Result::Invalid)); - TEST_DO(checkSelect(cs, db.getDoc(2u), Result::Invalid)); - TEST_DO(checkSelect(cs, db.getDoc(3u), Result::Invalid)); - TEST_DO(checkSelect(cs, db.getDoc(4u), Result::Invalid)); + TEST_DO(checkSelect(cs, 1u, db.getDoc(1u), Result::Invalid)); + TEST_DO(checkSelect(cs, 2u, db.getDoc(2u), Result::Invalid)); + TEST_DO(checkSelect(cs, 3u, db.getDoc(3u), Result::Invalid)); + TEST_DO(checkSelect(cs, 4u, db.getDoc(4u), Result::Invalid)); cs = f.testParse("test.ia[2] == \"hello\"", "test"); TEST_DO(assertEquals(Stats().allInvalid(), *cs)); - TEST_DO(checkSelect(cs, db.getDoc(1u), Result::Invalid)); - TEST_DO(checkSelect(cs, db.getDoc(2u), Result::Invalid)); - TEST_DO(checkSelect(cs, db.getDoc(3u), Result::Invalid)); - TEST_DO(checkSelect(cs, db.getDoc(4u), Result::Invalid)); + TEST_DO(checkSelect(cs, 1u, db.getDoc(1u), Result::Invalid)); + TEST_DO(checkSelect(cs, 2u, db.getDoc(2u), Result::Invalid)); + TEST_DO(checkSelect(cs, 3u, db.getDoc(3u), Result::Invalid)); + TEST_DO(checkSelect(cs, 4u, db.getDoc(4u), Result::Invalid)); cs = f.testParse("test.ia{foo} == \"hello\"", "test"); TEST_DO(assertEquals(Stats().allInvalid(), *cs)); - TEST_DO(checkSelect(cs, db.getDoc(1u), Result::Invalid)); - TEST_DO(checkSelect(cs, db.getDoc(2u), Result::Invalid)); - TEST_DO(checkSelect(cs, db.getDoc(3u), Result::Invalid)); - TEST_DO(checkSelect(cs, db.getDoc(4u), Result::Invalid)); + TEST_DO(checkSelect(cs, 1u, db.getDoc(1u), Result::Invalid)); + TEST_DO(checkSelect(cs, 2u, db.getDoc(2u), Result::Invalid)); + TEST_DO(checkSelect(cs, 3u, db.getDoc(3u), Result::Invalid)); + TEST_DO(checkSelect(cs, 4u, db.getDoc(4u), Result::Invalid)); cs = f.testParse("test.ia < \"hello\"", "test"); TEST_DO(assertEquals(Stats().fieldNodes(1).attrFieldNodes(0).svAttrFieldNodes(0), *cs)); - TEST_DO(checkSelect(cs, db.getDoc(1u), Result::False)); - TEST_DO(checkSelect(cs, db.getDoc(2u), Result::True)); - TEST_DO(checkSelect(cs, db.getDoc(3u), Result::True)); - TEST_DO(checkSelect(cs, db.getDoc(4u), Result::Invalid)); + TEST_DO(checkSelect(cs, 1u, db.getDoc(1u), Result::False)); + TEST_DO(checkSelect(cs, 2u, db.getDoc(2u), Result::True)); + TEST_DO(checkSelect(cs, 3u, db.getDoc(3u), Result::True)); + TEST_DO(checkSelect(cs, 4u, db.getDoc(4u), Result::Invalid)); cs = f.testParse("test.aa == 3", "test"); TEST_DO(assertEquals(Stats().preDocOnlySelect().fieldNodes(1).attrFieldNodes(1).svAttrFieldNodes(1), *cs)); - TEST_DO(checkSelect(cs, db.getDoc(1u), Result::False)); - TEST_DO(checkSelect(cs, db.getDoc(2u), Result::True)); - TEST_DO(checkSelect(cs, db.getDoc(3u), Result::False)); - TEST_DO(checkSelect(cs, db.getDoc(4u), Result::False)); + TEST_DO(checkSelect(cs, 1u, db.getDoc(1u), Result::False)); + TEST_DO(checkSelect(cs, 2u, db.getDoc(2u), Result::True)); + TEST_DO(checkSelect(cs, 3u, db.getDoc(3u), Result::False)); + TEST_DO(checkSelect(cs, 4u, db.getDoc(4u), Result::False)); TEST_DO(checkSelect(cs, 1u, Result::False)); TEST_DO(checkSelect(cs, 2u, Result::True)); TEST_DO(checkSelect(cs, 3u, Result::False)); @@ -521,24 +530,24 @@ TEST_F("Test that basic select works", TestFixture) cs = f.testParse("test.aa.foo == 3", "test"); TEST_DO(assertEquals(Stats().allInvalid(), *cs)); - TEST_DO(checkSelect(cs, db.getDoc(1u), Result::Invalid)); - TEST_DO(checkSelect(cs, db.getDoc(2u), Result::Invalid)); - TEST_DO(checkSelect(cs, db.getDoc(3u), Result::Invalid)); - TEST_DO(checkSelect(cs, db.getDoc(4u), Result::Invalid)); + TEST_DO(checkSelect(cs, 1u, db.getDoc(1u), Result::Invalid)); + TEST_DO(checkSelect(cs, 2u, db.getDoc(2u), Result::Invalid)); + TEST_DO(checkSelect(cs, 3u, db.getDoc(3u), Result::Invalid)); + TEST_DO(checkSelect(cs, 4u, db.getDoc(4u), Result::Invalid)); cs = f.testParse("test.aa[2] == 3", "test"); TEST_DO(assertEquals(Stats().allInvalid(), *cs)); - TEST_DO(checkSelect(cs, db.getDoc(1u), Result::Invalid)); - TEST_DO(checkSelect(cs, db.getDoc(2u), Result::Invalid)); - TEST_DO(checkSelect(cs, db.getDoc(3u), Result::Invalid)); - TEST_DO(checkSelect(cs, db.getDoc(4u), Result::Invalid)); + TEST_DO(checkSelect(cs, 1u, db.getDoc(1u), Result::Invalid)); + TEST_DO(checkSelect(cs, 2u, db.getDoc(2u), Result::Invalid)); + TEST_DO(checkSelect(cs, 3u, db.getDoc(3u), Result::Invalid)); + TEST_DO(checkSelect(cs, 4u, db.getDoc(4u), Result::Invalid)); cs = f.testParse("test.aa{4} > 3", "test"); TEST_DO(assertEquals(Stats().allInvalid(), *cs)); - TEST_DO(checkSelect(cs, db.getDoc(1u), Result::Invalid)); - TEST_DO(checkSelect(cs, db.getDoc(2u), Result::Invalid)); - TEST_DO(checkSelect(cs, db.getDoc(3u), Result::Invalid)); - TEST_DO(checkSelect(cs, db.getDoc(4u), Result::Invalid)); + TEST_DO(checkSelect(cs, 1u, db.getDoc(1u), Result::Invalid)); + TEST_DO(checkSelect(cs, 2u, db.getDoc(2u), Result::Invalid)); + TEST_DO(checkSelect(cs, 3u, db.getDoc(3u), Result::Invalid)); + TEST_DO(checkSelect(cs, 4u, db.getDoc(4u), Result::Invalid)); cs = f.testParse("test.aaa[2] == 3", "test"); TEST_DO(assertEquals(Stats().fieldNodes(1).attrFieldNodes(1).svAttrFieldNodes(0), *cs)); @@ -548,10 +557,10 @@ TEST_F("Test that basic select works", TestFixture) cs = f.testParse("test.aa < 45", "test"); TEST_DO(assertEquals(Stats().preDocOnlySelect().fieldNodes(1).attrFieldNodes(1).svAttrFieldNodes(1), *cs)); - TEST_DO(checkSelect(cs, db.getDoc(1u), Result::False)); - TEST_DO(checkSelect(cs, db.getDoc(2u), Result::True)); - TEST_DO(checkSelect(cs, db.getDoc(3u), Result::Invalid)); - TEST_DO(checkSelect(cs, db.getDoc(4u), Result::Invalid)); + TEST_DO(checkSelect(cs, 1u, db.getDoc(1u), Result::False)); + TEST_DO(checkSelect(cs, 2u, db.getDoc(2u), Result::True)); + TEST_DO(checkSelect(cs, 3u, db.getDoc(3u), Result::Invalid)); + TEST_DO(checkSelect(cs, 4u, db.getDoc(4u), Result::Invalid)); TEST_DO(checkSelect(cs, 1u, Result::False, false)); TEST_DO(checkSelect(cs, 2u, Result::True, true)); TEST_DO(checkSelect(cs, 3u, Result::Invalid, false)); @@ -580,9 +589,9 @@ TEST_F("Test that single value attribute combined with non-attribute field resul TEST_DO(checkSelect(cs, 1u, Result::Invalid, true)); TEST_DO(checkSelect(cs, 2u, Result::Invalid, true)); TEST_DO(checkSelect(cs, 3u, Result::False, false)); - TEST_DO(checkSelect(cs, f.db().getDoc(1u), Result::True)); - TEST_DO(checkSelect(cs, f.db().getDoc(2u), Result::False)); - TEST_DO(checkSelect(cs, f.db().getDoc(3u), Result::False)); + TEST_DO(checkSelect(cs, 1u, f.db().getDoc(1u), Result::True)); + TEST_DO(checkSelect(cs, 2u, f.db().getDoc(2u), Result::False)); + TEST_DO(checkSelect(cs, 3u, f.db().getDoc(3u), Result::False)); } TEST_F("Test that single value attribute with complex attribute field results in pre-document select pruner", PreDocSelectFixture) @@ -593,9 +602,39 @@ TEST_F("Test that single value attribute with complex attribute field results in TEST_DO(checkSelect(cs, 1u, Result::Invalid, true)); TEST_DO(checkSelect(cs, 2u, Result::Invalid, true)); TEST_DO(checkSelect(cs, 3u, Result::False, false)); - TEST_DO(checkSelect(cs, f.db().getDoc(1u), Result::False)); - TEST_DO(checkSelect(cs, f.db().getDoc(2u), Result::False)); - TEST_DO(checkSelect(cs, f.db().getDoc(3u), Result::False)); + TEST_DO(checkSelect(cs, 1u, f.db().getDoc(1u), Result::False)); + TEST_DO(checkSelect(cs, 2u, f.db().getDoc(2u), Result::False)); + TEST_DO(checkSelect(cs, 3u, f.db().getDoc(3u), Result::False)); +} + +TEST_F("Imported field can be used in pre-doc selections with only attribute fields", PreDocSelectFixture) { + auto cs = f.testParse("test.my_imported_field == 3", "test"); + TEST_DO(assertEquals(Stats().preDocOnlySelect().fieldNodes(1).attrFieldNodes(1).svAttrFieldNodes(1), *cs)); + + TEST_DO(checkSelect(cs, 1u, Result::True, true)); + TEST_DO(checkSelect(cs, 2u, Result::True, true)); + TEST_DO(checkSelect(cs, 3u, Result::False, false)); + // Cannot match against document here since preDocOnly is set; will just return false. + TEST_DO(checkSelect(cs, 1u, f.db().getDoc(1u), Result::False)); + TEST_DO(checkSelect(cs, 2u, f.db().getDoc(2u), Result::False)); + TEST_DO(checkSelect(cs, 3u, f.db().getDoc(3u), Result::False)); +} + +TEST_F("Imported field can be used in doc selections with mixed attribute/non-attribute fields", PreDocSelectFixture) { + // `id.namespace` requires a doc store fetch and cannot be satisfied by attributes alone + auto cs = f.testParse("test.my_imported_field == 3 and id.namespace != 'foo'", "test"); + TEST_DO(assertEquals(Stats().preDocSelect().fieldNodes(2).attrFieldNodes(1).svAttrFieldNodes(1), *cs)); + + // 2 first checks cannot be completed in pre-doc stage alone + TEST_DO(checkSelect(cs, 1u, Result::Invalid, true)); // -> doc eval stage + TEST_DO(checkSelect(cs, 2u, Result::Invalid, true)); // -> doc eval stage + TEST_DO(checkSelect(cs, 3u, Result::False, false)); // short-circuited since attr value 7 != 3 + // When matching against a concrete document, it's crucial that the selection AST contains + // attribute references for at least all imported fields, or we'll implicitly fall back to + // returning null for all imported fields (as they do not exist in the document itself). + TEST_DO(checkSelect(cs, 1u, f.db().getDoc(1u), Result::True)); + TEST_DO(checkSelect(cs, 2u, f.db().getDoc(2u), Result::True)); + TEST_DO(checkSelect(cs, 3u, f.db().getDoc(3u), Result::False)); } TEST_F("Test performance when using attributes", TestFixture) diff --git a/searchcore/src/tests/proton/common/selectpruner_test.cpp b/searchcore/src/tests/proton/common/selectpruner_test.cpp index e175836b838..1f71da5aeda 100644 --- a/searchcore/src/tests/proton/common/selectpruner_test.cpp +++ b/searchcore/src/tests/proton/common/selectpruner_test.cpp @@ -799,6 +799,12 @@ TEST_F("Imported fields with matching attribute names are supported", TestFixtur "test.my_imported_field > 0"); } +TEST_F("Imported fields can be used alongside non-attribute fields", TestFixture) +{ + f.testPrune("test.my_imported_field > 0 and id.namespace != \"foo\"", + "test.my_imported_field > 0 and id.namespace != \"foo\""); +} + // Edge case: document type reconfigured but attribute not yet visible in Proton TEST_F("Imported fields without matching attribute are mapped to constant NullValue", TestFixture) { diff --git a/searchcore/src/vespa/searchcore/proton/common/attributefieldvaluenode.cpp b/searchcore/src/vespa/searchcore/proton/common/attributefieldvaluenode.cpp index d94b0a4c909..d22fed89ad1 100644 --- a/searchcore/src/vespa/searchcore/proton/common/attributefieldvaluenode.cpp +++ b/searchcore/src/vespa/searchcore/proton/common/attributefieldvaluenode.cpp @@ -40,7 +40,7 @@ std::unique_ptr<document::select::Value> AttributeFieldValueNode:: getValue(const Context &context) const { - const auto &sc(static_cast<const SelectContext &>(context)); + const auto &sc(dynamic_cast<const SelectContext &>(context)); uint32_t docId(sc._docId); assert(docId != 0u); const auto& v = sc.guarded_attribute_at_index(_attr_guard_index); diff --git a/searchcore/src/vespa/searchcore/proton/common/cachedselect.cpp b/searchcore/src/vespa/searchcore/proton/common/cachedselect.cpp index 293ed3bdcb9..9f890f5d480 100644 --- a/searchcore/src/vespa/searchcore/proton/common/cachedselect.cpp +++ b/searchcore/src/vespa/searchcore/proton/common/cachedselect.cpp @@ -35,13 +35,16 @@ public: uint32_t _mvAttrs; uint32_t _complexAttrs; - uint32_t getFieldNodes() const { return _fieldNodes; } + [[nodiscard]] uint32_t getFieldNodes() const { return _fieldNodes; } - static uint32_t invalidIdx() { + constexpr static uint32_t invalidIdx() noexcept { return std::numeric_limits<uint32_t>::max(); } AttrVisitor(const search::IAttributeManager &amgr, CachedSelect::AttributeVectors &attributes); + AttrVisitor(const search::IAttributeManager &amgr, + CachedSelect::AttributeVectors &attributes, + AttrMap existing_attr_map); ~AttrVisitor() override; /* @@ -62,6 +65,18 @@ AttrVisitor::AttrVisitor(const search::IAttributeManager &amgr, CachedSelect::At _complexAttrs(0u) {} +AttrVisitor::AttrVisitor(const search::IAttributeManager &amgr, + CachedSelect::AttributeVectors &attributes, + AttrMap existing_attr_map) + : CloningVisitor(), + _amap(std::move(existing_attr_map)), + _amgr(amgr), + _attributes(attributes), + _svAttrs(0u), + _mvAttrs(0u), + _complexAttrs(0u) +{} + AttrVisitor::~AttrVisitor() = default; bool isSingleValueThatWeHandle(BasicType type) { @@ -130,7 +145,7 @@ CachedSelect::Session::Session(std::unique_ptr<document::select::Node> docSelect } bool -CachedSelect::Session::contains(const SelectContext &context) const +CachedSelect::Session::contains_pre_doc(const SelectContext &context) const { if (_preDocSelect && (_preDocSelect->contains(context) == document::select::Result::False)) { return false; @@ -140,10 +155,10 @@ CachedSelect::Session::contains(const SelectContext &context) const } bool -CachedSelect::Session::contains(const document::Document &doc) const +CachedSelect::Session::contains_doc(const SelectContext &context) const { return (_preDocOnlySelect) || - (_docSelect && (_docSelect->contains(doc) == document::select::Result::True)); + (_docSelect && (_docSelect->contains(context) == document::select::Result::True)); } const document::select::Node & @@ -177,8 +192,12 @@ CachedSelect::setPreDocumentSelect(const search::IAttributeManager &attrMgr, if (_fieldNodes == _svAttrFieldNodes) { _preDocOnlySelect = std::move(allAttrVisitor.getNode()); } else if (_svAttrFieldNodes > 0) { - _attributes.clear(); - AttrVisitor someAttrVisitor(attrMgr, _attributes); + // Also let document-level selection use attribute wiring; otherwise imported fields + // would not resolve to anything, as these do not exist in the concrete document itself. + _docSelect = std::move(allAttrVisitor.getNode()); + [[maybe_unused]] size_t attrs_before = _attributes.size(); + AttrVisitor someAttrVisitor(attrMgr, _attributes, std::move(allAttrVisitor._amap)); + assert(_attributes.size() == attrs_before); noDocsPruner.getNode()->visit(someAttrVisitor); _preDocSelect = std::move(someAttrVisitor.getNode()); } diff --git a/searchcore/src/vespa/searchcore/proton/common/cachedselect.h b/searchcore/src/vespa/searchcore/proton/common/cachedselect.h index b0660399f6f..89f002bd939 100644 --- a/searchcore/src/vespa/searchcore/proton/common/cachedselect.h +++ b/searchcore/src/vespa/searchcore/proton/common/cachedselect.h @@ -42,9 +42,10 @@ public: Session(std::unique_ptr<document::select::Node> docSelect, std::unique_ptr<document::select::Node> preDocOnlySelect, std::unique_ptr<document::select::Node> preDocSelect); - bool contains(const SelectContext &context) const; - bool contains(const document::Document &doc) const; - const document::select::Node &selectNode() const; + [[nodiscard]] bool contains_pre_doc(const SelectContext &context) const; + // Precondition: context must have non-nullptr _doc + [[nodiscard]] bool contains_doc(const SelectContext &context) const; + [[nodiscard]] const document::select::Node &selectNode() const; }; using AttributeVectors = std::vector<std::shared_ptr<search::attribute::ReadableAttributeVector>>; diff --git a/searchcore/src/vespa/searchcore/proton/common/selectpruner.h b/searchcore/src/vespa/searchcore/proton/common/selectpruner.h index 3612914fc6f..3322a07ebd7 100644 --- a/searchcore/src/vespa/searchcore/proton/common/selectpruner.h +++ b/searchcore/src/vespa/searchcore/proton/common/selectpruner.h @@ -36,7 +36,7 @@ public: class SelectPruner : public document::select::CloningVisitor, - public SelectPrunerBase + public SelectPrunerBase { public: private: @@ -53,16 +53,16 @@ public: bool hasFields, bool hasDocuments); - SelectPruner(const SelectPruner *rhs); - virtual ~SelectPruner(); + explicit SelectPruner(const SelectPruner *rhs); + ~SelectPruner() override; - uint32_t getFieldNodes() const { return _fieldNodes; } - uint32_t getAttrFieldNodes() const { return _attrFieldNodes; } - const document::select::ResultSet & getResultSet() const { return _resultSet; } - bool isFalse() const; - bool isTrue() const; - bool isInvalid() const; - bool isConst() const; + [[nodiscard]] uint32_t getFieldNodes() const noexcept { return _fieldNodes; } + [[nodiscard]] uint32_t getAttrFieldNodes() const noexcept { return _attrFieldNodes; } + [[nodiscard]] const document::select::ResultSet & getResultSet() const noexcept { return _resultSet; } + [[nodiscard]] bool isFalse() const; + [[nodiscard]] bool isTrue() const; + [[nodiscard]] bool isInvalid() const; + [[nodiscard]] bool isConst() const; void trace(std::ostream &t); void process(const document::select::Node &node); private: @@ -83,8 +83,8 @@ private: void setTernaryConst(bool val); void set_null_value_node(); void resolveTernaryConst(bool wantInverted); - bool isInvalidVal() const; - bool isNullVal() const; + [[nodiscard]] bool isInvalidVal() const; + [[nodiscard]] bool isNullVal() const; void swap(SelectPruner &rhs); }; diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp b/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp index e9d233ef6ec..85c0eb0c1f2 100644 --- a/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp +++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp @@ -171,28 +171,31 @@ public: } } - bool willAlwaysFail() const { return _willAlwaysFail; } + [[nodiscard]] bool willAlwaysFail() const noexcept { return _willAlwaysFail; } - bool match(const search::DocumentMetaData & meta) const { + [[nodiscard]] bool match(const search::DocumentMetaData & meta) const { if (meta.lid >= _docidLimit) { return false; } if (_dscTrue || _metaOnly) { return true; } - if (_selectCxt) { - _selectCxt->_docId = meta.lid; - } if (!_gidFilter.gid_might_match_selection(meta.gid)) { return false; } - return _selectSession->contains(*_selectCxt); + assert(_selectCxt); + _selectCxt->_docId = meta.lid; + _selectCxt->_doc = nullptr; + return _selectSession->contains_pre_doc(*_selectCxt); } - bool match(const search::DocumentMetaData & meta, const Document * doc) const { + [[nodiscard]] bool match(const search::DocumentMetaData & meta, const Document * doc) const { if (_dscTrue || _metaOnly) { return true; } - return (doc && (doc->getId().getGlobalId() == meta.gid) && _selectSession->contains(*doc)); + assert(_selectCxt); + _selectCxt->_docId = meta.lid; + _selectCxt->_doc = doc; + return (doc && (doc->getId().getGlobalId() == meta.gid) && _selectSession->contains_doc(*_selectCxt)); } private: bool _dscTrue; diff --git a/searchlib/src/tests/diskindex/bitvector/CMakeLists.txt b/searchlib/src/tests/diskindex/bitvector/CMakeLists.txt index b227d3d5fa8..f70a1efd23c 100644 --- a/searchlib/src/tests/diskindex/bitvector/CMakeLists.txt +++ b/searchlib/src/tests/diskindex/bitvector/CMakeLists.txt @@ -4,5 +4,6 @@ vespa_add_executable(searchlib_bitvector_test-diskindex_app TEST bitvector_test.cpp DEPENDS searchlib + GTest::gtest ) vespa_add_test(NAME searchlib_bitvector_test-diskindex_app COMMAND searchlib_bitvector_test-diskindex_app) diff --git a/searchlib/src/tests/diskindex/bitvector/bitvector_test.cpp b/searchlib/src/tests/diskindex/bitvector/bitvector_test.cpp index 6032b7156ff..5e4b8763534 100644 --- a/searchlib/src/tests/diskindex/bitvector/bitvector_test.cpp +++ b/searchlib/src/tests/diskindex/bitvector/bitvector_test.cpp @@ -1,16 +1,13 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/vespalib/testkit/testapp.h> #include <vespa/searchlib/index/field_length_info.h> #include <vespa/searchlib/diskindex/bitvectordictionary.h> #include <vespa/searchlib/diskindex/fieldwriter.h> #include <vespa/searchlib/index/dummyfileheadercontext.h> #include <vespa/searchcommon/common/schema.h> +#include <vespa/vespalib/gtest/gtest.h> #include <filesystem> -#include <vespa/log/log.h> -LOG_SETUP("bitvector_test"); - using namespace search::index; using search::index::schema::DataType; @@ -58,38 +55,57 @@ FieldWriterWrapper::add(uint32_t docId) daf.elements().emplace_back(0); daf.elements().back().setNumOccs(1); daf.word_positions().emplace_back(0); - //LOG(info, "add(%" PRIu64 ", %u)", wordNum, docId); _writer.add(daf); return *this; } -class Test : public vespalib::TestApp +struct TestParam { + bool directio; + bool readmmap; +}; + +std::ostream& operator<<(std::ostream& os, const TestParam& param) +{ + os << (param.directio ? "directio" : "normal") << (param.readmmap ? "mmap" : "read"); + return os; +} + +class BitVectorTest : public ::testing::TestWithParam<TestParam> { -private: +protected: Schema _schema; uint32_t _indexId; -public: - void requireThatDictionaryHandlesNoEntries(bool directio, bool readmmap); - void requireThatDictionaryHandlesMultipleEntries(bool directio, bool readmmap); - - Test(); - ~Test() override; - int Main() override; + BitVectorTest(); + ~BitVectorTest() override; }; -void -Test::requireThatDictionaryHandlesNoEntries(bool directio, bool readmmap) +BitVectorTest::BitVectorTest() + : ::testing::TestWithParam<TestParam>(), + _schema(), + _indexId(0) +{ + _schema.addIndexField(Schema::IndexField("f1", DataType::STRING)); +} + +BitVectorTest::~BitVectorTest() = default; + +INSTANTIATE_TEST_SUITE_P(BitVectorMultiTest, BitVectorTest, + ::testing::Values(TestParam{false, false}, TestParam{true, false}, TestParam{false, true}), + ::testing::PrintToStringParamName()); + +TEST_P(BitVectorTest, require_that_dictionary_handles_no_entries) { TuneFileSeqWrite tuneFileWrite; TuneFileRandRead tuneFileRead; DummyFileHeaderContext fileHeaderContext; - if (directio) { + if (GetParam().directio) { tuneFileWrite.setWantDirectIO(); tuneFileRead.setWantDirectIO(); } - if (readmmap) + if (GetParam().readmmap) { tuneFileRead.setWantMemoryMap(); + } std::filesystem::create_directory(std::filesystem::path("dump")); FieldWriterWrapper fww(5, 2, "dump/1/"); EXPECT_TRUE(fww.open(_schema, _indexId, tuneFileWrite, fileHeaderContext)); @@ -100,25 +116,25 @@ Test::requireThatDictionaryHandlesNoEntries(bool directio, bool readmmap) BitVectorDictionary dict; BitVectorKeyScope bvScope(BitVectorKeyScope::PERFIELD_WORDS); EXPECT_TRUE(dict.open("dump/1/", tuneFileRead, bvScope)); - EXPECT_EQUAL(5u, dict.getDocIdLimit()); - EXPECT_EQUAL(0u, dict.getEntries().size()); + EXPECT_EQ(5u, dict.getDocIdLimit()); + EXPECT_EQ(0u, dict.getEntries().size()); EXPECT_FALSE(dict.lookup(1)); EXPECT_FALSE(dict.lookup(2)); } -void -Test::requireThatDictionaryHandlesMultipleEntries(bool directio, bool readmmap) +TEST_P(BitVectorTest, require_that_dictionary_handles_multiple_entries) { TuneFileSeqWrite tuneFileWrite; TuneFileRandRead tuneFileRead; DummyFileHeaderContext fileHeaderContext; - if (directio) { + if (GetParam().directio) { tuneFileWrite.setWantDirectIO(); tuneFileRead.setWantDirectIO(); } - if (readmmap) + if (GetParam().readmmap) { tuneFileRead.setWantMemoryMap(); + } FieldWriterWrapper fww(64, 6, "dump/2/"); EXPECT_TRUE(fww.open(_schema, _indexId, tuneFileWrite, fileHeaderContext)); // must have >16 docs in order to create bitvector for a word @@ -149,16 +165,16 @@ Test::requireThatDictionaryHandlesMultipleEntries(bool directio, bool readmmap) BitVectorDictionary dict; BitVectorKeyScope bvScope(BitVectorKeyScope::PERFIELD_WORDS); EXPECT_TRUE(dict.open("dump/2/", tuneFileRead, bvScope)); - EXPECT_EQUAL(64u, dict.getDocIdLimit()); - EXPECT_EQUAL(2u, dict.getEntries().size()); + EXPECT_EQ(64u, dict.getDocIdLimit()); + EXPECT_EQ(2u, dict.getEntries().size()); BitVectorWordSingleKey e; e = dict.getEntries()[0]; - EXPECT_EQUAL(1u, e._wordNum); - EXPECT_EQUAL(17u, e._numDocs); + EXPECT_EQ(1u, e._wordNum); + EXPECT_EQ(17u, e._numDocs); e = dict.getEntries()[1]; - EXPECT_EQUAL(5u, e._wordNum); - EXPECT_EQUAL(23u, e._numDocs); + EXPECT_EQ(5u, e._wordNum); + EXPECT_EQ(23u, e._numDocs); EXPECT_FALSE(dict.lookup(2)); EXPECT_FALSE(dict.lookup(3)); @@ -174,33 +190,14 @@ Test::requireThatDictionaryHandlesMultipleEntries(bool directio, bool readmmap) EXPECT_TRUE(*bv5exp == *bv5act); } -Test::Test() - : _schema(), - _indexId(0) -{ - _schema.addIndexField(Schema::IndexField("f1", DataType::STRING)); } -Test::~Test() = default; - int -Test::Main() +main(int argc, char* argv[]) { - TEST_INIT("bitvector_test"); - - if (_argc > 0) { - DummyFileHeaderContext::setCreator(_argv[0]); + ::testing::InitGoogleTest(&argc, argv); + if (argc > 0) { + search::index::DummyFileHeaderContext::setCreator(argv[0]); } - TEST_DO(requireThatDictionaryHandlesNoEntries(false, false)); - TEST_DO(requireThatDictionaryHandlesMultipleEntries(false, false)); - TEST_DO(requireThatDictionaryHandlesNoEntries(true, false)); - TEST_DO(requireThatDictionaryHandlesMultipleEntries(true, false)); - TEST_DO(requireThatDictionaryHandlesNoEntries(false, true)); - TEST_DO(requireThatDictionaryHandlesMultipleEntries(false, true)); - - TEST_DONE(); + return RUN_ALL_TESTS(); } - -} - -TEST_APPHOOK(search::diskindex::Test); diff --git a/searchlib/src/tests/fef/attributecontent/CMakeLists.txt b/searchlib/src/tests/fef/attributecontent/CMakeLists.txt index 48d8375dbb9..d9c88fb9eaf 100644 --- a/searchlib/src/tests/fef/attributecontent/CMakeLists.txt +++ b/searchlib/src/tests/fef/attributecontent/CMakeLists.txt @@ -4,5 +4,6 @@ vespa_add_executable(searchlib_attributecontent_test_app TEST attributecontent_test.cpp DEPENDS searchlib + GTest::gtest ) vespa_add_test(NAME searchlib_attributecontent_test_app COMMAND searchlib_attributecontent_test_app) diff --git a/searchlib/src/tests/fef/attributecontent/attributecontent_test.cpp b/searchlib/src/tests/fef/attributecontent/attributecontent_test.cpp index 1c75d47a134..d501d259ea5 100644 --- a/searchlib/src/tests/fef/attributecontent/attributecontent_test.cpp +++ b/searchlib/src/tests/fef/attributecontent/attributecontent_test.cpp @@ -1,34 +1,21 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/vespalib/testkit/testapp.h> #include <vespa/searchcommon/attribute/attributecontent.h> #include <vespa/searchcommon/attribute/config.h> #include <vespa/searchlib/attribute/attributefactory.h> #include <vespa/searchlib/attribute/integerbase.h> -#include <vespa/log/log.h> -LOG_SETUP("attributecontent_test"); +#include <vespa/vespalib/gtest/gtest.h> using namespace search::attribute; -namespace search { -namespace fef { +namespace search::fef { -class Test : public vespalib::TestApp { -private: - void testWriteAndRead(); - void testFill(); - -public: - int Main() override; -}; - -void -Test::testWriteAndRead() +TEST(AttributeContentTest, test_write_and_read) { using UintContent = search::attribute::AttributeContent<uint32_t>; UintContent buf; - EXPECT_EQUAL(buf.capacity(), 16u); - EXPECT_EQUAL(buf.size(), 0u); + EXPECT_EQ(buf.capacity(), 16u); + EXPECT_EQ(buf.size(), 0u); uint32_t i; uint32_t * data; @@ -37,34 +24,33 @@ Test::testWriteAndRead() *data = i; } buf.setSize(16); - EXPECT_EQUAL(buf.size(), 16u); + EXPECT_EQ(buf.size(), 16u); for (i = 0, itr = buf.begin(); itr != buf.end(); ++i, ++itr) { - EXPECT_EQUAL(*itr, i); - EXPECT_EQUAL(buf[i], i); + EXPECT_EQ(*itr, i); + EXPECT_EQ(buf[i], i); } - EXPECT_EQUAL(i, 16u); + EXPECT_EQ(i, 16u); buf.allocate(10); - EXPECT_EQUAL(buf.capacity(), 16u); - EXPECT_EQUAL(buf.size(), 16u); + EXPECT_EQ(buf.capacity(), 16u); + EXPECT_EQ(buf.size(), 16u); buf.allocate(32); - EXPECT_EQUAL(buf.capacity(), 32u); - EXPECT_EQUAL(buf.size(), 0u); + EXPECT_EQ(buf.capacity(), 32u); + EXPECT_EQ(buf.size(), 0u); for (i = 0, data = buf.data(); i < 32; ++i, ++data) { *data = i; } buf.setSize(32); - EXPECT_EQUAL(buf.size(), 32u); + EXPECT_EQ(buf.size(), 32u); for (i = 0, itr = buf.begin(); itr != buf.end(); ++i, ++itr) { - EXPECT_EQUAL(*itr, i); - EXPECT_EQUAL(buf[i], i); + EXPECT_EQ(*itr, i); + EXPECT_EQ(buf[i], i); } - EXPECT_EQUAL(i, 32u); + EXPECT_EQ(i, 32u); } -void -Test::testFill() +TEST(AttributeContentTest, test_fill) { Config cfg(BasicType::INT32, CollectionType::ARRAY); AttributeVector::SP av = AttributeFactory::createAttribute("aint32", cfg); @@ -77,29 +63,17 @@ Test::testFill() const IAttributeVector & iav = *av.get(); IntegerContent buf; buf.fill(iav, 0); - EXPECT_EQUAL(1u, buf.size()); - EXPECT_EQUAL(10, buf[0]); + EXPECT_EQ(1u, buf.size()); + EXPECT_EQ(10, buf[0]); buf.fill(iav, 1); - EXPECT_EQUAL(2u, buf.size()); - EXPECT_EQUAL(20, buf[0]); - EXPECT_EQUAL(30, buf[1]); + EXPECT_EQ(2u, buf.size()); + EXPECT_EQ(20, buf[0]); + EXPECT_EQ(30, buf[1]); buf.fill(iav, 0); - EXPECT_EQUAL(1u, buf.size()); - EXPECT_EQUAL(10, buf[0]); + EXPECT_EQ(1u, buf.size()); + EXPECT_EQ(10, buf[0]); } -int -Test::Main() -{ - TEST_INIT("attributecontent_test"); - - testWriteAndRead(); - testFill(); - - TEST_DONE(); } -} // namespace fef -} // namespace search - -TEST_APPHOOK(search::fef::Test); +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchlib/src/tests/fef/featurenameparser/CMakeLists.txt b/searchlib/src/tests/fef/featurenameparser/CMakeLists.txt index e7874558122..cb040472a6f 100644 --- a/searchlib/src/tests/fef/featurenameparser/CMakeLists.txt +++ b/searchlib/src/tests/fef/featurenameparser/CMakeLists.txt @@ -4,5 +4,6 @@ vespa_add_executable(searchlib_featurenameparser_test_app TEST featurenameparser_test.cpp DEPENDS searchlib + GTest::gtest ) vespa_add_test(NAME searchlib_featurenameparser_test_app COMMAND searchlib_featurenameparser_test_app) diff --git a/searchlib/src/tests/fef/featurenameparser/featurenameparser_test.cpp b/searchlib/src/tests/fef/featurenameparser/featurenameparser_test.cpp index 90a9135389a..ee22a49f8db 100644 --- a/searchlib/src/tests/fef/featurenameparser/featurenameparser_test.cpp +++ b/searchlib/src/tests/fef/featurenameparser/featurenameparser_test.cpp @@ -1,13 +1,15 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/log/log.h> -LOG_SETUP("featurenameparser_test"); -#include <vespa/vespalib/testkit/test_kit.h> -#include <vespa/vespalib/testkit/testapp.h> -#include <vespa/vespalib/util/size_literals.h> + #include <vespa/searchlib/fef/featurenameparser.h> +#include <vespa/vespalib/gtest/gtest.h> +#include <vespa/vespalib/testkit/test_path.h> +#include <vespa/vespalib/util/size_literals.h> #include <vector> #include <string> +#include <vespa/log/log.h> +LOG_SETUP("featurenameparser_test"); + using namespace search::fef; struct ParamList { @@ -31,20 +33,10 @@ std::ostream &operator<<(std::ostream &os, const ParamList &pl) { return os; } -class Test : public vespalib::TestApp -{ -public: - bool testParse(const vespalib::string &input, bool valid, - const vespalib::string &base, ParamList pl, - const vespalib::string &output); - void testFile(const vespalib::string &name); - int Main() override; -}; - bool -Test::testParse(const vespalib::string &input, bool valid, - const vespalib::string &base, ParamList pl, - const vespalib::string &output) +testParse(const vespalib::string &input, bool valid, + const vespalib::string &base, ParamList pl, + const vespalib::string &output) { bool ok = true; FeatureNameParser parser(input); @@ -52,15 +44,15 @@ Test::testParse(const vespalib::string &input, bool valid, LOG(warning, "parse error: input:'%s', rest:'%s'", input.c_str(), input.substr(parser.parsedBytes()).c_str()); } - ok &= EXPECT_EQUAL(parser.valid(), valid); - ok &= EXPECT_EQUAL(parser.baseName(), base); - ok &= EXPECT_EQUAL(ParamList(parser.parameters()), pl); - ok &= EXPECT_EQUAL(parser.output(), output); + EXPECT_EQ(parser.valid(), valid) << (ok = false, ""); + EXPECT_EQ(parser.baseName(), base) << (ok = false, ""); + EXPECT_EQ(ParamList(parser.parameters()), pl) << (ok = false, ""); + EXPECT_EQ(parser.output(), output) << (ok = false, ""); return ok; } void -Test::testFile(const vespalib::string &name) +testFile(const vespalib::string &name) { char buf[4_Ki]; uint32_t lineN = 0; @@ -76,13 +68,16 @@ Test::testFile(const vespalib::string &name) continue; } uint32_t idx = line.find("<=>"); - if (!EXPECT_TRUE(idx < line.size())) { + bool failed = false; + EXPECT_TRUE(idx < line.size()) << (failed = true, ""); + if (failed) { LOG(error, "(%s:%u): malformed line: '%s'", name.c_str(), lineN, line.c_str()); } else { vespalib::string input = line.substr(0, idx); vespalib::string expect = line.substr(idx + strlen("<=>")); - if (!EXPECT_EQUAL(FeatureNameParser(input).featureName(), expect)) { + EXPECT_EQ(FeatureNameParser(input).featureName(), expect) << (failed = true, ""); + if (failed) { LOG(error, "(%s:%u): test failed: '%s'", name.c_str(), lineN, line.c_str()); } @@ -92,42 +87,54 @@ Test::testFile(const vespalib::string &name) fclose(f); } -int -Test::Main() +TEST(FeatureNameParserTest, test_normal_cases) { - TEST_INIT("featurenameparser_test"); - // normal cases EXPECT_TRUE(testParse("foo", true, "foo", ParamList(), "")); EXPECT_TRUE(testParse("foo.out", true, "foo", ParamList(), "out")); EXPECT_TRUE(testParse("foo(a)", true, "foo", ParamList().add("a"), "")); EXPECT_TRUE(testParse("foo(a,b)", true, "foo", ParamList().add("a").add("b"), "")); EXPECT_TRUE(testParse("foo(a,b).out", true, "foo", ParamList().add("a").add("b"), "out")); +} +TEST(FeatureNameParserTest, test_0_in_feature_name) +{ // @ in feature name (for macros) EXPECT_TRUE(testParse("foo@", true, "foo@", ParamList(), "")); EXPECT_TRUE(testParse("foo@.out", true, "foo@", ParamList(), "out")); EXPECT_TRUE(testParse("foo@(a)", true, "foo@", ParamList().add("a"), "")); EXPECT_TRUE(testParse("foo@(a,b)", true, "foo@", ParamList().add("a").add("b"), "")); EXPECT_TRUE(testParse("foo@(a,b).out", true, "foo@", ParamList().add("a").add("b"), "out")); +} +TEST(FeatureNameParserTest, test_dollar_in_feature_name) +{ // $ in feature name (for macros) EXPECT_TRUE(testParse("foo$", true, "foo$", ParamList(), "")); EXPECT_TRUE(testParse("foo$.out", true, "foo$", ParamList(), "out")); EXPECT_TRUE(testParse("foo$(a)", true, "foo$", ParamList().add("a"), "")); EXPECT_TRUE(testParse("foo$(a,b)", true, "foo$", ParamList().add("a").add("b"), "")); EXPECT_TRUE(testParse("foo$(a,b).out", true, "foo$", ParamList().add("a").add("b"), "out")); +} +TEST(FeatureNameParserTest, test_de_quoting_of_parameters) +{ // de-quoting of parameters EXPECT_TRUE(testParse("foo(a,\"b\")", true, "foo", ParamList().add("a").add("b"), "")); EXPECT_TRUE(testParse("foo(a,\" b \")", true, "foo", ParamList().add("a").add(" b "), "")); EXPECT_TRUE(testParse("foo( \"a\" , \" b \" )", true, "foo", ParamList().add("a").add(" b "), "")); EXPECT_TRUE(testParse("foo(\"\\\"\\\\\\t\\n\\r\\f\\x20\")", true, "foo", ParamList().add("\"\\\t\n\r\f "), "")); +} +TEST(FeatureNameParserTest, test_no_default_output_when_ending_with_dot) +{ // only default output if '.' not specified EXPECT_TRUE(testParse("foo.", false, "", ParamList(), "")); EXPECT_TRUE(testParse("foo(a,b).", false, "", ParamList(), "")); +} +TEST(FeatureNameParserTest, test_string_cannot_end_in_parmeter_list) +{ // string cannot end in parameter list EXPECT_TRUE(testParse("foo(", false, "", ParamList(), "")); EXPECT_TRUE(testParse("foo(a", false, "", ParamList(), "")); @@ -135,7 +142,10 @@ Test::Main() EXPECT_TRUE(testParse("foo(a\\)", false, "", ParamList(), "")); EXPECT_TRUE(testParse("foo(a,", false, "", ParamList(), "")); EXPECT_TRUE(testParse("foo(a,b", false, "", ParamList(), "")); +} +TEST(FeatureNameParserTest, test_empty_parameters) +{ // empty parameters EXPECT_TRUE(testParse("foo()", true, "foo", ParamList().add(""), "")); EXPECT_TRUE(testParse("foo(,)", true, "foo", ParamList().add("").add(""), "")); @@ -144,9 +154,11 @@ Test::Main() EXPECT_TRUE(testParse("foo( )", true, "foo", ParamList().add(""), "")); EXPECT_TRUE(testParse("foo( , , )", true, "foo", ParamList().add("").add("").add(""), "")); EXPECT_TRUE(testParse("foo( \t , \n , \r , \f )", true, "foo", ParamList().add("").add("").add("").add(""), "")); +} +TEST(FeatureNameParserTest, test_cases_from_file) +{ testFile(TEST_PATH("parsetest.txt")); - TEST_DONE(); } -TEST_APPHOOK(Test); +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchlib/src/tests/fef/parameter/CMakeLists.txt b/searchlib/src/tests/fef/parameter/CMakeLists.txt index da847e061f9..b238e09c98b 100644 --- a/searchlib/src/tests/fef/parameter/CMakeLists.txt +++ b/searchlib/src/tests/fef/parameter/CMakeLists.txt @@ -4,5 +4,6 @@ vespa_add_executable(searchlib_parameter_test_app TEST parameter_test.cpp DEPENDS searchlib + GTest::gtest ) vespa_add_test(NAME searchlib_parameter_test_app NO_VALGRIND COMMAND searchlib_parameter_test_app) diff --git a/searchlib/src/tests/fef/parameter/parameter_test.cpp b/searchlib/src/tests/fef/parameter/parameter_test.cpp index fa29f16f1d5..a8acd2ea34d 100644 --- a/searchlib/src/tests/fef/parameter/parameter_test.cpp +++ b/searchlib/src/tests/fef/parameter/parameter_test.cpp @@ -1,32 +1,34 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/log/log.h> -LOG_SETUP("parameter_test"); -#include <vespa/vespalib/testkit/testapp.h> #include <vespa/searchlib/fef/parametervalidator.h> #include <vespa/searchlib/fef/test/indexenvironment.h> #include <vespa/searchlib/fef/test/indexenvironmentbuilder.h> +#include <vespa/vespalib/gtest/gtest.h> + +#include <vespa/log/log.h> +LOG_SETUP("parameter_test"); using namespace search::fef::test; using CollectionType = search::fef::FieldInfo::CollectionType; using DataType = search::fef::FieldInfo::DataType; -namespace search { -namespace fef { +namespace search::fef { class StringList : public std::vector<vespalib::string> { public: StringList & add(const vespalib::string & str) { push_back(str); return *this; } }; -class ParameterTest : public vespalib::TestApp { -private: +class ParameterTest : public ::testing::Test { +protected: using PDS = ParameterDescriptions; using PT = ParameterType; using P = Parameter; using SL = StringList; using PVR = ParameterValidator::Result; + ParameterTest(); + ~ParameterTest() override; bool assertParameter(const Parameter & exp, const Parameter & act); bool validate(const IIndexEnvironment & env, const std::vector<vespalib::string> & params, @@ -35,24 +37,20 @@ private: const std::vector<vespalib::string> & params, const ParameterDescriptions & descs, const ParameterValidator::Result & result); - - void testDescriptions(); - void testValidator(); - void testParameters(); - -public: - int Main() override; }; +ParameterTest::ParameterTest() = default; +ParameterTest::~ParameterTest() = default; + bool ParameterTest::assertParameter(const Parameter & exp, const Parameter & act) { bool retval = true; - if (!EXPECT_EQUAL(exp.getType(), act.getType())) retval = false; - if (!EXPECT_EQUAL(exp.getValue(), act.getValue())) retval = false; - if (!EXPECT_EQUAL(exp.asDouble(), act.asDouble())) retval = false; - if (!EXPECT_EQUAL(exp.asInteger(), act.asInteger())) retval = false; - if (!EXPECT_EQUAL(exp.asField(), act.asField())) retval = false; + EXPECT_EQ(exp.getType(), act.getType()) << (retval = false, ""); + EXPECT_EQ(exp.getValue(), act.getValue()) << (retval = false, ""); + EXPECT_EQ(exp.asDouble(), act.asDouble()) << (retval = false, ""); + EXPECT_EQ(exp.asInteger(), act.asInteger()) << (retval = false, ""); + EXPECT_EQ(exp.asField(), act.asField()) << (retval = false, ""); return retval; } @@ -76,8 +74,15 @@ ParameterTest::validate(const IIndexEnvironment & env, if (!validate(env, params, descs)) return false; ParameterValidator pv(env, params, descs); ParameterValidator::Result actual = pv.validate(); - if (!EXPECT_EQUAL(result.getTag(), actual.getTag())) return false; - if (!EXPECT_EQUAL(result.getParameters().size(), actual.getParameters().size())) return false; + bool failed = false; + EXPECT_EQ(result.getTag(), actual.getTag()) << (failed = true, ""); + if (failed) { + return false; + } + EXPECT_EQ(result.getParameters().size(), actual.getParameters().size()) << (failed = true, ""); + if (failed) { + return false; + } bool retval = true; for (size_t i = 0; i < result.getParameters().size(); ++i) { if (!assertParameter(result.getParameters()[i], actual.getParameters()[i])) retval = false; @@ -85,52 +90,50 @@ ParameterTest::validate(const IIndexEnvironment & env, return retval; } -void -ParameterTest::testDescriptions() +TEST_F(ParameterTest, test_descriptions) { PDS descs = PDS(). desc().indexField(ParameterCollection::SINGLE).indexField(ParameterCollection::ARRAY).indexField(ParameterCollection::WEIGHTEDSET).attribute(ParameterCollection::ANY).attributeField(ParameterCollection::ANY).field(). desc(5).feature().number().string().attribute(ParameterCollection::ANY). desc().string().number().repeat(2); const PDS::DescriptionVector & v = descs.getDescriptions(); - EXPECT_EQUAL(v.size(), 3u); - EXPECT_EQUAL(v[0].getTag(), 0u); + EXPECT_EQ(v.size(), 3u); + EXPECT_EQ(v[0].getTag(), 0u); EXPECT_TRUE(!v[0].hasRepeat()); - EXPECT_EQUAL(v[0].getParams().size(), 6u); - EXPECT_EQUAL(v[0].getParam(0).type, ParameterType::INDEX_FIELD); - EXPECT_EQUAL(v[0].getParam(1).type, ParameterType::INDEX_FIELD); - EXPECT_EQUAL(v[0].getParam(2).type, ParameterType::INDEX_FIELD); - EXPECT_EQUAL(v[0].getParam(3).type, ParameterType::ATTRIBUTE); - EXPECT_EQUAL(v[0].getParam(4).type, ParameterType::ATTRIBUTE_FIELD); - EXPECT_EQUAL(v[0].getParam(5).type, ParameterType::FIELD); - EXPECT_EQUAL(v[0].getParam(0).collection, ParameterCollection::SINGLE); - EXPECT_EQUAL(v[0].getParam(1).collection, ParameterCollection::ARRAY); - EXPECT_EQUAL(v[0].getParam(2).collection, ParameterCollection::WEIGHTEDSET); - EXPECT_EQUAL(v[0].getParam(3).collection, ParameterCollection::ANY); - EXPECT_EQUAL(v[0].getParam(4).collection, ParameterCollection::ANY); - EXPECT_EQUAL(v[0].getParam(5).collection, ParameterCollection::ANY); + EXPECT_EQ(v[0].getParams().size(), 6u); + EXPECT_EQ(v[0].getParam(0).type, ParameterType::INDEX_FIELD); + EXPECT_EQ(v[0].getParam(1).type, ParameterType::INDEX_FIELD); + EXPECT_EQ(v[0].getParam(2).type, ParameterType::INDEX_FIELD); + EXPECT_EQ(v[0].getParam(3).type, ParameterType::ATTRIBUTE); + EXPECT_EQ(v[0].getParam(4).type, ParameterType::ATTRIBUTE_FIELD); + EXPECT_EQ(v[0].getParam(5).type, ParameterType::FIELD); + EXPECT_EQ(v[0].getParam(0).collection, ParameterCollection::SINGLE); + EXPECT_EQ(v[0].getParam(1).collection, ParameterCollection::ARRAY); + EXPECT_EQ(v[0].getParam(2).collection, ParameterCollection::WEIGHTEDSET); + EXPECT_EQ(v[0].getParam(3).collection, ParameterCollection::ANY); + EXPECT_EQ(v[0].getParam(4).collection, ParameterCollection::ANY); + EXPECT_EQ(v[0].getParam(5).collection, ParameterCollection::ANY); - EXPECT_EQUAL(v[1].getTag(), 5u); + EXPECT_EQ(v[1].getTag(), 5u); EXPECT_TRUE(!v[1].hasRepeat()); - EXPECT_EQUAL(v[1].getParams().size(), 4u); - EXPECT_EQUAL(v[1].getParam(0).type, ParameterType::FEATURE); - EXPECT_EQUAL(v[1].getParam(1).type, ParameterType::NUMBER); - EXPECT_EQUAL(v[1].getParam(2).type, ParameterType::STRING); - EXPECT_EQUAL(v[1].getParam(3).type, ParameterType::ATTRIBUTE); + EXPECT_EQ(v[1].getParams().size(), 4u); + EXPECT_EQ(v[1].getParam(0).type, ParameterType::FEATURE); + EXPECT_EQ(v[1].getParam(1).type, ParameterType::NUMBER); + EXPECT_EQ(v[1].getParam(2).type, ParameterType::STRING); + EXPECT_EQ(v[1].getParam(3).type, ParameterType::ATTRIBUTE); - EXPECT_EQUAL(v[2].getTag(), 6u); + EXPECT_EQ(v[2].getTag(), 6u); EXPECT_TRUE(v[2].hasRepeat()); - EXPECT_EQUAL(v[2].getParams().size(), 2u); - EXPECT_EQUAL(v[2].getParam(0).type, ParameterType::STRING); - EXPECT_EQUAL(v[2].getParam(1).type, ParameterType::NUMBER); - EXPECT_EQUAL(v[2].getParam(2).type, ParameterType::STRING); - EXPECT_EQUAL(v[2].getParam(3).type, ParameterType::NUMBER); - EXPECT_EQUAL(v[2].getParam(4).type, ParameterType::STRING); - EXPECT_EQUAL(v[2].getParam(5).type, ParameterType::NUMBER); + EXPECT_EQ(v[2].getParams().size(), 2u); + EXPECT_EQ(v[2].getParam(0).type, ParameterType::STRING); + EXPECT_EQ(v[2].getParam(1).type, ParameterType::NUMBER); + EXPECT_EQ(v[2].getParam(2).type, ParameterType::STRING); + EXPECT_EQ(v[2].getParam(3).type, ParameterType::NUMBER); + EXPECT_EQ(v[2].getParam(4).type, ParameterType::STRING); + EXPECT_EQ(v[2].getParam(5).type, ParameterType::NUMBER); } -void -ParameterTest::testValidator() +TEST_F(ParameterTest, test_validator) { IndexEnvironment env; IndexEnvironmentBuilder builder(env); @@ -201,8 +204,7 @@ ParameterTest::testValidator() EXPECT_TRUE(!validate(env, SL().add("str").add("bar").add("foo").add("bar"), d2)); } -void -ParameterTest::testParameters() +TEST_F(ParameterTest, test_parameters) { IndexEnvironment env; IndexEnvironmentBuilder builder(env); @@ -254,20 +256,6 @@ ParameterTest::testParameters() PVR(20).addParameter(P(PT::STRING, "baz")))); // second desc matching } -int -ParameterTest::Main() -{ - TEST_INIT("parameter_test"); - - testDescriptions(); - testValidator(); - testParameters(); - - TEST_DONE(); } -} -} - -TEST_APPHOOK(search::fef::ParameterTest); - +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchlib/src/tests/fef/phrasesplitter/CMakeLists.txt b/searchlib/src/tests/fef/phrasesplitter/CMakeLists.txt index 95f6eaad705..8f3946d110c 100644 --- a/searchlib/src/tests/fef/phrasesplitter/CMakeLists.txt +++ b/searchlib/src/tests/fef/phrasesplitter/CMakeLists.txt @@ -4,6 +4,7 @@ vespa_add_executable(searchlib_phrasesplitter_test_app TEST phrasesplitter_test.cpp DEPENDS searchlib + GTest::gtest ) vespa_add_test(NAME searchlib_phrasesplitter_test_app COMMAND searchlib_phrasesplitter_test_app) vespa_add_executable(searchlib_benchmark_app diff --git a/searchlib/src/tests/fef/phrasesplitter/phrasesplitter_test.cpp b/searchlib/src/tests/fef/phrasesplitter/phrasesplitter_test.cpp index c4767c571b9..f4b3c878a06 100644 --- a/searchlib/src/tests/fef/phrasesplitter/phrasesplitter_test.cpp +++ b/searchlib/src/tests/fef/phrasesplitter/phrasesplitter_test.cpp @@ -1,41 +1,25 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/log/log.h> -LOG_SETUP("phrasesplitter_test"); -#include <vespa/vespalib/testkit/testapp.h> #include <vespa/searchlib/fef/matchdatalayout.h> #include <vespa/searchlib/fef/phrasesplitter.h> #include <vespa/searchlib/fef/phrase_splitter_query_env.h> #include <vespa/searchlib/fef/test/queryenvironment.h> +#include <vespa/vespalib/gtest/gtest.h> -namespace search { -namespace fef { - -class PhraseSplitterTest : public vespalib::TestApp -{ -private: - void assertTermData(const ITermData * td, uint32_t uniqueId, uint32_t numTerms, - uint32_t fieldId, uint32_t termHandle); - void testCopyTermFieldMatchData(); - void testSplitter(); - void testSplitterUpdate(); - -public: - int Main() override; -}; +namespace search::fef { void -PhraseSplitterTest::assertTermData(const ITermData *td, uint32_t uniqueId, uint32_t numTerms, - uint32_t fieldId, uint32_t tfHandle) +assertTermData(const ITermData *td, uint32_t uniqueId, uint32_t numTerms, + uint32_t fieldId, uint32_t tfHandle, const vespalib::string& label) { + SCOPED_TRACE(label); // fprintf(stderr, "checking uid=%d numterms=%d field=%d handle=%d\n", uniqueId, numTerms, fieldId, tfHandle); - EXPECT_EQUAL(uniqueId, td->getUniqueId()); - EXPECT_EQUAL(numTerms, td->getPhraseLength()); - EXPECT_EQUAL(tfHandle, td->lookupField(fieldId)->getHandle()); + EXPECT_EQ(uniqueId, td->getUniqueId()); + EXPECT_EQ(numTerms, td->getPhraseLength()); + EXPECT_EQ(tfHandle, td->lookupField(fieldId)->getHandle()); } -void -PhraseSplitterTest::testCopyTermFieldMatchData() +TEST(PhraseSplitterTest, test_copy_term_field_match_data) { TermFieldMatchData src; src.reset(1); @@ -49,34 +33,33 @@ PhraseSplitterTest::testCopyTermFieldMatchData() dst.appendPosition(TermFieldMatchDataPosition(0, 10, 0, 1000)); { FieldPositionsIterator itr = dst.getIterator(); - EXPECT_EQUAL(itr.getPosition(), 10u); + EXPECT_EQ(itr.getPosition(), 10u); itr.next(); ASSERT_TRUE(!itr.valid()); } PhraseSplitter::copyTermFieldMatchData(dst, src, 2); - EXPECT_EQUAL(dst.getDocId(), 1u); + EXPECT_EQ(dst.getDocId(), 1u); { TermFieldMatchData::PositionsIterator itr = dst.begin(); - EXPECT_EQUAL(itr->getPosition(), 7u); + EXPECT_EQ(itr->getPosition(), 7u); ++itr; - EXPECT_EQUAL(itr->getPosition(), 17u); + EXPECT_EQ(itr->getPosition(), 17u); ++itr; ASSERT_TRUE(itr == dst.end()); } { FieldPositionsIterator itr = dst.getIterator(); - EXPECT_EQUAL(itr.getPosition(), 7u); + EXPECT_EQ(itr.getPosition(), 7u); itr.next(); - EXPECT_EQUAL(itr.getPosition(), 17u); + EXPECT_EQ(itr.getPosition(), 17u); itr.next(); ASSERT_TRUE(!itr.valid()); } } -void -PhraseSplitterTest::testSplitter() +TEST(PhraseSplitterTest, test_splitter) { { // single term test::QueryEnvironment qe; @@ -91,9 +74,9 @@ PhraseSplitterTest::testSplitter() ps.bind_match_data(*md); ps.update(); // check that nothing is served from the splitter - EXPECT_EQUAL(ps.get_query_env().getTerm(0), &terms[0]); + EXPECT_EQ(ps.get_query_env().getTerm(0), &terms[0]); TermFieldHandle handle = terms[0].lookupField(0)->getHandle(); - EXPECT_EQUAL(ps.resolveTermField(handle), md->resolveTermField(handle)); + EXPECT_EQ(ps.resolveTermField(handle), md->resolveTermField(handle)); } { // single phrase test::QueryEnvironment qe; @@ -114,14 +97,14 @@ PhraseSplitterTest::testSplitter() for (size_t i = 0; i < 3; ++i) { // fprintf(stderr, "checking term %d\n", (int)i); const ITermData *td = ps.get_query_env().getTerm(i); - EXPECT_NOT_EQUAL(td, &terms[0]); - EXPECT_NOT_EQUAL(td->lookupField(7), (ITermFieldData *)0); - EXPECT_EQUAL(td->lookupField(0), (ITermFieldData *)0); - TEST_DO(assertTermData(td, 1, 1, 7, i + 4)); // skipHandles = 4 - EXPECT_NOT_EQUAL(td->lookupField(7)->getHandle(), - terms[0].lookupField(7)->getHandle()); - EXPECT_NOT_EQUAL(ps.resolveTermField(td->lookupField(7)->getHandle()), - md->resolveTermField(terms[0].lookupField(7)->getHandle())); + EXPECT_NE(td, &terms[0]); + EXPECT_NE(td->lookupField(7), (ITermFieldData *)0); + EXPECT_EQ(td->lookupField(0), (ITermFieldData *)0); + assertTermData(td, 1, 1, 7, i + 4, "single phrase"); // skipHandles = 4 + EXPECT_NE(td->lookupField(7)->getHandle(), + terms[0].lookupField(7)->getHandle()); + EXPECT_NE(ps.resolveTermField(td->lookupField(7)->getHandle()), + md->resolveTermField(terms[0].lookupField(7)->getHandle())); } } { // combination @@ -145,40 +128,39 @@ PhraseSplitterTest::testSplitter() ps.update(); { // first term // fprintf(stderr, "first term\n"); - EXPECT_EQUAL(ps.get_query_env().getTerm(0), &terms[0]); - TEST_DO(assertTermData(ps.get_query_env().getTerm(0), 0, 1, 4, 0)); - TEST_DO(assertTermData(ps.get_query_env().getTerm(0), 0, 1, 7, 1)); + EXPECT_EQ(ps.get_query_env().getTerm(0), &terms[0]); + assertTermData(ps.get_query_env().getTerm(0), 0, 1, 4, 0, "first term 1"); + assertTermData(ps.get_query_env().getTerm(0), 0, 1, 7, 1, "first term 2"); TermFieldHandle handle = terms[0].lookupField(4)->getHandle(); - EXPECT_EQUAL(ps.resolveTermField(handle), md->resolveTermField(handle)); + EXPECT_EQ(ps.resolveTermField(handle), md->resolveTermField(handle)); handle = terms[0].lookupField(7)->getHandle(); - EXPECT_EQUAL(ps.resolveTermField(handle), md->resolveTermField(handle)); + EXPECT_EQ(ps.resolveTermField(handle), md->resolveTermField(handle)); } for (size_t i = 0; i < 3; ++i) { // phrase // fprintf(stderr, "phrase term %zd\n", i); const ITermData *td = ps.get_query_env().getTerm(i + 1); - EXPECT_NOT_EQUAL(td, &terms[1]); - TEST_DO(assertTermData(td, 1, 1, 4, i + 11)); // skipHandles == 11 - EXPECT_EQUAL(td->lookupField(7), (ITermFieldData *)0); - EXPECT_NOT_EQUAL(ps.resolveTermField(td->lookupField(4)->getHandle()), - md->resolveTermField(terms[1].lookupField(4)->getHandle())); + EXPECT_NE(td, &terms[1]); + assertTermData(td, 1, 1, 4, i + 11, "phrase term"); // skipHandles == 11 + EXPECT_EQ(td->lookupField(7), (ITermFieldData *)0); + EXPECT_NE(ps.resolveTermField(td->lookupField(4)->getHandle()), + md->resolveTermField(terms[1].lookupField(4)->getHandle())); } { // last term // fprintf(stderr, "last term\n"); - EXPECT_EQUAL(ps.get_query_env().getTerm(4), &terms[2]); - TEST_DO(assertTermData(ps.get_query_env().getTerm(4), 2, 1, 4, 4)); - TEST_DO(assertTermData(ps.get_query_env().getTerm(4), 2, 1, 7, 5)); + EXPECT_EQ(ps.get_query_env().getTerm(4), &terms[2]); + assertTermData(ps.get_query_env().getTerm(4), 2, 1, 4, 4, "last term 1"); + assertTermData(ps.get_query_env().getTerm(4), 2, 1, 7, 5, "last term 2"); // fprintf(stderr, "inspect term %p #f %zd\n", &terms[2], terms[2].numFields()); fflush(stderr); TermFieldHandle handle = terms[2].lookupField(4)->getHandle(); - EXPECT_EQUAL(ps.resolveTermField(handle), md->resolveTermField(handle)); + EXPECT_EQ(ps.resolveTermField(handle), md->resolveTermField(handle)); } } } -void -PhraseSplitterTest::testSplitterUpdate() +TEST(PhraseSplitterTest, test_splitter_update) { { test::QueryEnvironment qe; @@ -213,38 +195,24 @@ PhraseSplitterTest::testSplitterUpdate() for (size_t i = 0; i < 2; ++i) { // first phrase const TermFieldMatchData * tmd = ps.resolveTermField(ps.get_query_env().getTerm(i)->lookupField(0)->getHandle()); TermFieldMatchData::PositionsIterator itr = tmd->begin(); - EXPECT_EQUAL((itr++)->getPosition(), 10 + i); + EXPECT_EQ((itr++)->getPosition(), 10 + i); ASSERT_TRUE(itr == tmd->end()); } { // first term TermFieldMatchData * tmd = md->resolveTermField(ps.get_query_env().getTerm(2)->lookupField(0)->getHandle()); TermFieldMatchData::PositionsIterator itr = tmd->begin(); - EXPECT_EQUAL((itr++)->getPosition(), 20u); + EXPECT_EQ((itr++)->getPosition(), 20u); ASSERT_TRUE(itr == tmd->end()); } for (size_t i = 0; i < 2; ++i) { // second phrase const TermFieldMatchData * tmd = ps.resolveTermField(ps.get_query_env().getTerm(i + 3)->lookupField(0)->getHandle()); TermFieldMatchData::PositionsIterator itr = tmd->begin(); - EXPECT_EQUAL((itr++)->getPosition(), 30 + i); + EXPECT_EQ((itr++)->getPosition(), 30 + i); ASSERT_TRUE(itr == tmd->end()); } } } -int -PhraseSplitterTest::Main() -{ - - TEST_INIT("phrasesplitter_test"); - - testCopyTermFieldMatchData(); - testSplitter(); - testSplitterUpdate(); - - TEST_DONE(); -} - -} } -TEST_APPHOOK(search::fef::PhraseSplitterTest); +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/vespajlib/src/main/java/ai/vespa/http/DomainName.java b/vespajlib/src/main/java/ai/vespa/http/DomainName.java index fa6964002bc..51ae9c12036 100644 --- a/vespajlib/src/main/java/ai/vespa/http/DomainName.java +++ b/vespajlib/src/main/java/ai/vespa/http/DomainName.java @@ -32,4 +32,9 @@ public class DomainName extends PatternedStringWrapper<DomainName> { return requireMatch(label, "domain name label", labelPattern); } + public String leafLabel() { + int offset = value().lastIndexOf('.'); + return offset == -1 ? value() : value().substring(0, offset); + } + } diff --git a/vespajlib/src/test/java/ai/vespa/http/DomainNameTest.java b/vespajlib/src/test/java/ai/vespa/http/DomainNameTest.java index ed7650d2929..f3c9b70b7db 100644 --- a/vespajlib/src/test/java/ai/vespa/http/DomainNameTest.java +++ b/vespajlib/src/test/java/ai/vespa/http/DomainNameTest.java @@ -3,6 +3,7 @@ package ai.vespa.http; import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; /** @@ -30,6 +31,9 @@ class DomainNameTest { assertThrows(IllegalArgumentException.class, () -> DomainName.of("foo%")); assertThrows(IllegalArgumentException.class, () -> DomainName.of(("." + "a".repeat(32)).repeat(8).substring(1, 257))); assertThrows(IllegalArgumentException.class, () -> DomainName.of("a".repeat(64))); + + assertEquals("foo", DomainName.of("foo").leafLabel()); + assertEquals("foo", DomainName.of("foo.com").leafLabel()); } } |