Merge pull request #9742 from vespa-engine/bratseth/validate-query-profiles

Holistic query profile validation

8 files changed, 114 insertions, 13 deletions

diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java b/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java
index 937d511bb09..c19865fafc9 100644
--- a/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java
+++ b/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java
@@ -341,7 +341,7 @@ public class DeployState implements ConfigDefinitionStore {
 
         public DeployState build(ValidationParameters validationParameters) {
             RankProfileRegistry rankProfileRegistry = new RankProfileRegistry();
-            QueryProfiles queryProfiles = new QueryProfilesBuilder().build(applicationPackage);
+            QueryProfiles queryProfiles = new QueryProfilesBuilder().build(applicationPackage, logger);
             SemanticRules semanticRules = new SemanticRuleBuilder().build(applicationPackage);
             SearchDocumentModel searchDocumentModel = createSearchDocumentModel(rankProfileRegistry, logger, queryProfiles, validationParameters);
             return new DeployState(applicationPackage,
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java b/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java
index bd4daa58253..9804b0b6329 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java
@@ -246,7 +246,7 @@ public class SearchBuilder {
         DocumentModelBuilder builder = new DocumentModelBuilder(model);
         for (Search search : new SearchOrderer().order(searchList)) {
             new FieldOperationApplierForSearch().process(search); // TODO: Why is this not in the regular list?
-            process(search, deployLogger, new QueryProfiles(queryProfileRegistry), validate);
+            process(search, deployLogger, new QueryProfiles(queryProfileRegistry, deployLogger), validate);
             built.add(search);
         }
         builder.addToModel(searchList);
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/search/QueryProfiles.java b/config-model/src/main/java/com/yahoo/vespa/model/container/search/QueryProfiles.java
index 526b2abe1e1..c7114178ad6 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/search/QueryProfiles.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/search/QueryProfiles.java
@@ -1,6 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.model.container.search;
 
+import com.yahoo.config.application.api.DeployLogger;
 import com.yahoo.search.query.profile.BackedOverridableQueryProfile;
 import com.yahoo.search.query.profile.QueryProfile;
 import com.yahoo.search.query.profile.QueryProfileRegistry;
@@ -12,6 +13,8 @@ import com.yahoo.search.query.profile.config.QueryProfilesConfig;
 import java.io.Serializable;
 import java.util.*;
 import java.util.Map.Entry;
+import java.util.logging.Level;
+import java.util.stream.Collectors;
 
 /**
  * Owns the query profiles and query profile types to be handed to the qrs nodes.
@@ -29,8 +32,9 @@ public class QueryProfiles implements Serializable, QueryProfilesConfig.Producer
      * @param registry the registry containing the query profiles and types of this.
      *        The given registry cannot be frozen on calling this.
      */
-    public QueryProfiles(QueryProfileRegistry registry) {
+    public QueryProfiles(QueryProfileRegistry registry, DeployLogger logger) {
         this.registry = registry;
+        validate(registry, logger);
     }
 
     public QueryProfiles() {
@@ -41,6 +45,28 @@ public class QueryProfiles implements Serializable, QueryProfilesConfig.Producer
         return registry;
     }
 
+    /** Emits warnings/hints on some common configuration errors */
+    private void validate(QueryProfileRegistry registry, DeployLogger logger) {
+        Set<String> tensorFields = new HashSet<>();
+        for (QueryProfileType type : registry.getTypeRegistry().allComponents()) {
+            for (var fieldEntry : type.fields().entrySet()) {
+                if (fieldEntry.getValue().getType().asTensorType().rank() > 0)
+                    tensorFields.add(fieldEntry.getKey());
+            }
+        }
+
+        if ( registry.getTypeRegistry().hasApplicationTypes() && registry.allComponents().isEmpty()) {
+            logger.log(Level.WARNING, "This application define query profile types, but has " +
+                                      "no query profiles referencing them so they have no effect. "  +
+                                      (tensorFields.isEmpty()
+                                       ? ""
+                                       : "In particular, the tensors (" + String.join(", ", tensorFields) +
+                                         ") will be interpreted as strings, not tensors if sent in requests. ") +
+                                      "See https://docs.vespa.ai/documentation/query-profiles.html");
+        }
+
+    }
+
     @Override
     public void getConfig(QueryProfilesConfig.Builder builder) {
         for (QueryProfile profile : registry.allComponents()) {
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/search/QueryProfilesBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/search/QueryProfilesBuilder.java
index b85cb88bf2e..b832c1bbdcd 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/search/QueryProfilesBuilder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/search/QueryProfilesBuilder.java
@@ -1,6 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.model.container.search;
 
+import com.yahoo.config.application.api.DeployLogger;
 import com.yahoo.io.reader.NamedReader;
 import com.yahoo.search.query.profile.config.QueryProfileXMLReader;
 import com.yahoo.config.application.api.ApplicationPackage;
@@ -17,13 +18,14 @@ import java.util.List;
 public class QueryProfilesBuilder {
 
     /** Build the set of query profiles for an application package */
-    public QueryProfiles build(ApplicationPackage applicationPackage) {
+    public QueryProfiles build(ApplicationPackage applicationPackage, DeployLogger logger) {
         List<NamedReader> queryProfileTypeFiles = null;
         List<NamedReader> queryProfileFiles = null;
         try {
             queryProfileTypeFiles = applicationPackage.getQueryProfileTypeFiles();
             queryProfileFiles = applicationPackage.getQueryProfileFiles();
-            return new QueryProfiles(new QueryProfileXMLReader().read(queryProfileTypeFiles, queryProfileFiles));
+            return new QueryProfiles(new QueryProfileXMLReader().read(queryProfileTypeFiles, queryProfileFiles),
+                                     logger);
         }
         finally {
             NamedReader.closeAll(queryProfileTypeFiles);
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/QueryProfileVariantsTestCase.java b/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/QueryProfileVariantsTestCase.java
index 5833bc79ebf..5e559b64bd1 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/QueryProfileVariantsTestCase.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/QueryProfileVariantsTestCase.java
@@ -1,6 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.model.container.search.test;
 
+import com.yahoo.config.application.api.DeployLogger;
 import com.yahoo.search.query.profile.QueryProfile;
 import com.yahoo.search.query.profile.QueryProfileRegistry;
 import com.yahoo.search.query.profile.config.QueryProfileXMLReader;
@@ -8,6 +9,7 @@ import com.yahoo.vespa.model.container.search.QueryProfiles;
 import org.junit.Test;
 
 import java.io.IOException;
+import java.util.logging.Level;
 
 import static helpers.CompareConfigTestHelper.assertSerializedConfigFileEquals;
 import static org.junit.Assert.assertEquals;
@@ -22,28 +24,28 @@ public class QueryProfileVariantsTestCase {
     @Test
     public void testConfigCreation() throws IOException {
         QueryProfileRegistry registry = new QueryProfileXMLReader().read(root + "queryprofilevariants");
-        QueryProfiles profiles = new QueryProfiles(registry);
+        QueryProfiles profiles = new QueryProfiles(registry, new SilentDeployLogger());
         assertSerializedConfigFileEquals(root + "query-profile-variants-configuration.cfg", profiles.getConfig().toString());
     }
 
     @Test
     public void testConfigCreation2() throws IOException {
         QueryProfileRegistry registry = new QueryProfileXMLReader().read("src/test/java/com/yahoo/vespa/model/container/search/test/queryprofilevariants2");
-        QueryProfiles profiles = new QueryProfiles(registry);
+        QueryProfiles profiles = new QueryProfiles(registry, new SilentDeployLogger());
         assertSerializedConfigFileEquals(root + "query-profile-variants2-configuration.cfg", profiles.getConfig().toString());
     }
 
     @Test
     public void testConfigCreationNewsBESimple() throws IOException {
         QueryProfileRegistry registry = new QueryProfileXMLReader().read(root + "newsbesimple");
-        QueryProfiles profiles = new QueryProfiles(registry);
+        QueryProfiles profiles = new QueryProfiles(registry, new SilentDeployLogger());
         assertSerializedConfigFileEquals(root + "newsbe-query-profiles-simple.cfg", profiles.getConfig().toString());
     }
 
     @Test
     public void testConfigCreationNewsFESimple() throws IOException {
         QueryProfileRegistry registry = new QueryProfileXMLReader().read(root + "newsfesimple");
-        QueryProfiles profiles = new QueryProfiles(registry);
+        QueryProfiles profiles = new QueryProfiles(registry, new SilentDeployLogger());
         assertSerializedConfigFileEquals(root + "newsfe-query-profiles-simple.cfg", profiles.getConfig().toString());
     }
 
@@ -63,7 +65,7 @@ public class QueryProfileVariantsTestCase {
         registry.register(a1);
         registry.register(profile);
 
-        QueryProfiles profiles = new QueryProfiles(registry);
+        QueryProfiles profiles = new QueryProfiles(registry, new SilentDeployLogger());
         assertSerializedConfigFileEquals(root + "variants-of-explicit-compound.cfg", profiles.getConfig().toString());
     }
 
@@ -88,7 +90,7 @@ public class QueryProfileVariantsTestCase {
         registry.register(a2);
         registry.register(profile);
 
-        QueryProfiles profiles = new QueryProfiles(registry);
+        QueryProfiles profiles = new QueryProfiles(registry, new SilentDeployLogger());
         assertSerializedConfigFileEquals(root + "variants-of-explicit-compound-with-reference.cfg", profiles.getConfig().toString());
     }
 
@@ -108,8 +110,15 @@ public class QueryProfileVariantsTestCase {
         registry.register(a1);
         registry.register(profile);
 
-        QueryProfiles profiles = new QueryProfiles(registry);
+        QueryProfiles profiles = new QueryProfiles(registry, new SilentDeployLogger());
         assertSerializedConfigFileEquals(root + "explicit-reference-override.cfg", profiles.getConfig().toString());
     }
 
+    private static class SilentDeployLogger implements DeployLogger {
+
+        @Override
+        public void log(Level level, String message) {}
+
+    }
+
 }
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/QueryProfilesTestCase.java b/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/QueryProfilesTestCase.java
index 8c725ecc43c..746e771667f 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/QueryProfilesTestCase.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/container/search/test/QueryProfilesTestCase.java
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.model.container.search.test;
 
 import com.yahoo.component.ComponentId;
+import com.yahoo.config.application.api.DeployLogger;
 import com.yahoo.search.query.profile.QueryProfile;
 import com.yahoo.search.query.profile.QueryProfileRegistry;
 import com.yahoo.search.query.profile.compiled.CompiledQueryProfileRegistry;
@@ -11,12 +12,18 @@ import com.yahoo.search.query.profile.types.FieldDescription;
 import com.yahoo.search.query.profile.types.FieldType;
 import com.yahoo.search.query.profile.types.QueryProfileType;
 import com.yahoo.search.query.profile.types.QueryProfileTypeRegistry;
+import com.yahoo.searchdefinition.SearchBuilder;
+import com.yahoo.searchdefinition.parser.ParseException;
 import com.yahoo.vespa.model.container.search.QueryProfiles;
+import com.yahoo.vespa.model.test.utils.DeployLoggerStub;
 import org.junit.Test;
 
 import java.io.IOException;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 
 import static helpers.CompareConfigTestHelper.assertSerializedConfigFileEquals;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
@@ -112,15 +119,63 @@ public class QueryProfilesTestCase {
         registry.register(untypedUser);
 
         assertConfig("query-profiles.cfg",registry);
+
+        DeployLoggerStub logger = new DeployLoggerStub();
+        new QueryProfiles(registry, logger);
+        assertTrue(logger.entries.isEmpty());
+    }
+
+    @Test
+    public void testValidation() {
+        QueryProfileRegistry registry = new QueryProfileRegistry();
+        QueryProfileTypeRegistry typeRegistry = registry.getTypeRegistry();
+
+        QueryProfileType userType = new QueryProfileType("user");
+        typeRegistry.register(userType);
+
+        DeployLoggerStub logger = new DeployLoggerStub();
+        new QueryProfiles(registry, logger);
+        assertEquals(1, logger.entries.size());
+        assertEquals("This application define query profile types, but has no query profiles referencing them " +
+                     "so they have no effect. " +
+                     "See https://docs.vespa.ai/documentation/query-profiles.html",
+                     logger.entries.get(0).message);
+    }
+
+    @Test
+    public void testValidationWithTensorFields() {
+        QueryProfileRegistry registry = new QueryProfileRegistry();
+        QueryProfileTypeRegistry typeRegistry = registry.getTypeRegistry();
+
+        QueryProfileType userType = new QueryProfileType("user");
+        userType.addField(new FieldDescription("vector", FieldType.fromString("tensor(x[5])", typeRegistry)));
+        userType.addField(new FieldDescription("matrix", FieldType.fromString("tensor(x[5],y[5])", typeRegistry)));
+        typeRegistry.register(userType);
+
+        DeployLoggerStub logger = new DeployLoggerStub();
+        new QueryProfiles(registry, logger);
+        assertEquals(1, logger.entries.size());
+        assertEquals("This application define query profile types, but has no query profiles referencing them " +
+                     "so they have no effect. " +
+                     "In particular, the tensors (vector, matrix) will be interpreted as strings, not tensors if sent in requests. " +
+                     "See https://docs.vespa.ai/documentation/query-profiles.html",
+                     logger.entries.get(0).message);
     }
 
     protected void assertConfig(String correctFileName, QueryProfileRegistry check) throws IOException {
         assertSerializedConfigFileEquals(root + "/" + correctFileName,
-                com.yahoo.text.StringUtilities.implodeMultiline(com.yahoo.config.ConfigInstance.serialize(new QueryProfiles(check).getConfig())));
+                com.yahoo.text.StringUtilities.implodeMultiline(com.yahoo.config.ConfigInstance.serialize(new QueryProfiles(check, new SilentDeployLogger()).getConfig())));
 
         // Also assert that the correct config config can actually be read as a config source
         QueryProfileConfigurer configurer = new QueryProfileConfigurer("file:" + root + "empty.cfg");
         configurer.shutdown();
     }
 
+    private static class SilentDeployLogger implements DeployLogger {
+
+        @Override
+        public void log(Level level, String message) {}
+
+    }
+
 }
diff --git a/container-search/abi-spec.json b/container-search/abi-spec.json
index 80038da6750..38ce2aa2cf2 100644
--- a/container-search/abi-spec.json
+++ b/container-search/abi-spec.json
@@ -6177,6 +6177,7 @@
     "methods": [
       "public void <init>()",
       "public void register(com.yahoo.search.query.profile.types.QueryProfileType)",
+      "public boolean hasApplicationTypes()",
       "public void freeze()",
       "public static com.yahoo.search.query.profile.types.QueryProfileTypeRegistry emptyFrozen()"
     ],
diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/types/QueryProfileTypeRegistry.java b/container-search/src/main/java/com/yahoo/search/query/profile/types/QueryProfileTypeRegistry.java
index ff8c4845845..b76ae88ede1 100644
--- a/container-search/src/main/java/com/yahoo/search/query/profile/types/QueryProfileTypeRegistry.java
+++ b/container-search/src/main/java/com/yahoo/search/query/profile/types/QueryProfileTypeRegistry.java
@@ -12,8 +12,11 @@ import com.yahoo.search.query.profile.QueryProfileRegistry;
  */
 public class QueryProfileTypeRegistry extends ComponentRegistry<QueryProfileType> {
 
+    private final int nativeProfileCount;
+
     public QueryProfileTypeRegistry() {
         Query.addNativeQueryProfileTypesTo(this);
+        nativeProfileCount = allComponents().size();
     }
 
     /** Register this type by its id */
@@ -21,6 +24,11 @@ public class QueryProfileTypeRegistry extends ComponentRegistry<QueryProfileType
         super.register(type.getId(), type);
     }
 
+    /** Returns true if this has types in addition to the native Vespa types */
+    public boolean hasApplicationTypes() {
+        return allComponents().size() > nativeProfileCount;
+    }
+
     @Override
     public void freeze() {
         if (isFrozen()) return;