Changes from review.

6 files changed, 48 insertions, 28 deletions

diff --git a/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java b/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java
index c9e723db6d0..5ac1f834031 100644
--- a/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java
+++ b/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java
@@ -586,10 +586,9 @@ public class SearchHandler extends LoggingRequestHandler {
                 throw new QueryException("Illegal query: Query contains both yql- and select-parameter");
             }
 
-            // The query-parameter overrides the select-parameter. Removing them here to prevent further processing of select-parameters.
-            if (requestMap.containsKey("query")) {
-                requestMap.remove("select.where");
-                requestMap.remove("select.grouping");
+            // Throws QueryException if query contains both query- and select-parameter
+            if (requestMap.containsKey("query") && (requestMap.containsKey("select.where") || requestMap.containsKey("select.grouping")) ) {
+                throw new QueryException("Illegal query: Query contains both query- and select-parameter");
             }
 
             return requestMap;
diff --git a/container-search/src/main/java/com/yahoo/search/query/Model.java b/container-search/src/main/java/com/yahoo/search/query/Model.java
index 167bb312f61..ddd33cb7c78 100644
--- a/container-search/src/main/java/com/yahoo/search/query/Model.java
+++ b/container-search/src/main/java/com/yahoo/search/query/Model.java
@@ -249,7 +249,7 @@ public class Model implements Cloneable {
     public String getQueryString() { return queryString; }
 
     /**
-     * Returns the query as an object structure.
+     * Returns the query as an object structure. Remember to have the correct Query.Type set.
      * This causes parsing of the query string if it has changed since this was last called
      * (i.e query parsing is lazy)
      */
diff --git a/container-search/src/main/java/com/yahoo/search/query/Select.java b/container-search/src/main/java/com/yahoo/search/query/Select.java
index a234b81c1be..e35eeb57278 100644
--- a/container-search/src/main/java/com/yahoo/search/query/Select.java
+++ b/container-search/src/main/java/com/yahoo/search/query/Select.java
@@ -56,8 +56,8 @@ public class Select implements Cloneable {
     }
 
 
-    /** returns the query owning this, never null */
-    public Query getParent() { return parent; }
+    /** Returns the query owning this, never null */
+    private Query getParent() { return parent; }
 
 
     /** Assigns the query owning this */
@@ -67,31 +67,24 @@ public class Select implements Cloneable {
     }
 
 
-    public static Select getFrom(Query q) {
-        return (Select) q.properties().get(argumentTypeName);
-    }
-
-
-    public void setQueryString(String queryString) {
-        model.setQueryString(queryString);
-    }
-
-
-    /** Set the where-clause for the query. Must be a JSON-string. */
+    /** Set the where-clause for the query. Must be a JSON-string, with the format described in the Select Reference doc. */
     public void setWhere(String where) {
         this.where = where;
+        model.setType(SELECT);
+
+        // Setting the queryTree to null
+        model.setQueryString(null);
     }
 
 
     /** Returns the where-clause in the query */
-    public String getWhere(){
+    public String getWhereString(){
         return this.where;
     }
 
 
-    /** Set the grouping-string for the query. Must be a JSON-string. */
+    /** Set the grouping-string for the query. Must be a JSON-string, with the format described in the Select Reference doc. */
     public void setGrouping(String grouping){
-
         this.grouping = grouping;
         SelectParser parser = (SelectParser) ParserFactory.newInstance(Query.Type.SELECT, new ParserEnvironment());
 
@@ -100,12 +93,11 @@ public class Select implements Cloneable {
                     .setRootOperation(step.getOperation())
                     .continuations().addAll(step.continuations());
         }
-
     }
 
 
     /** Returns the grouping in the query */
-    public String getGrouping(){
+    public String getGroupingString(){
         return this.grouping;
     }
 
diff --git a/container-search/src/main/java/com/yahoo/search/query/SelectParser.java b/container-search/src/main/java/com/yahoo/search/query/SelectParser.java
index a8406413856..13ebacb62ef 100644
--- a/container-search/src/main/java/com/yahoo/search/query/SelectParser.java
+++ b/container-search/src/main/java/com/yahoo/search/query/SelectParser.java
@@ -164,7 +164,7 @@ public class SelectParser implements Parser {
 
 
     private QueryTree buildTree() {
-        Inspector inspector = SlimeUtils.jsonToSlime(this.query.getSelect().getWhere().getBytes()).get();
+        Inspector inspector = SlimeUtils.jsonToSlime(this.query.getSelect().getWhereString().getBytes()).get();
         if (inspector.field("error_message").valid()){
             throw new QueryException("Illegal query: "+inspector.field("error_message").asString() + ", at: "+ new String(inspector.field("offending_input").asData(), StandardCharsets.UTF_8));
         }
diff --git a/container-search/src/main/java/com/yahoo/search/query/properties/QueryProperties.java b/container-search/src/main/java/com/yahoo/search/query/properties/QueryProperties.java
index 286062db755..71002166b11 100644
--- a/container-search/src/main/java/com/yahoo/search/query/properties/QueryProperties.java
+++ b/container-search/src/main/java/com/yahoo/search/query/properties/QueryProperties.java
@@ -114,8 +114,8 @@ public class QueryProperties extends Properties {
             }
         }
         else if (key.size()==2 && key.first().equals(Select.SELECT)) {
-            if (key.last().equals(Select.WHERE)) return query.getSelect().getWhere();
-            if (key.last().equals(Select.GROUPING)) return query.getSelect().getGrouping();
+            if (key.last().equals(Select.WHERE)) return query.getSelect().getWhereString();
+            if (key.last().equals(Select.GROUPING)) return query.getSelect().getGroupingString();
         }
         else if (key.size()==2 && key.first().equals(Presentation.PRESENTATION)) {
             if (key.last().equals(Presentation.BOLDING)) return query.getPresentation().getBolding();
@@ -254,10 +254,8 @@ public class QueryProperties extends Properties {
             else if (key.size()==2 && key.first().equals(Select.SELECT)) {
                 if (key.last().equals(Select.WHERE)){
                     query.getSelect().setWhere(asString(value, ""));
-                    query.getModel().setType(asString(value, "SELECT"));
                 } else if (key.last().equals(Select.GROUPING)) {
                     query.getSelect().setGrouping(asString(value, ""));
-                    query.getModel().setType(asString(value, "SELECT"));
                 }
             }
             else if (key.first().equals("rankfeature") || key.first().equals("featureoverride") ) { // featureoverride is deprecated
diff --git a/container-search/src/test/java/com/yahoo/select/SelectParserTestCase.java b/container-search/src/test/java/com/yahoo/select/SelectParserTestCase.java
index 31809f5f7d9..031ba386ad4 100644
--- a/container-search/src/test/java/com/yahoo/select/SelectParserTestCase.java
+++ b/container-search/src/test/java/com/yahoo/select/SelectParserTestCase.java
@@ -15,6 +15,7 @@ import com.yahoo.prelude.query.WeakAndItem;
 import com.yahoo.prelude.query.WordAlternativesItem;
 import com.yahoo.prelude.query.WordItem;
 import com.yahoo.search.Query;
+import com.yahoo.search.federation.ProviderConfig;
 import com.yahoo.search.query.QueryTree;
 import com.yahoo.search.query.Select;
 import com.yahoo.search.query.SelectParser;
@@ -48,6 +49,7 @@ public class SelectParserTestCase {
 
     private final SelectParser parser = new SelectParser(new ParserEnvironment());
 
+
     /** WHERE TESTS */
 
     @Test
@@ -659,6 +661,35 @@ public class SelectParserTestCase {
 
 
 
+    /** OTHER TESTS */
+
+    @Test
+    public void testOverridingOtherQueryTree() {
+        Query query = new Query("?query=default:query");
+        assertEquals("default:query", query.getModel().getQueryTree().toString());
+        assertEquals(Query.Type.ALL, query.getModel().getType());
+
+        query.getSelect().setWhere("{\"contains\" : [\"default\", \"select\"] }");
+        assertEquals("default:select", query.getModel().getQueryTree().toString());
+        assertEquals(Query.Type.SELECT, query.getModel().getType());
+    }
+
+
+    @Test
+    public void testOverridingWhereQueryTree() {
+        Query query = new Query();
+        query.getSelect().setWhere("{\"contains\" : [\"default\", \"select\"] }");
+        assertEquals("default:select", query.getModel().getQueryTree().toString());
+        assertEquals(Query.Type.SELECT, query.getModel().getType());
+
+        query.getModel().setQueryString("default:query");
+        query.getModel().setType("all");
+        assertEquals("default:query", query.getModel().getQueryTree().toString());
+        assertEquals(Query.Type.ALL, query.getModel().getType());
+    }
+
+
+
 
     /** Assert-methods */
     private void assertParse(String where, String expectedQueryTree) {