Allow old style select with query profile types

8 files changed, 79 insertions, 21 deletions

diff --git a/container-search/src/main/java/com/yahoo/search/Query.java b/container-search/src/main/java/com/yahoo/search/Query.java
index ca52c517bbf..84827a46572 100644
--- a/container-search/src/main/java/com/yahoo/search/Query.java
+++ b/container-search/src/main/java/com/yahoo/search/Query.java
@@ -346,7 +346,7 @@ public class Query extends com.yahoo.processing.Request implements Cloneable {
             Properties queryProfileProperties = new QueryProfileProperties(queryProfile);
             properties().chain(queryProfileProperties);
             // TODO: Just checking legality rather than actually setting would be faster
-            setPropertiesFromRequestMap(requestMap, properties()); // Adds errors to the query for illegal set attempts
+            setPropertiesFromRequestMap(requestMap, properties(), true); // Adds errors to the query for illegal set attempts
 
             // Create the full chain
             properties().chain(new QueryProperties(this, queryProfile.getRegistry())).
@@ -358,13 +358,20 @@ public class Query extends com.yahoo.processing.Request implements Cloneable {
             // Pass the values from the query profile which maps through a field in the Query object model
             // through the property chain to cause those values to be set in the Query object model
             setFieldsFrom(queryProfileProperties, requestMap);
+
+            // We need special handling for "select" because it can be both the prefix of the nested JSON select
+            // parameters, and a plain select expression. The latter will be disallowed by query profile types
+            // since they contain the former.
+            String select = requestMap.get(Select.SELECT);
+            if (select != null)
+                properties().set(Select.SELECT, select);
         }
         else { // bypass these complications if there is no query profile to get values from and validate against
             properties().
                     chain(new QueryProperties(this, CompiledQueryProfileRegistry.empty)).
                     chain(new PropertyMap()).
                     chain(new DefaultProperties());
-            setPropertiesFromRequestMap(requestMap, properties());
+            setPropertiesFromRequestMap(requestMap, properties(), false);
         }
 
         properties().setParentQuery(this);
@@ -434,9 +441,10 @@ public class Query extends com.yahoo.processing.Request implements Cloneable {
     }
 
     /** Calls properties.set on all entries in requestMap */
-    private void setPropertiesFromRequestMap(Map<String, String> requestMap, Properties properties) {
+    private void setPropertiesFromRequestMap(Map<String, String> requestMap, Properties properties, boolean ignoreSelect) {
         for (var entry : requestMap.entrySet()) {
             try {
+                if (ignoreSelect && entry.getKey().equals(Select.SELECT)) continue;
                 properties.set(entry.getKey(), entry.getValue(), requestMap);
             }
             catch (IllegalArgumentException e) {
diff --git a/container-search/src/main/java/com/yahoo/search/grouping/GroupingQueryParser.java b/container-search/src/main/java/com/yahoo/search/grouping/GroupingQueryParser.java
index 94d2340ebee..9924a05bb46 100644
--- a/container-search/src/main/java/com/yahoo/search/grouping/GroupingQueryParser.java
+++ b/container-search/src/main/java/com/yahoo/search/grouping/GroupingQueryParser.java
@@ -9,6 +9,7 @@ import com.yahoo.search.Query;
 import com.yahoo.search.Result;
 import com.yahoo.search.Searcher;
 import com.yahoo.search.grouping.request.GroupingOperation;
+import com.yahoo.search.query.Select;
 import com.yahoo.search.searchchain.Execution;
 import com.yahoo.search.searchchain.PhaseNames;
 
@@ -28,7 +29,7 @@ public class GroupingQueryParser extends Searcher {
 
     public static final String SELECT_PARAMETER_PARSING = "SelectParameterParsing";
     public static final CompoundName PARAM_CONTINUE = new CompoundName("continue");
-    public static final CompoundName PARAM_REQUEST = new CompoundName("select");
+    public static final CompoundName PARAM_REQUEST = new CompoundName(Select.SELECT);
     public static final CompoundName PARAM_TIMEZONE = new CompoundName("timezone");
     private static final ThreadLocal<ZoneCache> zoneCache = new ThreadLocal<>();
 
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 f2a534a014e..cb662dcd671 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
@@ -37,6 +37,7 @@ public class Select implements Cloneable {
 
     private String where;
     private String grouping;
+    private String groupingExpressionString;
 
     static {
         argumentType = new QueryProfileType(SELECT);
@@ -107,6 +108,15 @@ public class Select implements Cloneable {
         }
     }
 
+    /**
+     * Sets the grouping expression string directly.
+     * This will not be parsed by this but will be accessed later by GroupingQueryParser.
+     */
+    public void setGroupingExpressionString(String groupingExpressionString) {
+        this.groupingExpressionString = groupingExpressionString;
+    }
+
+    public String getGroupingExpressionString() { return groupingExpressionString; }
 
     /** Returns the grouping in the query */
     public String getGroupingString(){
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 7540f06266c..83e6e122a8a 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
@@ -210,7 +210,7 @@ public class SelectParser implements Parser {
         List<String> operations = new ArrayList<>();
         Inspector inspector = SlimeUtils.jsonToSlime(grouping.getBytes()).get();
         if (inspector.field("error_message").valid()){
-            throw new QueryException("Illegal query: "+inspector.field("error_message").asString() +
+            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/profile/QueryProfileProperties.java b/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileProperties.java
index 3e1f664cf87..ea79b10d779 100644
--- a/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileProperties.java
+++ b/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileProperties.java
@@ -45,7 +45,7 @@ public class QueryProfileProperties extends Properties {
 
     /** Gets a value from the query profile, or from the nested profile if the value is null */
     @Override
-    public Object get(CompoundName name, Map<String,String> context,
+    public Object get(CompoundName name, Map<String, String> context,
                       com.yahoo.processing.request.Properties substitution) {
         name = unalias(name, context);
         Object value = null;
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 71752f6ad87..4cdd4488f7b 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
@@ -101,17 +101,22 @@ public class QueryProperties extends Properties {
                 if (key.last().equals(Matching.MINHITSPERTHREAD)) return matching.getMinHitsPerThread();
 
             }
-            else if (key.size()>2) {
+            else if (key.size() > 2) {
                 // pass the portion after "ranking.features/properties" down
                 if (key.get(1).equals(Ranking.FEATURES)) return ranking.getFeatures().getObject(key.rest().rest().toString());
                 if (key.get(1).equals(Ranking.PROPERTIES)) return ranking.getProperties().get(key.rest().rest().toString());
             }
         }
-        else if (key.size()==2 && key.first().equals(Select.SELECT)) {
-            if (key.last().equals(Select.WHERE)) return query.getSelect().getWhereString();
-            if (key.last().equals(Select.GROUPING)) return query.getSelect().getGroupingString();
+        else if (key.first().equals(Select.SELECT)) {
+            if (key.size() == 1) {
+                return query.getSelect().getGroupingExpressionString();
+            }
+            else if (key.size() == 2) {
+                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)) {
+        else if (key.size() == 2 && key.first().equals(Presentation.PRESENTATION)) {
             if (key.last().equals(Presentation.BOLDING)) return query.getPresentation().getBolding();
             if (key.last().equals(Presentation.SUMMARY)) return query.getPresentation().getSummary();
             if (key.last().equals(Presentation.FORMAT)) return query.getPresentation().getFormat();
@@ -144,7 +149,7 @@ public class QueryProperties extends Properties {
     public void set(CompoundName key, Object value, Map<String,String> context) {
         // Note: The defaults here are never used
         try {
-            if (key.size()==2 && key.first().equals(Model.MODEL)) {
+            if (key.size() == 2 && key.first().equals(Model.MODEL)) {
                 Model model = query.getModel();
                 if (key.last().equals(Model.QUERY_STRING))
                     model.setQueryString(asString(value, ""));
@@ -171,7 +176,7 @@ public class QueryProperties extends Properties {
             }
             else if (key.first().equals(Ranking.RANKING)) {
                 Ranking ranking = query.getRanking();
-                if (key.size()==2) {
+                if (key.size() == 2) {
                     if (key.last().equals(Ranking.LOCATION))
                         ranking.setLocation(asString(value,""));
                     else if (key.last().equals(Ranking.PROFILE))
@@ -225,7 +230,7 @@ public class QueryProperties extends Properties {
                     if (key.last().equals(Matching.NUMSEARCHPARTITIIONS)) matching.setNumSearchPartitions(asInteger(value, 1));
                     if (key.last().equals(Matching.MINHITSPERTHREAD)) matching.setMinHitsPerThread(asInteger(value, 0));
                 }
-                else if (key.size()>2) {
+                else if (key.size() > 2) {
                     String restKey = key.rest().rest().toString();
                     if (key.get(1).equals(Ranking.FEATURES))
                         setRankingFeature(query, restKey, toSpecifiedType(restKey, value, profileRegistry.getTypeRegistry().getComponent("features")));
@@ -235,7 +240,7 @@ public class QueryProperties extends Properties {
                         throwIllegalParameter(key.rest().toString(),Ranking.RANKING);
                 }
             }
-            else if (key.size()==2 && key.first().equals(Presentation.PRESENTATION)) {
+            else if (key.size() == 2 && key.first().equals(Presentation.PRESENTATION)) {
                 if (key.last().equals(Presentation.BOLDING))
                     query.getPresentation().setBolding(asBoolean(value, true));
                 else if (key.last().equals(Presentation.SUMMARY))
@@ -249,11 +254,19 @@ public class QueryProperties extends Properties {
                 else
                     throwIllegalParameter(key.last(), Presentation.PRESENTATION);
             }
-            else if (key.size()==2 && key.first().equals(Select.SELECT)) {
-                if (key.last().equals(Select.WHERE)){
-                    query.getSelect().setWhereString(asString(value, ""));
-                } else if (key.last().equals(Select.GROUPING)) {
-                    query.getSelect().setGroupingString(asString(value, ""));
+            else if (key.first().equals(Select.SELECT)) {
+                if (key.size() == 1) {
+                    query.getSelect().setGroupingExpressionString(asString(value, ""));
+                }
+                else if (key.size() == 2) {
+                    if (key.last().equals(Select.WHERE)) {
+                        query.getSelect().setWhereString(asString(value, ""));
+                    } else if (key.last().equals(Select.GROUPING)) {
+                        query.getSelect().setGroupingString(asString(value, ""));
+                    }
+                }
+                else {
+                    throwIllegalParameter(key.last(), Select.SELECT);
                 }
             }
             else if (key.first().equals("rankfeature") || key.first().equals("featureoverride") ) { // featureoverride is deprecated
diff --git a/container-search/src/test/java/com/yahoo/search/querytransform/test/SortingDegraderTestCase.java b/container-search/src/test/java/com/yahoo/search/querytransform/test/SortingDegraderTestCase.java
index fd6f4c42d1d..5a14cff1818 100644
--- a/container-search/src/test/java/com/yahoo/search/querytransform/test/SortingDegraderTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/querytransform/test/SortingDegraderTestCase.java
@@ -84,6 +84,7 @@ public class SortingDegraderTestCase {
         Query query = new Query("?ranking.sorting=%2ba1%20-a2&select=all(group(a1)%20each(output(a1)))");
         execute(query);
         assertNull(query.getRanking().getMatchPhase().getAttribute());
+
     }
 
     @Test
diff --git a/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java b/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java
index ac9c4e48eb3..fb628156aab 100644
--- a/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java
@@ -30,6 +30,8 @@ import com.yahoo.processing.request.CompoundName;
 import com.yahoo.search.Query;
 import com.yahoo.search.Result;
 import com.yahoo.search.Searcher;
+import com.yahoo.search.grouping.GroupingQueryParser;
+import com.yahoo.search.grouping.GroupingRequest;
 import com.yahoo.search.query.QueryTree;
 import com.yahoo.search.query.SessionId;
 import com.yahoo.search.query.profile.QueryProfile;
@@ -935,16 +937,39 @@ public class QueryTestCase {
     }
 
     @Test
-    public void testGroupingAndQueryProfileType() {
+    public void testOldStyleSelect() {
+        // The same as testOldStyleSelectAndNativeQueryProfileType but not inheriting native
+        QueryProfileRegistry registry = new QueryProfileRegistry();
         QueryProfileType type = new QueryProfileType("mytype");
         QueryProfile profile = new QueryProfile("default");
         profile.setType(type);
+        registry.register(profile);
+        registry.getTypeRegistry().register(type);
+        CompiledQueryProfileRegistry cRegistry = registry.compile();
+        Query query = new Query(httpEncode("?query=sddocname:sentence&select=all(group(context_id) max(10) each(each(output(summary()))))"),
+                                cRegistry.findQueryProfile("default"));
+        GroupingQueryParser parser = new GroupingQueryParser();
+        parser.search(query, new Execution(parser, Execution.Context.createContextStub()));
+        assertEquals("[all(group(context_id) max(10) each(each(output(summary())))), all(group(context_id) max(10) each(each(output(summary()))))]",
+                     query.getSelect().getGrouping().toString());
+    }
+
+    @Test
+    public void testOldStyleSelectAndNativeQueryProfileType() {
         QueryProfileRegistry registry = new QueryProfileRegistry();
+        QueryProfileType type = new QueryProfileType("mytype");
+        type.inherited().add(registry.getType("native"));
+        QueryProfile profile = new QueryProfile("default");
+        profile.setType(type);
         registry.register(profile);
         registry.getTypeRegistry().register(type);
         CompiledQueryProfileRegistry cRegistry = registry.compile();
         Query query = new Query(httpEncode("?query=sddocname:sentence&select=all(group(context_id) max(10) each(each(output(summary()))))"),
                                 cRegistry.findQueryProfile("default"));
+        GroupingQueryParser parser = new GroupingQueryParser();
+        parser.search(query, new Execution(parser, Execution.Context.createContextStub()));
+        assertEquals("[all(group(context_id) max(10) each(each(output(summary())))), all(group(context_id) max(10) each(each(output(summary()))))]",
+                     query.getSelect().getGrouping().toString());
     }
 
     private void assertDetectionText(String expectedDetectionText, String queryString, String ... indexSpecs) {