diff options
9 files changed, 81 insertions, 23 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) { diff --git a/processing/src/main/java/com/yahoo/processing/request/Properties.java b/processing/src/main/java/com/yahoo/processing/request/Properties.java index 5a6131e2245..250e7214881 100644 --- a/processing/src/main/java/com/yahoo/processing/request/Properties.java +++ b/processing/src/main/java/com/yahoo/processing/request/Properties.java @@ -212,8 +212,8 @@ public class Properties implements Cloneable { * @throws RuntimeException if no instance in the chain accepted this name-value pair */ public void set(CompoundName name, Object value, Map<String, String> context) { - if (chained == null) throw new RuntimeException("Property '" + name + "->" + value + - "' was not accepted in this property chain"); + if (chained == null) throw new RuntimeException("Property '" + name + "->" + + value + "' was not accepted in this property chain"); chained.set(name, value, context); } |