From 040f40d4497585e45458a907be73e5c251416040 Mon Sep 17 00:00:00 2001 From: David Wayne Smiley Date: Sun, 1 Apr 2012 04:06:20 +0000 Subject: [PATCH] SOLR-435 Improvements to SOLR-2001 (blank 'q') includes empty string check, dismax support, and test git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1308016 13f79535-47bb-0310-9956-ffa450edef68 --- solr/CHANGES.txt | 9 ++--- .../org/apache/solr/search/DisMaxQParser.java | 13 ++++--- .../search/ExtendedDismaxQParserPlugin.java | 2 +- .../solr/search/LuceneQParserPlugin.java | 8 ++--- .../java/org/apache/solr/search/QParser.java | 3 +- .../apache/solr/search/QueryParsingTest.java | 35 +++++++++++++++++++ 6 files changed, 56 insertions(+), 14 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index cdfccb6dfda..2febbc7ae21 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -91,10 +91,6 @@ New Features * SOLR-1665: Add debug component options for timings, results and query info only (gsingers, hossman, yonik) -* SOLR-2001: The query component will substitute an empty query that matches - no documents if the query parser returns null. This also prevents an - exception from being thrown by the default parser if "q" is missing. (yonik) - * SOLR-2112: Solrj API now supports streaming results. (ryan) * SOLR-792: Adding PivotFacetComponent for Hierarchical faceting @@ -602,6 +598,11 @@ New Features the same behaviour as solr 3.5, favouring throughput over latency. More information can be found on the wiki (http://wiki.apache.org/solr/SolrConfigXml) (Greg Bowyer) +* SOLR-2001: The query component will substitute an empty query that matches + no documents if the query parser returns null. This also prevents an + exception from being thrown by the default parser if "q" is missing. (yonik) + SOLR-435: if q is "" then it's also acceptable. (dsmiley, hoss) + Optimizations ---------------------- * SOLR-1931: Speedup for LukeRequestHandler and admin/schema browser. New parameter diff --git a/solr/core/src/java/org/apache/solr/search/DisMaxQParser.java b/solr/core/src/java/org/apache/solr/search/DisMaxQParser.java index 527cd363e3c..60e3a8e16d9 100644 --- a/solr/core/src/java/org/apache/solr/search/DisMaxQParser.java +++ b/solr/core/src/java/org/apache/solr/search/DisMaxQParser.java @@ -22,7 +22,6 @@ import org.apache.lucene.search.BooleanClause; import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.Query; import org.apache.solr.schema.IndexSchema; -import org.apache.solr.common.SolrException; import org.apache.solr.common.params.DisMaxParams; import org.apache.solr.common.params.SolrParams; import org.apache.solr.common.util.NamedList; @@ -92,7 +91,9 @@ public class DisMaxQParser extends QParser { */ BooleanQuery query = new BooleanQuery(true); - addMainQuery(query, solrParams); + boolean notBlank = addMainQuery(query, solrParams); + if (!notBlank) + return null; addBoostQuery(query, solrParams); addBoostFunctions(query, solrParams); @@ -151,7 +152,8 @@ public class DisMaxQParser extends QParser { } } - protected void addMainQuery(BooleanQuery query, SolrParams solrParams) throws ParseException { + /** Adds the main query to the query argument. If its blank then false is returned. */ + protected boolean addMainQuery(BooleanQuery query, SolrParams solrParams) throws ParseException { Map phraseFields = SolrPluginUtils.parseFieldBoosts(solrParams.getParams(DisMaxParams.PF)); float tiebreaker = solrParams.getFloat(DisMaxParams.TIE, 0.0f); @@ -170,6 +172,8 @@ public class DisMaxQParser extends QParser { if (userQuery == null || userQuery.trim().length() < 1) { // If no query is specified, we may have an alternate altUserQuery = getAlternateUserQuery(solrParams); + if (altUserQuery == null) + return false; query.add(altUserQuery, BooleanClause.Occur.MUST); } else { // There is a valid query string @@ -184,6 +188,7 @@ public class DisMaxQParser extends QParser { query.add(phrase, BooleanClause.Occur.SHOULD); } } + return true; } protected Query getAlternateUserQuery(SolrParams solrParams) throws ParseException { @@ -192,7 +197,7 @@ public class DisMaxQParser extends QParser { QParser altQParser = subQuery(altQ, null); return altQParser.getQuery(); } else { - throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "missing query string"); + return null; } } diff --git a/solr/core/src/java/org/apache/solr/search/ExtendedDismaxQParserPlugin.java b/solr/core/src/java/org/apache/solr/search/ExtendedDismaxQParserPlugin.java index f21636f8168..3f94783be1b 100755 --- a/solr/core/src/java/org/apache/solr/search/ExtendedDismaxQParserPlugin.java +++ b/solr/core/src/java/org/apache/solr/search/ExtendedDismaxQParserPlugin.java @@ -172,7 +172,7 @@ class ExtendedDismaxQParser extends QParser { query.add( altUserQuery , BooleanClause.Occur.MUST ); } else { return null; - // throw new SolrException( SolrException.ErrorCode.BAD_REQUEST, "missing query string" ); + // throw new ParseException("missing query string" ); } } else { diff --git a/solr/core/src/java/org/apache/solr/search/LuceneQParserPlugin.java b/solr/core/src/java/org/apache/solr/search/LuceneQParserPlugin.java index 793af1c7e0c..3d6b500dc89 100755 --- a/solr/core/src/java/org/apache/solr/search/LuceneQParserPlugin.java +++ b/solr/core/src/java/org/apache/solr/search/LuceneQParserPlugin.java @@ -19,7 +19,6 @@ package org.apache.solr.search; import org.apache.lucene.queryparser.classic.ParseException; import org.apache.lucene.search.Query; import org.apache.lucene.search.Sort; -import org.apache.solr.common.SolrException; import org.apache.solr.common.params.CommonParams; import org.apache.solr.common.params.SolrParams; import org.apache.solr.common.util.NamedList; @@ -48,7 +47,6 @@ public class LuceneQParserPlugin extends QParserPlugin { } class LuceneQParser extends QParser { - String sortStr; SolrQueryParser lparser; public LuceneQParser(String qstr, SolrParams localParams, SolrParams params, SolrQueryRequest req) { @@ -59,7 +57,7 @@ class LuceneQParser extends QParser { @Override public Query parse() throws ParseException { String qstr = getString(); - if (qstr == null) return null; + if (qstr == null || qstr.length()==0) return null; String defaultField = getParam(CommonParams.DF); if (defaultField==null) { @@ -95,6 +93,8 @@ class OldLuceneQParser extends LuceneQParser { // handle legacy "query;sort" syntax if (getLocalParams() == null) { String qstr = getString(); + if (qstr == null || qstr.length() == 0) + return null; sortStr = getParams().get(CommonParams.SORT); if (sortStr == null) { // sort may be legacy form, included in the query string @@ -107,7 +107,7 @@ class OldLuceneQParser extends LuceneQParser { qstr = commands.get(0); } else if (commands.size() > 2) { - throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "If you want to use multiple ';' in the query, use the 'sort' param."); + throw new ParseException("If you want to use multiple ';' in the query, use the 'sort' param."); } } setString(qstr); diff --git a/solr/core/src/java/org/apache/solr/search/QParser.java b/solr/core/src/java/org/apache/solr/search/QParser.java index a18ad72ab2d..f9c419b45c1 100755 --- a/solr/core/src/java/org/apache/solr/search/QParser.java +++ b/solr/core/src/java/org/apache/solr/search/QParser.java @@ -96,7 +96,8 @@ public abstract class QParser { lst.add(val); } - /** Create and return the Query object represented by qstr + /** Create and return the Query object represented by qstr. Null MAY be returned to signify + * there was no input (e.g. no query string) to parse. * @see #getQuery() **/ public abstract Query parse() throws ParseException; diff --git a/solr/core/src/test/org/apache/solr/search/QueryParsingTest.java b/solr/core/src/test/org/apache/solr/search/QueryParsingTest.java index ff2dc53a4ae..8ac4a908063 100644 --- a/solr/core/src/test/org/apache/solr/search/QueryParsingTest.java +++ b/solr/core/src/test/org/apache/solr/search/QueryParsingTest.java @@ -16,6 +16,7 @@ package org.apache.solr.search; * limitations under the License. */ +import org.apache.lucene.search.Query; import org.apache.lucene.search.Sort; import org.apache.lucene.search.SortField; import org.apache.solr.SolrTestCaseJ4; @@ -35,6 +36,40 @@ public class QueryParsingTest extends SolrTestCaseJ4 { initCore("solrconfig.xml","schema.xml"); } + /** + * Test that the main QParserPlugins people are likely to use + * as defaults fail with a consistent exception when the query string + * is either empty or null. + * @see SOLR-435 + * @see SOLR-2001 + */ + public void testQParserEmptyInput() throws Exception { + + SolrQueryRequest req = req(); + + final String[] parsersTested = new String[] { + OldLuceneQParserPlugin.NAME, + LuceneQParserPlugin.NAME, + DisMaxQParserPlugin.NAME, + ExtendedDismaxQParserPlugin.NAME + }; + + for (String defType : parsersTested) { + for (String qstr : new String[] {null, ""}) { + QParser parser = null; + try { + parser = QParser.getParser(qstr, defType, req); + } catch (Exception e) { + throw new RuntimeException("getParser excep using defType=" + + defType + " with qstr="+qstr, e); + } + + Query q = parser.parse(); + assertNull("expected no query",q); + } + } + } + @Test public void testSort() throws Exception { Sort sort;