You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ho...@apache.org on 2016/01/14 20:54:58 UTC
svn commit: r1724679 - in /lucene/dev/trunk/solr: ./
core/src/java/org/apache/solr/search/ core/src/test/org/apache/solr/
core/src/test/org/apache/solr/request/ core/src/test/org/apache/solr/search/
Author: hossman
Date: Thu Jan 14 19:54:58 2016
New Revision: 1724679
URL: http://svn.apache.org/viewvc?rev=1724679&view=rev
Log:
SOLR-2798: Fixed local params to work correctly with multivalued params
Modified:
lucene/dev/trunk/solr/CHANGES.txt
lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/FunctionQParser.java
lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/QParser.java
lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/QueryParsing.java
lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/SolrReturnFields.java
lucene/dev/trunk/solr/core/src/test/org/apache/solr/DisMaxRequestHandlerTest.java
lucene/dev/trunk/solr/core/src/test/org/apache/solr/request/SimpleFacetsTest.java
lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/QueryEqualityTest.java
lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/QueryParsingTest.java
Modified: lucene/dev/trunk/solr/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/CHANGES.txt?rev=1724679&r1=1724678&r2=1724679&view=diff
==============================================================================
--- lucene/dev/trunk/solr/CHANGES.txt (original)
+++ lucene/dev/trunk/solr/CHANGES.txt Thu Jan 14 19:54:58 2016
@@ -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
----------------------
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/FunctionQParser.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/FunctionQParser.java?rev=1724679&r1=1724678&r2=1724679&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/FunctionQParser.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/FunctionQParser.java Thu Jan 14 19:54:58 2016
@@ -20,13 +20,13 @@ import org.apache.lucene.queries.functio
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 QPa
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;
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/QParser.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/QParser.java?rev=1724679&r1=1724678&r2=1724679&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/QParser.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/QParser.java Thu Jan 14 19:54:58 2016
@@ -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);
}
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/QueryParsing.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/QueryParsing.java?rev=1724679&r1=1724678&r2=1724679&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/QueryParsing.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/QueryParsing.java Thu Jan 14 19:54:58 2016
@@ -32,6 +32,7 @@ import org.apache.lucene.search.Wildcard
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;
}
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/SolrReturnFields.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/SolrReturnFields.java?rev=1724679&r1=1724678&r2=1724679&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/SolrReturnFields.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/SolrReturnFields.java Thu Jan 14 19:54:58 2016
@@ -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.transfor
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 Re
// 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 Re
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) {
Modified: lucene/dev/trunk/solr/core/src/test/org/apache/solr/DisMaxRequestHandlerTest.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/test/org/apache/solr/DisMaxRequestHandlerTest.java?rev=1724679&r1=1724678&r2=1724679&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/test/org/apache/solr/DisMaxRequestHandlerTest.java (original)
+++ lucene/dev/trunk/solr/core/src/test/org/apache/solr/DisMaxRequestHandlerTest.java Thu Jan 14 19:54:58 2016
@@ -100,6 +100,11 @@ public class DisMaxRequestHandlerTest ex
)
,"//*[@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"
Modified: lucene/dev/trunk/solr/core/src/test/org/apache/solr/request/SimpleFacetsTest.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/test/org/apache/solr/request/SimpleFacetsTest.java?rev=1724679&r1=1724678&r2=1724679&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/test/org/apache/solr/request/SimpleFacetsTest.java (original)
+++ lucene/dev/trunk/solr/core/src/test/org/apache/solr/request/SimpleFacetsTest.java Thu Jan 14 19:54:58 2016
@@ -2548,27 +2548,38 @@ public class SimpleFacetsTest extends So
,"*[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]"
Modified: lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/QueryEqualityTest.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/QueryEqualityTest.java?rev=1724679&r1=1724678&r2=1724679&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/QueryEqualityTest.java (original)
+++ lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/QueryEqualityTest.java Thu Jan 14 19:54:58 2016
@@ -354,6 +354,21 @@ public class QueryEqualityTest extends S
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",
Modified: lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/QueryParsingTest.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/QueryParsingTest.java?rev=1724679&r1=1724678&r2=1724679&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/QueryParsingTest.java (original)
+++ lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/QueryParsingTest.java Thu Jan 14 19:54:58 2016
@@ -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 So
}
}
+ 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;