Verify that predicate fields are not used with non-predicate operators

2 files changed, 38 insertions, 16 deletions

diff --git a/container-search/src/main/java/com/yahoo/prelude/searcher/ValidatePredicateSearcher.java b/container-search/src/main/java/com/yahoo/prelude/searcher/ValidatePredicateSearcher.java
index 9b6f5926b61..a8b3c76fe00 100644
--- a/container-search/src/main/java/com/yahoo/prelude/searcher/ValidatePredicateSearcher.java
+++ b/container-search/src/main/java/com/yahoo/prelude/searcher/ValidatePredicateSearcher.java
@@ -1,12 +1,12 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.prelude.searcher;
 
-import java.util.Optional;
 import com.yahoo.component.chain.dependencies.After;
 import com.yahoo.prelude.Index;
 import com.yahoo.prelude.IndexFacts;
 import com.yahoo.prelude.query.Item;
 import com.yahoo.prelude.query.PredicateQueryItem;
+import com.yahoo.prelude.query.SimpleIndexedItem;
 import com.yahoo.prelude.query.ToolBox;
 import com.yahoo.search.Query;
 import com.yahoo.search.Result;
@@ -15,7 +15,8 @@ import com.yahoo.search.querytransform.BooleanSearcher;
 import com.yahoo.search.result.ErrorMessage;
 import com.yahoo.search.searchchain.Execution;
 
-import java.util.Collection;
+import java.util.ArrayList;
+import java.util.List;
 
 /**
  * Checks that predicate queries don't use values outside the defined upper/lower bounds.
@@ -27,26 +28,26 @@ public class ValidatePredicateSearcher extends Searcher {
 
     @Override
     public Result search(Query query, Execution execution) {
-        Optional<ErrorMessage> e = validate(query, execution.context().getIndexFacts().newSession(query));
-        if (e.isPresent()) {
+        List<ErrorMessage> errorMessages = validate(query, execution.context().getIndexFacts().newSession(query));
+        if (!errorMessages.isEmpty()) {
             Result r = new Result(query);
-            r.hits().addError(e.get());
+            errorMessages.forEach(msg -> r.hits().addError(msg));
             return r;
         }
         return execution.search(query);
     }
 
-    private Optional<ErrorMessage> validate(Query query, IndexFacts.Session indexFacts) {
+    private List<ErrorMessage> validate(Query query, IndexFacts.Session indexFacts) {
         ValidatePredicateVisitor visitor = new ValidatePredicateVisitor(indexFacts);
         ToolBox.visit(visitor, query.getModel().getQueryTree().getRoot());
-        return visitor.errorMessage;
+        return visitor.errorMessages;
     }
 
     private static class ValidatePredicateVisitor extends ToolBox.QueryVisitor {
 
         private final IndexFacts.Session indexFacts;
 
-        public Optional<ErrorMessage> errorMessage = Optional.empty();
+        final List<ErrorMessage> errorMessages = new ArrayList<>();
 
         public ValidatePredicateVisitor(IndexFacts.Session indexFacts) {
             this.indexFacts = indexFacts;
@@ -57,22 +58,37 @@ public class ValidatePredicateSearcher extends Searcher {
             if (item instanceof PredicateQueryItem) {
                 visit((PredicateQueryItem) item);
             }
+            if (item instanceof SimpleIndexedItem) {
+                visit((SimpleIndexedItem) item);
+            }
             return true;
         }
 
         private void visit(PredicateQueryItem item) {
-            Index index = getIndexFromUnionOfDocumentTypes(item);
+            Index index = getIndexFromUnionOfDocumentTypes(item.getIndexName());
+            if (!index.isPredicate()) {
+                errorMessages.add(ErrorMessage.createIllegalQuery(String.format("Index '%s' is not a predicate attribute.", index.getName())));
+            }
             for (PredicateQueryItem.RangeEntry entry : item.getRangeFeatures()) {
                 long value = entry.getValue();
                 if (value < index.getPredicateLowerBound() || value > index.getPredicateUpperBound()) {
-                    errorMessage = Optional.of(ErrorMessage.createIllegalQuery(
-                            String.format("%s=%d outside configured predicate bounds.", entry.getKey(), value)));
+                    errorMessages.add(
+                            ErrorMessage.createIllegalQuery(String.format("%s=%d outside configured predicate bounds.", entry.getKey(), value)));
                 }
             }
         }
 
-        private Index getIndexFromUnionOfDocumentTypes(PredicateQueryItem item) {
-            return indexFacts.getIndex(item.getIndexName());
+        private void visit(SimpleIndexedItem item) {
+            String indexName = item.getIndexName();
+            Index index = getIndexFromUnionOfDocumentTypes(indexName);
+            if (index.isPredicate()) {
+                errorMessages.add(
+                        ErrorMessage.createIllegalQuery(String.format("Index '%s' is predicate attribute and can only be used in conjunction with a predicate query operator.", indexName)));
+            }
+        }
+
+        private Index getIndexFromUnionOfDocumentTypes(String indexName) {
+            return indexFacts.getIndex(indexName);
         }
 
         @Override
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 1b9ca1cd29b..2187cb89ae2 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
@@ -2,7 +2,6 @@
 package com.yahoo.prelude.searcher.test;
 
 import com.google.common.util.concurrent.MoreExecutors;
-import com.yahoo.language.Linguistics;
 import com.yahoo.language.simple.SimpleLinguistics;
 import com.yahoo.prelude.Index;
 import com.yahoo.prelude.IndexFacts;
@@ -20,8 +19,6 @@ import com.yahoo.search.searchchain.Execution;
 import com.yahoo.search.yql.YqlParser;
 import org.junit.Test;
 
-import java.util.*;
-
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 
@@ -46,6 +43,14 @@ public class ValidatePredicateSearcherTestCase {
         assertEquals(ErrorMessage.createIllegalQuery("age=200 outside configured predicate bounds."), r.hits().getError());
     }
 
+    @Test
+    public void queryFailsWhenPredicateFieldIsUsedInTermSearch() {
+        ValidatePredicateSearcher searcher = new ValidatePredicateSearcher();
+        String q = "select * from sources * where predicate_field CONTAINS \"true\";";
+        Result r = doSearch(searcher, q, "predicate-bounds [0..99]");
+        assertEquals(ErrorMessage.createIllegalQuery("Index 'predicate_field' is predicate attribute and can only be used in conjunction with a predicate query operator."), r.hits().getError());
+    }
+
     private static Result doSearch(ValidatePredicateSearcher searcher, String yqlQuery, String command) {
         QueryTree queryTree = new YqlParser(new ParserEnvironment()).parse(new Parsable().setQuery(yqlQuery));
         Query query = new Query();
@@ -53,6 +58,7 @@ public class ValidatePredicateSearcherTestCase {
 
         SearchDefinition searchDefinition = new SearchDefinition("document");
         Index index = new Index("predicate_field");
+        index.setPredicate(true);
         index.addCommand(command);
         searchDefinition.addIndex(index);
         IndexFacts indexFacts = new IndexFacts(new IndexModel(searchDefinition));