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/06/25 04:45:02 UTC
[lucene-solr] branch branch_8x updated: SOLR-12979: fail fast when
collapse field is non-docValued & non-uninvertible
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 438364a SOLR-12979: fail fast when collapse field is non-docValued & non-uninvertible
438364a is described below
commit 438364ab94d937a450ed8bd62bc09eedab915f6e
Author: Munendra S N <mu...@apache.org>
AuthorDate: Tue Jun 25 09:42:10 2019 +0530
SOLR-12979: fail fast when collapse field is non-docValued & non-uninvertible
* Improve error message when collapse field is non-docValued & non-uninvertible.
Return error code 400 instead of 500 in the above case
---
solr/CHANGES.txt | 3 +++
.../solr/search/CollapsingQParserPlugin.java | 8 +++++--
.../solr/search/TestCollapseQParserPlugin.java | 28 +++++++++++++++-------
3 files changed, 28 insertions(+), 11 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 239653f..5b094f1 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -144,6 +144,9 @@ Bug Fixes
* SOLR-13187: Fix NPE when invalid query parser is specified and return 400 error code.
(Cesar Rodriguez, Marek, Charles Sanders, Munendra S N, Mikhail Khludnev)
+* SOLR-12979: Improve error message and change error code to 400 when collapse field is non-docValued and
+ non-uninvertible (hossman, Munendra S N)
+
Other Changes
----------------------
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 c0e9223..c99fc85 100644
--- a/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
+++ b/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
@@ -286,8 +286,12 @@ public class CollapsingQParserPlugin extends QParserPlugin {
// if unknown field, this would fail fast
SchemaField collapseFieldSf = request.getSchema().getField(this.collapseField);
- // collapseFieldSf won't be null
- if (collapseFieldSf.multiValued()) {
+ if (!(collapseFieldSf.isUninvertible() || collapseFieldSf.hasDocValues())) {
+ // uninvertible=false and docvalues=false
+ // field can't be indexed=false and uninvertible=true
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Collapsing field '" + collapseField +
+ "' should be either docValues enabled or indexed with uninvertible enabled");
+ } else if (collapseFieldSf.multiValued()) {
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Collapsing not supported on multivalued fields");
}
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 acdd599..16790c0 100644
--- a/solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java
+++ b/solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java
@@ -814,8 +814,12 @@ public class TestCollapseQParserPlugin extends SolrTestCaseJ4 {
// this is currently ok on fields that don't exist on any docs in the index
for (String f : Arrays.asList("not_indexed_sS", "indexed_s_not_uninvert")) {
for (String hint : Arrays.asList("", " hint=top_fc")) {
- assertQ(req(params("q", "*:*", "fq", "{!collapse field="+f+hint+"}"))
- , "*[count(//doc)=0]");
+ SolrException e = expectThrows(SolrException.class,
+ () -> h.query(req("q", "*:*", "fq", "{!collapse field="+f + hint +"}")));
+ assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, e.code());
+ assertTrue("unexpected Message: " + e.getMessage(),
+ e.getMessage().contains("Collapsing field '" + f + "' " +
+ "should be either docValues enabled or indexed with uninvertible enabled"));
}
}
}
@@ -941,18 +945,24 @@ public class TestCollapseQParserPlugin extends SolrTestCaseJ4 {
req("q","*:*", "fq","{!collapse field=bleh}"), SolrException.ErrorCode.BAD_REQUEST);
// if a field is uninvertible=false, it should behave the same as a field that is indexed=false ...
+ // this also tests docValues=false along with indexed=false or univertible=false
for (String f : Arrays.asList("not_indexed_sS", "indexed_s_not_uninvert")) {
- { // this currently propogates up the low level DocValues error in the common case...
- Exception e = expectThrows(RuntimeException.class, IllegalStateException.class,
+ {
+ SolrException e = expectThrows(SolrException.class,
() -> h.query(req(params("q", "*:*",
"fq", "{!collapse field="+f+"}"))));
+ assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, e.code());
assertTrue("unexpected Message: " + e.getMessage(),
- e.getMessage().contains("Re-index with correct docvalues type"));
+ e.getMessage().contains("Collapsing field '" + f + "' " +
+ "should be either docValues enabled or indexed with uninvertible enabled"));
}
- { // ... but in the case of hint=top_fc a bare NPE gets propogated up (SOLR-12979)...
- expectThrows(RuntimeException.class, NullPointerException.class,
- () -> h.query(req(params("q", "*:*",
- "fq", "{!collapse field="+f+" hint=top_fc}"))));
+ {
+ SolrException e = expectThrows(SolrException.class,
+ () -> h.query(req("q", "*:*", "fq", "{!collapse field="+f+" hint=top_fc}")));
+ assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, e.code());
+ assertTrue("unexpected Message: " + e.getMessage(),
+ e.getMessage().contains("Collapsing field '" + f + "' " +
+ "should be either docValues enabled or indexed with uninvertible enabled"));
}
}