You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by mu...@apache.org on 2019/12/12 05:02:47 UTC
[lucene-solr] branch branch_8x updated: SOLR-13970: disallow using
collapse/expand with grouping
This is an automated email from the ASF dual-hosted git repository.
munendrasn pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/branch_8x by this push:
new 5765912 SOLR-13970: disallow using collapse/expand with grouping
5765912 is described below
commit 576591205c2c386c2ae052477bc41ac18c0181d6
Author: Munendra S N <mu...@apache.org>
AuthorDate: Thu Dec 12 10:06:50 2019 +0530
SOLR-13970: disallow using collapse/expand with grouping
* Using collapse with grouping would cause inconsistent behavior.
This is because grouping calls the same postfilter twice without
resetting the internal state of the DocValues cache
* Using expand with grouping would cause NPE
---
solr/CHANGES.txt | 5 +++++
.../apache/solr/handler/component/ExpandComponent.java | 4 ++++
.../apache/solr/search/CollapsingQParserPlugin.java | 6 ++++++
.../solr/handler/component/TestExpandComponent.java | 9 ++++++++-
.../apache/solr/search/TestCollapseQParserPlugin.java | 18 ++++++++----------
.../src/collapse-and-expand-results.adoc | 2 +-
6 files changed, 32 insertions(+), 12 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 869d966..f188311 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -51,6 +51,9 @@ Upgrade Notes
* SOLR-13904: timeAllowed parameter is allowed to have 0 value (Houston Putman, Mikhail Khludnev)
+* SOLR-13970: Using Collapse filter or expand component with grouping is explicitly disallowed as the combination
+ would cause inconsistent behavior and NPEs.
+
New Features
---------------------
* SOLR-13821: A Package store to store and load package artifacts (noble, Ishan Chattopadhyaya)
@@ -117,6 +120,8 @@ Improvements
* SOLR-13904: Analytic component abandons requests exceedig a limit passed via timeAllowed parameter.
(Houston Putman, Mikhail Khludnev).
+* SOLR-13970: Fail the request when collapsing or expand is used with Grouping. (Erick Erickson, Joel Bernstein, Munendra S N)
+
Optimizations
---------------------
(No changes)
diff --git a/solr/core/src/java/org/apache/solr/handler/component/ExpandComponent.java b/solr/core/src/java/org/apache/solr/handler/component/ExpandComponent.java
index 0440a3e..8cee08c 100644
--- a/solr/core/src/java/org/apache/solr/handler/component/ExpandComponent.java
+++ b/solr/core/src/java/org/apache/solr/handler/component/ExpandComponent.java
@@ -63,6 +63,7 @@ import org.apache.lucene.util.LongValues;
import org.apache.solr.common.SolrDocumentList;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.params.ExpandParams;
+import org.apache.solr.common.params.GroupParams;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.common.util.SimpleOrderedMap;
@@ -113,6 +114,9 @@ public class ExpandComponent extends SearchComponent implements PluginInfoInitia
@Override
public void prepare(ResponseBuilder rb) throws IOException {
if (rb.req.getParams().getBool(ExpandParams.EXPAND, false)) {
+ if (rb.req.getParams().getBool(GroupParams.GROUP, false)) {
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Can not use expand with Grouping enabled");
+ }
rb.doExpand = true;
}
}
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 b63c119..fc25c3c 100644
--- a/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
+++ b/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
@@ -65,6 +65,7 @@ import org.apache.lucene.util.BytesRef;
import org.apache.lucene.util.FixedBitSet;
import org.apache.lucene.util.LongValues;
import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.GroupParams;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.handler.component.QueryElevationComponent;
@@ -279,6 +280,11 @@ public class CollapsingQParserPlugin extends QParserPlugin {
}
public CollapsingPostFilter(SolrParams localParams, SolrParams params, SolrQueryRequest request) {
+ // Don't allow collapsing if grouping is being used.
+ if (request.getParams().getBool(GroupParams.GROUP, false)) {
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Can not use collapse with Grouping enabled");
+ }
+
this.collapseField = localParams.get("field");
if (this.collapseField == null) {
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Required 'field' param is missing.");
diff --git a/solr/core/src/test/org/apache/solr/handler/component/TestExpandComponent.java b/solr/core/src/test/org/apache/solr/handler/component/TestExpandComponent.java
index 690ad60..7adc942 100644
--- a/solr/core/src/test/org/apache/solr/handler/component/TestExpandComponent.java
+++ b/solr/core/src/test/org/apache/solr/handler/component/TestExpandComponent.java
@@ -392,8 +392,15 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
ignoreException("Can't determine a Sort Order");
ignoreException("Expand not supported for fieldType:'text'");
+ // expand with grouping
+ SolrException e = expectThrows(SolrException.class, () -> {
+ h.query(req("q", "*:*", "expand", "true", "expand.field", "id", "group", "true", "group.field", "id"));
+ });
+ assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, e.code());
+ assertEquals("Can not use expand with Grouping enabled", e.getMessage());
+
// no expand field
- SolrException e = expectThrows(SolrException.class, () -> h.query(req("q", "*:*", "expand", "true")));
+ e = expectThrows(SolrException.class, () -> h.query(req("q", "*:*", "expand", "true")));
assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, e.code());
assertEquals("missing expand field", e.getMessage());
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 2753542..e7dd357 100644
--- a/solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java
+++ b/solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java
@@ -33,6 +33,8 @@ import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
+import static org.hamcrest.core.StringContains.containsString;
+
public class TestCollapseQParserPlugin extends SolrTestCaseJ4 {
@BeforeClass
public static void beforeClass() throws Exception {
@@ -206,7 +208,6 @@ public class TestCollapseQParserPlugin extends SolrTestCaseJ4 {
}
@Test
- @AwaitsFix(bugUrl="https://issues.apache.org/jira/browse/SOLR-11974")
public void testStringCollapse() throws Exception {
for (final String hint : new String[] {"", " hint="+CollapsingQParserPlugin.HINT_TOP_FC}) {
testCollapseQueries("group_s", hint, false);
@@ -759,14 +760,12 @@ public class TestCollapseQParserPlugin extends SolrTestCaseJ4 {
params.add("facet.mincount", "1");
assertQ(req(params), "*[count(//doc)=1]", "*[count(//lst[@name='facet_fields']/lst[@name='test_i']/int)=2]");
- // SOLR-5230 - ensure CollapsingFieldValueCollector.finish() is called
- params = new ModifiableSolrParams();
- params.add("q", "*:*");
- params.add("fq", "{!collapse field="+group+hint+"}");
- params.add("group", "true");
- params.add("group.field", "id");
- assertQ(req(params), "*[count(//doc)=2]");
-
+ // SOLR-13970
+ SolrException ex = expectThrows(SolrException.class, () -> {
+ h.query(req(params("q", "*:*", "fq", "{!collapse field="+group+hint+"}", "group", "true", "group.field", "id")));
+ });
+ assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, ex.code());
+ assertThat(ex.getMessage(), containsString("Can not use collapse with Grouping enabled"));
// delete the elevated docs, confirm collapsing still works
assertU(delI("1"));
@@ -784,7 +783,6 @@ public class TestCollapseQParserPlugin extends SolrTestCaseJ4 {
"//result/doc[2]/str[@name='id'][.='6']",
"//result/doc[3]/str[@name='id'][.='7']");
-
}
@Test
diff --git a/solr/solr-ref-guide/src/collapse-and-expand-results.adoc b/solr/solr-ref-guide/src/collapse-and-expand-results.adoc
index 6eb4e1b..598bf5b 100644
--- a/solr/solr-ref-guide/src/collapse-and-expand-results.adoc
+++ b/solr/solr-ref-guide/src/collapse-and-expand-results.adoc
@@ -18,7 +18,7 @@
The Collapsing query parser and the Expand component combine to form an approach to grouping documents for field collapsing in search results.
-The Collapsing query parser groups documents (collapsing the result set) according to your parameters, while the Expand component provides access to documents in the collapsed group for use in results display or other processing by a client application. Collapse & Expand can together do what the older <<result-grouping.adoc#result-grouping,Result Grouping>> (`group=true`) does for _most_ use-cases but not all. Generally, you should prefer Collapse & Expand.
+The Collapsing query parser groups documents (collapsing the result set) according to your parameters, while the Expand component provides access to documents in the collapsed group for use in results display or other processing by a client application. Collapse & Expand can together do what the older <<result-grouping.adoc#result-grouping,Result Grouping>> (`group=true`) does for _most_ use-cases but not all. Collapse and Expand are not supported when Result Grouping is enabled. General [...]
[IMPORTANT]
====