Revert "Revert "Revert "Added a object structure for GroupingRequest objects, accessable from…"""

3 files changed, 54 insertions, 92 deletions

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 cbc15bf39a1..bd0c229085b 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
@@ -237,13 +237,12 @@ public class Model implements Cloneable {
     public void setQueryString(String queryString) {
         if (queryString == null) queryString="";
         this.queryString = queryString;
-        clearQueryTree();
+        queryTree = null; // Cause parsing of the new query string next time the tree is accessed
     }
 
     /**
      * Returns the query string which caused the original query tree of this model to come about.
      * Note that changes to the query tree are <b>not</b> reflected in this query string.
-     * Note that changes to the query tree are <b>not</b> reflected in this query string.
      *
      * @return the original (or reassigned) query string - never null
      */
@@ -266,14 +265,6 @@ public class Model implements Cloneable {
     }
 
     /**
-     * Clears the parsed query such that it will be created anew from the textual representation (a query string or
-     * select.where expression) on the next access.
-     */
-    public void clearQueryTree() {
-        queryTree = null;
-    }
-
-    /**
      * Returns the filter string set for this query.
      * The filter is included in the query tree at the time the query tree is parsed
      */
@@ -346,25 +337,25 @@ public class Model implements Cloneable {
                QueryHelper.combineHash(encoding,filter,language,getQueryTree(),sources,restrict,defaultIndex,type,searchPath);
     }
 
-    @Override
+
     public Object clone() {
         try {
-            Model clone = (Model)super.clone();
+            Model clone = (Model) super.clone();
             if (queryTree != null)
                 clone.queryTree = this.queryTree.clone();
-            if (sources != null)
+            if (sources !=null)
                 clone.sources = new LinkedHashSet<>(this.sources);
-            if (restrict != null)
+            if (restrict !=null)
                 clone.restrict = new LinkedHashSet<>(this.restrict);
             return clone;
         }
         catch (CloneNotSupportedException e) {
-            throw new RuntimeException("Someone inserted a noncloneable superclass", e);
+            throw new RuntimeException("Someone inserted a noncloneable superclass",e);
         }
     }
 
     public Model cloneFor(Query q)  {
-        Model model = (Model)this.clone();
+        Model model = (Model) this.clone();
         model.setParent(q);
         return model;
     }
@@ -374,7 +365,7 @@ public class Model implements Cloneable {
 
     /** Assigns the query owning this */
     public void setParent(Query parent) {
-        if (parent == null) throw new NullPointerException("A query models owner cannot be null");
+        if (parent==null) throw new NullPointerException("A query models owner cannot be null");
         this.parent = parent;
     }
 
@@ -412,7 +403,7 @@ public class Model implements Cloneable {
 
     /** Sets the execution working on this. For internal use. */
     public void setExecution(Execution execution) {
-        if (execution == this.execution) return;
+        if (execution==this.execution) return;
 
         // If not already coupled, bind the trace of the new execution into the existing execution trace
         if (execution.trace().traceNode().isRoot()
@@ -434,7 +425,7 @@ public class Model implements Cloneable {
     /** Returns the Execution working on this, or a null execution if none. For internal use. */
     public Execution getExecution() { return execution; }
 
-    private void setFromString(String string, Set<String> set) {
+    private void setFromString(String string,Set<String> set) {
         set.clear();
         for (String item : string.split(","))
             set.add(item.trim());
@@ -529,6 +520,7 @@ public class Model implements Cloneable {
         return false;
     }
 
+
     /**
      * Set the YTrace header value to use when transmitting this model to a
      * search backend (of some kind).
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 bbc152c6391..3ffc6bddb24 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
@@ -10,11 +10,6 @@ import com.yahoo.search.query.profile.types.FieldDescription;
 import com.yahoo.search.query.profile.types.QueryProfileType;
 import com.yahoo.search.yql.VespaGroupingStep;
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.LinkedHashSet;
-import java.util.List;
-import java.util.Objects;
 
 
 /**
@@ -32,75 +27,67 @@ public class Select implements Cloneable {
     public static final String WHERE = "where";
     public static final String GROUPING = "grouping";
 
-    private final Query parent;
-    private final List<GroupingRequest> groupingRequests;
 
-    private String where;
-    private String grouping;
+    private static Model model;
+    private Query parent;
+    private String where = "";
+    private String grouping = "";
 
     static {
         argumentType = new QueryProfileType(SELECT);
         argumentType.setStrict(true);
         argumentType.setBuiltin(true);
-        argumentType.addField(new FieldDescription(WHERE, "string"));
-        argumentType.addField(new FieldDescription(GROUPING, "string"));
+        argumentType.addField(new FieldDescription(WHERE, "string", "where"));
+        argumentType.addField(new FieldDescription(GROUPING, "string", "grouping"));
         argumentType.freeze();
-        argumentTypeName = new CompoundName(argumentType.getId().getName());
+        argumentTypeName=new CompoundName(argumentType.getId().getName());
     }
 
     public static QueryProfileType getArgumentType() { return argumentType; }
 
-    /** Creates an empty select statement */
-    public Select(Query query) {
-        this("", "", query);
+    public Select(String where, String grouping){
+        this.where = where;
+        this.grouping = grouping;
     }
 
-    public Select(String where, String grouping, Query query) {
-        this(where, grouping, query, Collections.emptyList());
+    public Select(Query query) {
+        setParent(query);
+        model = query.getModel();
     }
 
-    private Select(String where, String grouping, Query query, List<GroupingRequest> groupingRequests) {
-        this.where = Objects.requireNonNull(where, "A Select must have a where string (possibly the empty string)");
-        this.grouping = Objects.requireNonNull(grouping, "A Select must have a select string (possibly the empty string)");
-        this.parent = Objects.requireNonNull(query, "A Select must have a parent query");
-        this.groupingRequests = deepCopy(groupingRequests, this);
-    }
 
-    private static List<GroupingRequest> deepCopy(List<GroupingRequest> groupingRequests, Select parentOfCopy) {
-        List<GroupingRequest> copy = new ArrayList<>(groupingRequests.size());
-        for (GroupingRequest request : groupingRequests)
-            copy.add(request.copy(parentOfCopy));
-        return copy;
+    /** Returns the query owning this, never null */
+    private Query getParent() { return parent; }
+
+
+    /** Assigns the query owning this */
+    public void setParent(Query parent) {
+        if (parent==null) throw new NullPointerException("A query models owner cannot be null");
+        this.parent = parent;
     }
 
-    /**
-     * Sets the document selection criterion of the query.
-     *
-     * @param where the documents to select as a JSON string on the format specified in
-     *        <a href="https://docs.vespa.ai/documentation/reference/select-reference.html">the select reference doc</a>
-     */
-    public void setWhereString(String where) {
+
+    /** Set the where-clause for the query. Must be a JSON-string, with the format described in the Select Reference doc - https://docs.vespa.ai/documentation/reference/select-reference.html. */
+    public void setWhere(String where) {
         this.where = where;
-        parent.getModel().setType(SELECT);
+        model.setType(SELECT);
 
-        // This replaces the current query
-        parent.getModel().clearQueryTree();
+        // Setting the queryTree to null
+        model.setQueryString(null);
     }
 
-    /** Returns the where clause string previously assigned, or an empty string if none */
-    public String getWhereString(){ return where; }
 
+    /** Returns the where-clause in the query */
+    public String getWhereString(){
+        return this.where;
+    }
 
-    /**
-     * Sets the grouping operation of the query.
-     *
-     * @param grouping the grouping to perform as a JSON string on the format specified in
-     *        <a href="https://docs.vespa.ai/documentation/reference/select-reference.html">the select reference doc</a>
-     */
-    public void setGroupingString(String grouping) {
-        groupingRequests.clear();
+
+    /** Set the grouping-string for the query. Must be a JSON-string, with the format described in the Select Reference doc - https://docs.vespa.ai/documentation/reference/select-reference.html. */
+    public void setGrouping(String grouping){
         this.grouping = grouping;
         SelectParser parser = (SelectParser) ParserFactory.newInstance(Query.Type.SELECT, new ParserEnvironment());
+
         for (VespaGroupingStep step : parser.getGroupingSteps(grouping)) {
             GroupingRequest.newInstance(parent)
                     .setRootOperation(step.getOperation())
@@ -111,28 +98,13 @@ public class Select implements Cloneable {
 
     /** Returns the grouping in the query */
     public String getGroupingString(){
-        return grouping;
+        return this.grouping;
     }
 
-    /**
-     * Returns the query's {@link GroupingRequest} as a mutable list. Changing this directly changes the grouping
-     * operations which will be performed by this query.
-     */
-    public List<GroupingRequest> getGrouping() { return groupingRequests; }
-
 
     @Override
     public String toString() {
         return "where: [" + where + "], grouping: [" + grouping+ "]";
     }
 
-    @Override
-    public Object clone() {
-        return new Select(where, grouping, parent, groupingRequests);
-    }
-
-    public Select cloneFor(Query parent)  {
-        return new Select(where, grouping, parent, groupingRequests);
-    }
-
 }
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 559a7279f83..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
@@ -3,11 +3,10 @@ package com.yahoo.search.query.properties;
 
 import com.yahoo.processing.request.CompoundName;
 import com.yahoo.search.Query;
-import com.yahoo.search.grouping.GroupingRequest;
-import com.yahoo.search.grouping.vespa.GroupingExecutor;
 import com.yahoo.search.query.*;
 import com.yahoo.search.query.profile.compiled.CompiledQueryProfileRegistry;
 import com.yahoo.search.query.profile.types.FieldDescription;
+import com.yahoo.search.query.profile.types.QueryProfileFieldType;
 import com.yahoo.search.query.profile.types.QueryProfileType;
 import com.yahoo.search.query.ranking.Diversity;
 import com.yahoo.search.query.ranking.MatchPhase;
@@ -15,11 +14,11 @@ import com.yahoo.search.query.ranking.Matching;
 import com.yahoo.search.query.ranking.SoftTimeout;
 import com.yahoo.tensor.Tensor;
 
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
-
-
 /**
  * Maps between the query model and text properties.
  * This can be done simpler by using reflection but the performance penalty was not worth it,
@@ -140,9 +139,7 @@ public class QueryProperties extends Properties {
             if (key.toString().equals(Model.MODEL)) return query.getModel();
             if (key.toString().equals(Ranking.RANKING)) return query.getRanking();
             if (key.toString().equals(Presentation.PRESENTATION)) return query.getPresentation();
-
         }
-
         return super.get(key, context, substitution);
     }
 
@@ -256,9 +253,9 @@ public class QueryProperties extends Properties {
             }
             else if (key.size()==2 && key.first().equals(Select.SELECT)) {
                 if (key.last().equals(Select.WHERE)){
-                    query.getSelect().setWhereString(asString(value, ""));
+                    query.getSelect().setWhere(asString(value, ""));
                 } else if (key.last().equals(Select.GROUPING)) {
-                    query.getSelect().setGroupingString(asString(value, ""));
+                    query.getSelect().setGrouping(asString(value, ""));
                 }
             }
             else if (key.first().equals("rankfeature") || key.first().equals("featureoverride") ) { // featureoverride is deprecated
@@ -280,7 +277,8 @@ public class QueryProperties extends Properties {
                     query.setGroupingSessionCache(asBoolean(value, false));
                 else
                     super.set(key,value,context);
-            } else
+            }
+            else
                 super.set(key,value,context);
         }
         catch (Exception e) { // Make sure error messages are informative. This should be moved out of this properties implementation