Merge pull request #15522 from vespa-engine/bratseth/reject-tensor-attribute-searches

Reject queries searching tensor fields

13 files changed, 151 insertions, 26 deletions

diff --git a/container-search/abi-spec.json b/container-search/abi-spec.json
index cd3f011352d..49c8d6c82d6 100644
--- a/container-search/abi-spec.json
+++ b/container-search/abi-spec.json
@@ -7974,6 +7974,18 @@
     ],
     "fields": []
   },
+  "com.yahoo.search.searchers.QueryValidator": {
+    "superClass": "com.yahoo.search.Searcher",
+    "interfaces": [],
+    "attributes": [
+      "public"
+    ],
+    "methods": [
+      "public void <init>()",
+      "public com.yahoo.search.Result search(com.yahoo.search.Query, com.yahoo.search.searchchain.Execution)"
+    ],
+    "fields": []
+  },
   "com.yahoo.search.searchers.RateLimitingSearcher": {
     "superClass": "com.yahoo.search.Searcher",
     "interfaces": [],
diff --git a/container-search/src/main/java/com/yahoo/prelude/Index.java b/container-search/src/main/java/com/yahoo/prelude/Index.java
index 0dfbf6470ad..42a8e05b8c0 100644
--- a/container-search/src/main/java/com/yahoo/prelude/Index.java
+++ b/container-search/src/main/java/com/yahoo/prelude/Index.java
@@ -44,11 +44,14 @@ public class Index {
     /** The null index - don't use this for name lookups */
     public static final Index nullIndex = new Index("(null)");
 
-    private String name;
+    private final String name;
 
-    private List<String> aliases = new ArrayList<>();
+    private String type; // TODO: Parse top a type object; do not expose this as a string
+
+    private final List<String> aliases = new ArrayList<>();
 
     // The state resulting from adding commands to this (using addCommand)
+    private boolean tensor = false;
     private boolean uriIndex = false;
     private boolean hostIndex = false;
     private StemMode stemMode = StemMode.NONE;
@@ -79,11 +82,11 @@ public class Index {
     /** The string terminating an exact token in this index, or null to use the default (space) */
     private String exactTerminator = null;
 
-    /** Commands which are not coverted into a field */
-    private Set<String> commands = new java.util.HashSet<>();
+    /** Commands which are not converted into a field */
+    private final Set<String> commands = new java.util.HashSet<>();
 
     /** All the commands added to this, including those converted to fields above */
-    private List<String> allCommands = new java.util.ArrayList<>();
+    private final List<String> allCommands = new java.util.ArrayList<>();
 
     public Index(String name) {
         this.name = name;
@@ -142,7 +145,9 @@ public class Index {
     public Index addCommand(String commandString) {
         allCommands.add(commandString);
 
-        if ("fullurl".equals(commandString)) {
+        if (commandString.startsWith("type tensor(")) { // TODO: Type info can replace numerical, predicate, multivalue
+            setTensor(true);
+        } else if ("fullurl".equals(commandString)) {
             setUriIndex(true);
         } else if ("urlhost".equals(commandString)) {
             setHostIndex(true);
@@ -196,6 +201,12 @@ public class Index {
         return this;
     }
 
+    private void setTensor(boolean tensor) {
+        this.tensor = tensor;
+    }
+
+    public boolean isTensor() { return tensor; }
+
     private void setPredicateBounds(String bounds) {
         if ( ! bounds.startsWith("[..")) {
             predicateLowerBound = Long.parseLong(bounds.substring(1, bounds.indexOf("..")));
@@ -270,9 +281,7 @@ public class Index {
         return "(null)".equals(name);
     }
 
-    public boolean isAttribute() {
-        return isAttribute;
-    }
+    public boolean isAttribute() { return isAttribute; }
 
     public void setAttribute(boolean isAttribute) {
         this.isAttribute = isAttribute;
diff --git a/container-search/src/main/java/com/yahoo/prelude/SearchDefinition.java b/container-search/src/main/java/com/yahoo/prelude/SearchDefinition.java
index 7859a9698d9..3d1240237e4 100644
--- a/container-search/src/main/java/com/yahoo/prelude/SearchDefinition.java
+++ b/container-search/src/main/java/com/yahoo/prelude/SearchDefinition.java
@@ -1,8 +1,6 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.prelude;
 
-import com.yahoo.prelude.Index.Attribute;
-
 import java.util.HashMap;
 import java.util.Map;
 
@@ -17,13 +15,13 @@ import static com.yahoo.text.Lowercase.toLowerCase;
 // TODO: Make freezable!
 public class SearchDefinition {
 
-    private String name;
+    private final String name;
 
     /** A map of all indices in this search definition, indexed by name */
-    private Map<String, Index> indices = new HashMap<>();
+    private final Map<String, Index> indices = new HashMap<>();
 
     /* A map of all indices in this search definition, indexed by lower cased name. */
-    private Map<String, Index> lowerCase = new HashMap<>();
+    private final Map<String, Index> lowerCase = new HashMap<>();
 
     private String defaultPosition;
 
diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java
index 2090390890e..d30e67195c3 100644
--- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java
+++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java
@@ -43,7 +43,7 @@ public abstract class VespaBackEndSearcher extends PingableSearcher {
     private String serverId;
 
     /** The set of all document databases available in the backend handled by this searcher */
-    private Map<String, DocumentDatabase> documentDbs = new LinkedHashMap<>();
+    private final Map<String, DocumentDatabase> documentDbs = new LinkedHashMap<>();
     private DocumentDatabase defaultDocumentDb = null;
 
     /** Default docsum class. null means "unset" and is the default value */
diff --git a/container-search/src/main/java/com/yahoo/prelude/query/HasIndexItem.java b/container-search/src/main/java/com/yahoo/prelude/query/HasIndexItem.java
index 6641dee9780..c70ee9e4def 100644
--- a/container-search/src/main/java/com/yahoo/prelude/query/HasIndexItem.java
+++ b/container-search/src/main/java/com/yahoo/prelude/query/HasIndexItem.java
@@ -2,8 +2,7 @@
 package com.yahoo.prelude.query;
 
 /**
- * An interface for items where it is useful to access an associated
- * index name.
+ * An interface for items where it is useful to access an index name.
  *
  * @author  Steinar Knutsen
  */
@@ -11,7 +10,7 @@ public interface HasIndexItem {
 
     String getIndexName();
 
-    /** @return how many phrase words does this item contain */
+    /** Returns how many phrase words does this item contain */
     int getNumWords();
 
 }
diff --git a/container-search/src/main/java/com/yahoo/prelude/query/IndexedItem.java b/container-search/src/main/java/com/yahoo/prelude/query/IndexedItem.java
index 3dafa230eb8..cba289fa5d8 100644
--- a/container-search/src/main/java/com/yahoo/prelude/query/IndexedItem.java
+++ b/container-search/src/main/java/com/yahoo/prelude/query/IndexedItem.java
@@ -3,7 +3,7 @@ package com.yahoo.prelude.query;
 
 
 /**
- * Interface for Items that is indexed
+ * Interface for Items that are indexed
  *
  * @author Lars Christian Jensen
  */
diff --git a/container-search/src/main/java/com/yahoo/prelude/query/ToolBox.java b/container-search/src/main/java/com/yahoo/prelude/query/ToolBox.java
index 72ba012ab07..5a6d4cd6382 100644
--- a/container-search/src/main/java/com/yahoo/prelude/query/ToolBox.java
+++ b/container-search/src/main/java/com/yahoo/prelude/query/ToolBox.java
@@ -18,8 +18,7 @@ public final class ToolBox {
          * {@link ToolBox#visit(QueryVisitor, Item)}. Return true to visit the
          * sub-items of the given item, return false to ignore the sub-items.
          *
-         * @param item
-         *            each item in the query tree
+         * @param item each item in the query tree
          * @return whether or not to visit the sub-items of the argument item
          *         (and then invoke the {@link #onExit()} method)
          */
diff --git a/container-search/src/main/java/com/yahoo/search/searchchain/model/federation/LocalProviderSpec.java b/container-search/src/main/java/com/yahoo/search/searchchain/model/federation/LocalProviderSpec.java
index 8db690678bb..5df12dc2053 100644
--- a/container-search/src/main/java/com/yahoo/search/searchchain/model/federation/LocalProviderSpec.java
+++ b/container-search/src/main/java/com/yahoo/search/searchchain/model/federation/LocalProviderSpec.java
@@ -33,6 +33,7 @@ public class LocalProviderSpec {
                             com.yahoo.search.querytransform.RangeQueryOptimizer.class,
                             com.yahoo.search.querytransform.SortingDegrader.class,
                             com.yahoo.prelude.searcher.ValidateSortingSearcher.class,
+                            com.yahoo.search.searchers.QueryValidator.class,
                             com.yahoo.prelude.cluster.ClusterSearcher.class,
                             com.yahoo.search.grouping.GroupingValidator.class,
                             com.yahoo.search.grouping.vespa.GroupingExecutor.class,
diff --git a/container-search/src/main/java/com/yahoo/search/searchers/InputCheckingSearcher.java b/container-search/src/main/java/com/yahoo/search/searchers/InputCheckingSearcher.java
index 5e15a8ba14b..d8391fe08f3 100644
--- a/container-search/src/main/java/com/yahoo/search/searchers/InputCheckingSearcher.java
+++ b/container-search/src/main/java/com/yahoo/search/searchers/InputCheckingSearcher.java
@@ -12,6 +12,8 @@ import java.util.Map;
 import java.util.logging.Logger;
 
 import java.util.logging.Level;
+
+import com.yahoo.component.chain.dependencies.Before;
 import com.yahoo.metrics.simple.Counter;
 import com.yahoo.metrics.simple.MetricReceiver;
 import com.yahoo.prelude.query.CompositeItem;
@@ -25,14 +27,16 @@ import com.yahoo.search.Result;
 import com.yahoo.search.Searcher;
 import com.yahoo.search.result.ErrorMessage;
 import com.yahoo.search.searchchain.Execution;
+import com.yahoo.search.searchchain.PhaseNames;
 import com.yahoo.yolean.Exceptions;
 
 /**
- * Check whether the query tree seems to be "well formed". In other words, run heurestics against
+ * Check whether the query tree seems to be "well formed". In other words, run heuristics against
  * the input data to see whether the query should sent to the search backend.
  *
  * @author Steinar Knutsen
  */
+@Before(PhaseNames.BACKEND)
 public class InputCheckingSearcher extends Searcher {
 
     private final Counter utfRejections;
diff --git a/container-search/src/main/java/com/yahoo/search/searchers/QueryValidator.java b/container-search/src/main/java/com/yahoo/search/searchers/QueryValidator.java
new file mode 100644
index 00000000000..26b5c2bc665
--- /dev/null
+++ b/container-search/src/main/java/com/yahoo/search/searchers/QueryValidator.java
@@ -0,0 +1,60 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.search.searchers;
+
+import com.yahoo.component.chain.dependencies.After;
+import com.yahoo.component.chain.dependencies.Before;
+import com.yahoo.prelude.Index;
+import com.yahoo.prelude.IndexFacts;
+import com.yahoo.prelude.query.CompositeItem;
+import com.yahoo.prelude.query.HasIndexItem;
+import com.yahoo.prelude.query.Item;
+import com.yahoo.prelude.query.ToolBox;
+import com.yahoo.search.Query;
+import com.yahoo.search.Result;
+import com.yahoo.search.Searcher;
+import com.yahoo.search.searchchain.Execution;
+import com.yahoo.search.searchchain.PhaseNames;
+
+import static com.yahoo.search.grouping.GroupingQueryParser.SELECT_PARAMETER_PARSING;
+
+/**
+ * Validation of query operators against the schema which is searched
+ *
+ * @author bratseth
+ */
+@After(SELECT_PARAMETER_PARSING)
+@Before(PhaseNames.BACKEND)
+public class QueryValidator extends Searcher {
+
+    @Override
+    public Result search(Query query, Execution execution) {
+        IndexFacts.Session session = execution.context().getIndexFacts().newSession(query);
+        ToolBox.visit(new ItemValidator(session), query.getModel().getQueryTree().getRoot());
+        return execution.search(query);
+    }
+
+    private static class ItemValidator extends ToolBox.QueryVisitor {
+
+        IndexFacts.Session session;
+
+        public ItemValidator(IndexFacts.Session session) {
+            this.session = session;
+        }
+
+        @Override
+        public boolean visit(Item item) {
+            if (item instanceof HasIndexItem) {
+                String indexName = ((HasIndexItem)item).getIndexName();
+                Index index = session.getIndex(indexName);
+                if (index != null && index.isTensor())
+                    throw new IllegalArgumentException("Cannot search '" + indexName + "': It is a tensor field");
+            }
+            return true;
+        }
+
+        @Override
+        public void onExit() { }
+
+    }
+
+}
diff --git a/container-search/src/main/java/com/yahoo/search/searchers/ValidateNearestNeighborSearcher.java b/container-search/src/main/java/com/yahoo/search/searchers/ValidateNearestNeighborSearcher.java
index aca2998cba3..65ca4a93cc1 100644
--- a/container-search/src/main/java/com/yahoo/search/searchers/ValidateNearestNeighborSearcher.java
+++ b/container-search/src/main/java/com/yahoo/search/searchers/ValidateNearestNeighborSearcher.java
@@ -33,7 +33,7 @@ import java.util.Optional;
 @Before(GroupingExecutor.COMPONENT_NAME) // Must happen before query.prepare()
 public class ValidateNearestNeighborSearcher extends Searcher {
 
-    private Map<String, TensorType> validAttributes = new HashMap<>();
+    private final Map<String, TensorType> validAttributes = new HashMap<>();
 
     public ValidateNearestNeighborSearcher(AttributesConfig attributesConfig) {
         for (AttributesConfig.Attribute a : attributesConfig.attribute()) {
@@ -61,12 +61,10 @@ public class ValidateNearestNeighborSearcher extends Searcher {
 
         public Optional<ErrorMessage> errorMessage = Optional.empty();
 
-        private final RankProperties rankProperties;
         private final Map<String, TensorType> validAttributes;
         private final Query query;
 
         public NNVisitor(RankProperties rankProperties, Map<String, TensorType> validAttributes, Query query) {
-            this.rankProperties = rankProperties;
             this.validAttributes = validAttributes;
             this.query = query;
         }
diff --git a/container-search/src/test/java/com/yahoo/prelude/searcher/test/ValidatePredicateSearcherTestCase.java b/container-search/src/test/java/com/yahoo/prelude/searcher/test/ValidatePredicateSearcherTestCase.java
index 2187cb89ae2..850586ba8c5 100644
--- a/container-search/src/test/java/com/yahoo/prelude/searcher/test/ValidatePredicateSearcherTestCase.java
+++ b/container-search/src/test/java/com/yahoo/prelude/searcher/test/ValidatePredicateSearcherTestCase.java
@@ -23,7 +23,7 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 
 /**
- * @author <a href="mailto:magnarn@yahoo-inc.com">Magnar Nedland</a>
+ * @author Magnar Nedland
  */
 public class ValidatePredicateSearcherTestCase {
 
diff --git a/container-search/src/test/java/com/yahoo/search/searchers/test/QueryValidatorTestCase.java b/container-search/src/test/java/com/yahoo/search/searchers/test/QueryValidatorTestCase.java
new file mode 100644
index 00000000000..62089f48317
--- /dev/null
+++ b/container-search/src/test/java/com/yahoo/search/searchers/test/QueryValidatorTestCase.java
@@ -0,0 +1,45 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.search.searchers.test;
+
+import com.yahoo.config.subscription.ConfigGetter;
+import com.yahoo.prelude.IndexFacts;
+import com.yahoo.prelude.IndexModel;
+import com.yahoo.prelude.SearchDefinition;
+import com.yahoo.search.Query;
+import com.yahoo.search.config.IndexInfoConfig;
+import com.yahoo.search.searchchain.Execution;
+import com.yahoo.search.searchers.QueryValidator;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+/**
+ * @author bratseth
+ */
+public class QueryValidatorTestCase {
+
+    @Test
+    public void testValidation() {
+        SearchDefinition sd = new SearchDefinition("test");
+        sd.addCommand("mytensor", "type tensor(x[100]");
+        sd.addCommand("mystring", "type string");
+        IndexModel model = new IndexModel(sd);
+
+        IndexFacts indexFacts = new IndexFacts(model);
+        Execution execution = new Execution(Execution.Context.createContextStub(indexFacts));
+        new QueryValidator().search(new Query("?query=mystring:foo"), execution);
+
+        try {
+            new QueryValidator().search(new Query("?query=mytensor:foo"), execution);
+            fail("Excpected validation error");
+        }
+        catch (IllegalArgumentException e) {
+            // success
+            assertEquals("Cannot search 'mytensor': It is a tensor field", e.getMessage());
+        }
+    }
+
+}