You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by da...@apache.org on 2018/09/20 23:59:14 UTC

[08/29] lucene-solr:jira/http2: SOLR-6280: CollapseQParser now throws an error when pointing to a multi-valued field.

SOLR-6280: CollapseQParser now throws an error when pointing to a multi-valued field.


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/ac7969e3
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/ac7969e3
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/ac7969e3

Branch: refs/heads/jira/http2
Commit: ac7969e3c05cf9db28dbe52d0911d64a864d8c97
Parents: 4940b36
Author: David Smiley <ds...@apache.org>
Authored: Mon Sep 17 09:08:54 2018 -0400
Committer: David Smiley <ds...@apache.org>
Committed: Mon Sep 17 09:08:54 2018 -0400

----------------------------------------------------------------------
 solr/CHANGES.txt                                    |  6 ++++++
 .../apache/solr/search/CollapsingQParserPlugin.java | 15 +++++++++++++--
 .../org/apache/solr/search/QueryEqualityTest.java   |  2 +-
 .../solr/search/TestCollapseQParserPlugin.java      | 16 ++++++++++++++++
 4 files changed, 36 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ac7969e3/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 8873834..e773904 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -83,6 +83,12 @@ Velocity 1.7 and Velocity Tools 2.0
 Apache ZooKeeper 3.4.11
 Jetty 9.4.11.v20180605
 
+New Features
+----------------------
+
+* SOLR-6280: {!collapse}: if you attempt to use CollapseQParser on a field that is multi-valued, you will now get an
+  error.  Previously, the collapsing behavior was unreliable and undefined despite no explicit error.
+  (Munendra S N, David Smiley)
 
 Other Changes
 ----------------------

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ac7969e3/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java b/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
index 08867db..13417ab 100644
--- a/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
+++ b/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
@@ -73,6 +73,7 @@ import org.apache.solr.request.SolrQueryRequest;
 import org.apache.solr.request.SolrRequestInfo;
 import org.apache.solr.schema.FieldType;
 import org.apache.solr.schema.NumberType;
+import org.apache.solr.schema.SchemaField;
 import org.apache.solr.schema.StrField;
 import org.apache.solr.uninverting.UninvertingReader;
 
@@ -270,12 +271,19 @@ public class CollapsingQParserPlugin extends QParserPlugin {
       return s;
     }
 
-    public CollapsingPostFilter(SolrParams localParams, SolrParams params, SolrQueryRequest request) throws IOException {
+    public CollapsingPostFilter(SolrParams localParams, SolrParams params, SolrQueryRequest request) {
       this.collapseField = localParams.get("field");
       if (this.collapseField == null) {
         throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Required 'field' param is missing.");
       }
 
+      // if unknown field, this would fail fast
+      SchemaField collapseFieldSf = request.getSchema().getField(this.collapseField);
+      // collapseFieldSf won't be null
+      if (collapseFieldSf.multiValued()) {
+        throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Collapsing not supported on multivalued fields");
+      }
+
       this.groupHeadSelector = GroupHeadSelector.build(localParams);
       
       if (groupHeadSelector.type.equals(GroupHeadSelectorType.SORT) &&
@@ -334,7 +342,7 @@ public class CollapsingQParserPlugin extends QParserPlugin {
       } else if(nPolicy.equals((NULL_EXPAND))) {
         this.nullPolicy = NULL_POLICY_EXPAND;
       } else {
-        throw new IOException("Invalid nullPolicy:"+nPolicy);
+        throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Invalid nullPolicy:"+nPolicy);
       }
     }
 
@@ -369,6 +377,9 @@ public class CollapsingQParserPlugin extends QParserPlugin {
                                              boostDocsMap,
                                              searcher);
 
+      } catch (SolrException e) {
+        // handle SolrException separately
+        throw e;
       } catch (Exception e) {
         throw new RuntimeException(e);
       }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ac7969e3/solr/core/src/test/org/apache/solr/search/QueryEqualityTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/search/QueryEqualityTest.java b/solr/core/src/test/org/apache/solr/search/QueryEqualityTest.java
index d39a42f..be7fe91 100644
--- a/solr/core/src/test/org/apache/solr/search/QueryEqualityTest.java
+++ b/solr/core/src/test/org/apache/solr/search/QueryEqualityTest.java
@@ -275,7 +275,7 @@ public class QueryEqualityTest extends SolrTestCaseJ4 {
   }
 
   public void testQueryCollapse() throws Exception {
-    SolrQueryRequest req = req("myField","foo_s",
+    SolrQueryRequest req = req("myField","foo_s1",
                                "g_sort","foo_s1 asc, foo_i desc");
 
     try {

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ac7969e3/solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java b/solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java
index d0521d3..9f65ba5 100644
--- a/solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java
+++ b/solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java
@@ -920,6 +920,22 @@ public class TestCollapseQParserPlugin extends SolrTestCaseJ4 {
   }
 
   @Test
+  public void testForNotSupportedCases() {
+    String[] doc = {"id","3", "term_s", "YYYY", "test_ii", "5000", "test_l", "100", "test_f", "200"};
+    assertU(adoc(doc));
+    assertU(commit());
+
+    // collapsing on multivalued field
+    assertQEx("Should Fail with Bad Request", "Collapsing not supported on multivalued fields",
+        req("q","*:*", "fq","{!collapse field=test_ii}"), SolrException.ErrorCode.BAD_REQUEST);
+
+    // collapsing on unknown field
+    assertQEx("Should Fail with Bad Request", "org.apache.solr.search.SyntaxError: undefined field: \"bleh\"",
+        req("q","*:*", "fq","{!collapse field=bleh}"), SolrException.ErrorCode.BAD_REQUEST);
+
+  }
+
+  @Test
   public void test64BitCollapseFieldException() {
     ModifiableSolrParams doubleParams = new ModifiableSolrParams();
     doubleParams.add("q", "*:*");