SOLR-2798: Fixed local params to work correctly with multivalued params

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1724679 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Chris M. Hostetter 2016-01-14 19:54:58 +00:00
parent 1e1cdc9935
commit 57d85d8839
9 changed files with 138 additions and 42 deletions

View File

@ -396,6 +396,9 @@ Bug Fixes
* SOLR-6279: cores?action=UNLOAD now waits for the core to close before unregistering it from ZK.
(Christine Poerschke)
* SOLR-2798: Fixed local params to work correctly with multivalued params
(Demian Katz via hossman)
Optimizations
----------------------

View File

@ -20,13 +20,13 @@ import org.apache.lucene.queries.function.FunctionQuery;
import org.apache.lucene.queries.function.ValueSource;
import org.apache.lucene.queries.function.valuesource.*;
import org.apache.lucene.search.Query;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.schema.SchemaField;
import org.apache.solr.search.facet.AggValueSource;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
public class FunctionQParser extends QParser {
@ -252,7 +252,7 @@ public class FunctionQParser extends QParser {
String v = sp.val;
String qs = v;
HashMap nestedLocalParams = new HashMap<String,String>();
ModifiableSolrParams nestedLocalParams = new ModifiableSolrParams();
int end = QueryParsing.parseLocalParams(qs, start, nestedLocalParams, getParams());
QParser sub;

View File

@ -19,7 +19,7 @@ package org.apache.solr.search;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.Sort;
import org.apache.solr.common.params.CommonParams;
import org.apache.solr.common.params.MapSolrParams;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.common.util.StrUtils;
@ -277,16 +277,16 @@ public abstract class QParser {
// SolrParams localParams = QueryParsing.getLocalParams(qstr, req.getParams());
String stringIncludingLocalParams = qstr;
SolrParams localParams = null;
ModifiableSolrParams localParams = null;
SolrParams globalParams = req.getParams();
boolean valFollowedParams = true;
int localParamsEnd = -1;
if (qstr != null && qstr.startsWith(QueryParsing.LOCALPARAM_START)) {
Map<String, String> localMap = new HashMap<>();
localParamsEnd = QueryParsing.parseLocalParams(qstr, 0, localMap, globalParams);
localParams = new ModifiableSolrParams();
localParamsEnd = QueryParsing.parseLocalParams(qstr, 0, localParams, globalParams);
String val = localMap.get(QueryParsing.V);
String val = localParams.get(QueryParsing.V);
if (val != null) {
// val was directly specified in localParams via v=<something> or v=$arg
valFollowedParams = false;
@ -294,9 +294,8 @@ public abstract class QParser {
// use the remainder of the string as the value
valFollowedParams = true;
val = qstr.substring(localParamsEnd);
localMap.put(QueryParsing.V, val);
localParams.set(QueryParsing.V, val);
}
localParams = new MapSolrParams(localMap);
}

View File

@ -32,6 +32,7 @@ import org.apache.lucene.search.WildcardQuery;
import org.apache.lucene.util.BytesRef;
import org.apache.lucene.util.CharsRefBuilder;
import org.apache.solr.common.params.MapSolrParams;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.parser.QueryParser;
import org.apache.solr.schema.FieldType;
@ -85,13 +86,59 @@ public class QueryParsing {
return df != null ? df : s.getDefaultSearchFieldName();
}
// note to self: something needs to detect infinite recursion when parsing queries
/**
* @param txt Text to parse
* @param start Index into text for start of parsing
* @param target Object to inject with parsed settings
* @param params Additional existing parameters
* @deprecated use {@link #parseLocalParams(String, int, ModifiableSolrParams, SolrParams)} instead
*/
@Deprecated
public static int parseLocalParams(String txt, int start, Map<String, String> target, SolrParams params) throws SyntaxError {
return parseLocalParams(txt, start, target, params, LOCALPARAM_START, LOCALPARAM_END);
}
/**
* @param txt Text to parse
* @param start Index into text for start of parsing
* @param target Object to inject with parsed settings
* @param params Additional existing parameters
* @param startString String that indicates the start of a localParams section
* @param endChar Character that indicates the end of a localParams section
* @deprecated use {@link #parseLocalParams(String, int, ModifiableSolrParams, SolrParams, String, char)} instead
*/
@Deprecated
public static int parseLocalParams(String txt, int start, Map<String, String> target, SolrParams params, String startString, char endChar) throws SyntaxError {
ModifiableSolrParams newTarget = new ModifiableSolrParams();
int retVal = parseLocalParams(txt, start, newTarget, params, startString, endChar);
// Translate ModifiableSolrParams to Map<String, String>, implementing "last value wins" for multi-valued params for backward compatibility
for (String param : newTarget.getParameterNames()) {
for (String value : newTarget.getParams(param)) {
target.put(param, value);
}
}
return retVal;
}
/**
* @param txt Text to parse
* @param start Index into text for start of parsing
* @param target Object to inject with parsed settings
* @param params Additional existing parameters
*/
public static int parseLocalParams(String txt, int start, ModifiableSolrParams target, SolrParams params) throws SyntaxError {
return parseLocalParams(txt, start, target, params, LOCALPARAM_START, LOCALPARAM_END);
}
/**
* @param txt Text to parse
* @param start Index into text for start of parsing
* @param target Object to inject with parsed settings
* @param params Additional existing parameters
* @param startString String that indicates the start of a localParams section
* @param endChar Character that indicates the end of a localParams section
*/
public static int parseLocalParams(String txt, int start, ModifiableSolrParams target, SolrParams params, String startString, char endChar) throws SyntaxError {
int off = start;
if (!txt.startsWith(startString, off)) return start;
StrParser p = new StrParser(txt, start, txt.length());
@ -156,7 +203,7 @@ public class QueryParsing {
}
}
}
if (target != null) target.put(id, val);
if (target != null) target.add(id, val);
}
}
@ -197,17 +244,17 @@ public class QueryParsing {
if (txt == null || !txt.startsWith(LOCALPARAM_START)) {
return null;
}
Map<String, String> localParams = new HashMap<>();
ModifiableSolrParams localParams = new ModifiableSolrParams();
int start = QueryParsing.parseLocalParams(txt, 0, localParams, params);
String val = localParams.get(V);
if (val == null) {
val = txt.substring(start);
localParams.put(V, val);
localParams.set(V, val);
} else {
// localParams.put(VAL_EXPLICIT, "true");
}
return new MapSolrParams(localParams);
return localParams;
}

View File

@ -24,7 +24,7 @@ import org.apache.lucene.search.Query;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.SolrException.ErrorCode;
import org.apache.solr.common.params.CommonParams;
import org.apache.solr.common.params.MapSolrParams;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.request.SolrQueryRequest;
@ -36,11 +36,9 @@ import org.apache.solr.response.transform.TransformerFactory;
import org.apache.solr.response.transform.ValueSourceAugmenter;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
/**
@ -248,12 +246,13 @@ public class SolrReturnFields extends ReturnFields {
// This is identical to localParams syntax except it uses [] instead of {!}
if (funcStr.startsWith("[")) {
Map<String,String> augmenterArgs = new HashMap<>();
int end = QueryParsing.parseLocalParams(funcStr, 0, augmenterArgs, req.getParams(), "[", ']');
ModifiableSolrParams augmenterParams = new ModifiableSolrParams();
int end = QueryParsing.parseLocalParams(funcStr, 0, augmenterParams, req.getParams(), "[", ']');
sp.pos += end;
// [foo] is short for [type=foo] in localParams syntax
String augmenterName = augmenterArgs.remove("type");
String augmenterName = augmenterParams.get("type");
augmenterParams.remove("type");
String disp = key;
if( disp == null ) {
disp = '['+augmenterName+']';
@ -261,7 +260,6 @@ public class SolrReturnFields extends ReturnFields {
TransformerFactory factory = req.getCore().getTransformerFactory( augmenterName );
if( factory != null ) {
MapSolrParams augmenterParams = new MapSolrParams( augmenterArgs );
DocTransformer t = factory.create(disp, augmenterParams, req);
if(t!=null) {
if(!_wantsAllFields) {

View File

@ -100,6 +100,11 @@ public class DisMaxRequestHandlerTest extends SolrTestCaseJ4 {
)
,"//*[@numFound='3']"
);
assertQ("multi qf as local params",
req("q", "{!dismax qf=subject qf=features_t}cool")
,"//*[@numFound='3']"
);
assertQ("boost query",
req("q", "cool stuff"

View File

@ -2548,27 +2548,38 @@ public class SimpleFacetsTest extends SolrTestCaseJ4 {
,"*[count(//lst[@name='facet_ranges']/lst[@name='" + field + "']/lst[@name='before'])=0]"
,"*[count(//lst[@name='facet_ranges']/lst[@name='" + field + "']/lst[@name='between'])=0]"
);
assertQ("Test facet.range.other",
// these should have equivilent behavior (multivalued 'other' param: top level vs local)
for (SolrQueryRequest req : new SolrQueryRequest[] {
req("q", "id:[42 TO 47]"
,"facet","true"
,"fl","id," + field
,"facet.range", field
,"facet.range.method", method.toString()
,"facet.range.start","43"
,"facet.range.end","45"
,"facet.range.gap","1"
,"facet.range.other",FacetRangeOther.BEFORE.toString()
,"facet.range.other",FacetRangeOther.AFTER.toString()
)
,"*[count(//lst[@name='facet_ranges']/lst)=1]"
,"*[count(//lst[@name='facet_ranges']/lst[@name='" + field + "'])=1]"
,"*[count(//lst[@name='facet_ranges']/lst[@name='" + field + "']/lst[@name='counts'])=1]"
,"*[count(//lst[@name='facet_ranges']/lst[@name='" + field + "']/lst[@name='counts']/int)=2]"
,"*[count(//lst[@name='facet_ranges']/lst[@name='" + field + "']/lst[@name='between'])=0]"
,"//lst[@name='facet_ranges']/lst[@name='" + field + "']/int[@name='after'][.='3']"
,"//lst[@name='facet_ranges']/lst[@name='" + field + "']/int[@name='before'][.='1']"
);
,"facet","true"
,"fl","id," + field
,"facet.range", field
,"facet.range.method", method.toString()
,"facet.range.start","43"
,"facet.range.end","45"
,"facet.range.gap","1"
,"facet.range.other",FacetRangeOther.BEFORE.toString()
,"facet.range.other",FacetRangeOther.AFTER.toString()),
req("q", "id:[42 TO 47]"
,"facet","true"
,"fl","id," + field
,"facet.range", "{!facet.range.other=before facet.range.other=after}" + field
,"facet.range.method", method.toString()
,"facet.range.start","43"
,"facet.range.end","45"
,"facet.range.gap","1") }) {
assertQ("Test facet.range.other: " + req.toString(), req
,"*[count(//lst[@name='facet_ranges']/lst)=1]"
,"*[count(//lst[@name='facet_ranges']/lst[@name='" + field + "'])=1]"
,"*[count(//lst[@name='facet_ranges']/lst[@name='" + field + "']/lst[@name='counts'])=1]"
,"*[count(//lst[@name='facet_ranges']/lst[@name='" + field + "']/lst[@name='counts']/int)=2]"
,"*[count(//lst[@name='facet_ranges']/lst[@name='" + field + "']/lst[@name='between'])=0]"
,"//lst[@name='facet_ranges']/lst[@name='" + field + "']/int[@name='after'][.='3']"
,"//lst[@name='facet_ranges']/lst[@name='" + field + "']/int[@name='before'][.='1']"
);
}
assertQ("Test facet.range.other",
req("q", "id:[42 TO 47]"

View File

@ -354,6 +354,21 @@ public class QueryEqualityTest extends SolrTestCaseJ4 {
checkQuerySpatial("bbox");
}
public void testLocalParamsWithRepeatingParam() throws Exception {
SolrQueryRequest req = req("q", "foo",
"bq", "111",
"bq", "222");
try {
assertQueryEquals("dismax",
req,
"{!dismax}foo",
"{!dismax bq=111 bq=222}foo",
"{!dismax bq=222 bq=111}foo");
} finally {
req.close();
}
}
private void checkQuerySpatial(final String type) throws Exception {
SolrQueryRequest req = req("myVar", "5",
"d","109",

View File

@ -20,12 +20,14 @@ import org.apache.lucene.search.Query;
import org.apache.lucene.search.Sort;
import org.apache.lucene.search.SortField;
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.SolrException;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.schema.SchemaField;
import org.junit.BeforeClass;
import org.junit.Test;
import java.util.LinkedHashMap;
import java.util.List;
/**
@ -72,6 +74,22 @@ public class QueryParsingTest extends SolrTestCaseJ4 {
}
}
public void testLocalParamsWithLinkedHashMap() throws Exception {
LinkedHashMap<String, String> target = new LinkedHashMap<String, String>();
QueryParsing.parseLocalParams("{!handler foo1=bar1 foo2=bar2 multi=loser multi=winner}", 0, target, new ModifiableSolrParams(), "{!", '}');
assertEquals("bar1", target.get("foo1"));
assertEquals("bar2", target.get("foo2"));
assertEquals("winner", target.get("multi"));
}
public void testLocalParamsWithModifiableSolrParams() throws Exception {
ModifiableSolrParams target = new ModifiableSolrParams();
QueryParsing.parseLocalParams("{!handler foo1=bar1 foo2=bar2 multi=loser multi=winner}", 0, target, new ModifiableSolrParams(), "{!", '}');
assertEquals("bar1", target.get("foo1"));
assertEquals("bar2", target.get("foo2"));
assertArrayEquals(new String[]{"loser", "winner"}, target.getParams("multi"));
}
public void testLiteralFunction() throws Exception {
final String NAME = FunctionQParserPlugin.NAME;