You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by er...@apache.org on 2019/06/04 17:27:15 UTC
[lucene-solr] branch master updated: SOLR-12249: Better error
message when grouping on a tokenized (non SortableText) field in SolrCloud
This is an automated email from the ASF dual-hosted git repository.
erick pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/master by this push:
new 7fb5b7e SOLR-12249: Better error message when grouping on a tokenized (non SortableText) field in SolrCloud
7fb5b7e is described below
commit 7fb5b7ed357b730c93ece574d1e977ecd1268533
Author: erick <er...@gmail.com>
AuthorDate: Tue Jun 4 10:27:06 2019 -0700
SOLR-12249: Better error message when grouping on a tokenized (non SortableText) field in SolrCloud
---
solr/CHANGES.txt | 2 +
.../solr/handler/component/QueryComponent.java | 13 ++
.../test-files/solr/collection1/conf/schema.xml | 10 +-
.../apache/solr/cloud/BasicDistributedZkTest.java | 138 ++++++++++++++++++---
4 files changed, 145 insertions(+), 18 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 9379e99..6f9f0c1 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -130,6 +130,8 @@ Bug Fixes
* SOLR-13510: Intermittent 401's for internode requests with basicauth enabled (Cao Manh Dat, Colvin Cowie)
+* SOLR-12249: Better error message when grouping on a tokenized (non SortableText) field in SolrCloud (Erick Erickson)
+
Other Changes
----------------------
diff --git a/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java b/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java
index fea238b..b0b9fb8 100644
--- a/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java
+++ b/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java
@@ -69,6 +69,7 @@ import org.apache.solr.response.SolrQueryResponse;
import org.apache.solr.schema.FieldType;
import org.apache.solr.schema.IndexSchema;
import org.apache.solr.schema.SchemaField;
+import org.apache.solr.schema.SortableTextField;
import org.apache.solr.search.CursorMark;
import org.apache.solr.search.DocIterator;
import org.apache.solr.search.DocList;
@@ -281,6 +282,18 @@ public class QueryComponent extends SearchComponent
}
groupingSpec.setResponseFormat(responseFormat);
+ // See SOLR-12249. Disallow grouping on text fields that are not SortableText in cloud mode
+ if (req.getCore().getCoreContainer().isZooKeeperAware()) {
+ IndexSchema schema = rb.req.getSchema();
+ String[] fields = params.getParams(GroupParams.GROUP_FIELD);
+ for (String field : fields) {
+ SchemaField schemaField = schema.getField(field);
+ if (schemaField.getType().isTokenized() && (schemaField.getType() instanceof SortableTextField) == false) {
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, String.format(Locale.ROOT, "Sorting on a tokenized field that is not a SortableTextField is not supported in cloud mode."));
+ }
+ }
+ }
+
groupingSpec.setFields(params.getParams(GroupParams.GROUP_FIELD));
groupingSpec.setQueries(params.getParams(GroupParams.GROUP_QUERY));
groupingSpec.setFunctions(params.getParams(GroupParams.GROUP_FUNC));
diff --git a/solr/core/src/test-files/solr/collection1/conf/schema.xml b/solr/core/src/test-files/solr/collection1/conf/schema.xml
index 81c8e33..5dd65bd 100644
--- a/solr/core/src/test-files/solr/collection1/conf/schema.xml
+++ b/solr/core/src/test-files/solr/collection1/conf/schema.xml
@@ -499,6 +499,13 @@
</fieldType>
<fieldType name="severityType" class="${solr.tests.EnumFieldType}" enumsConfig="enumsConfig.xml" enumName="severity"/>
+ <fieldType name="sortable_text" class="solr.SortableTextField">
+ <analyzer>
+ <tokenizer class="solr.WhitespaceTokenizerFactory"/>
+ </analyzer>
+ </fieldType>
+
+
<field name="id" type="string" indexed="true" stored="${solr.tests.id.stored:true}" multiValued="false" required="false" docValues="${solr.tests.id.docValues:false}"/>
<field name="_root_" type="string" indexed="true" stored="true" multiValued="false" required="false"/>
@@ -799,7 +806,8 @@
<dynamicField name="*_ds_p" type="pdouble" indexed="true" stored="true" docValues="true" multiValued="true"/>
<dynamicField name="*_d_ni_p" type="pdouble" indexed="false" stored="true" docValues="true" multiValued="false"/>
<dynamicField name="*_ds_ni_p" type="pdouble" indexed="false" stored="true" docValues="true" multiValued="true"/>
-
+ <dynamicField name="*_sortable" type="sortable_text" indexed="true" multiValued="false" stored="true" />
+
<copyField source="single_i_dvn" dest="copy_single_i_dvn"/>
<copyField source="single_d_dvn" dest="copy_single_d_dvn"/>
<copyField source="single_s_dvn" dest="copy_single_s_dvn"/>
diff --git a/solr/core/src/test/org/apache/solr/cloud/BasicDistributedZkTest.java b/solr/core/src/test/org/apache/solr/cloud/BasicDistributedZkTest.java
index 510c099..98b5a8a 100644
--- a/solr/core/src/test/org/apache/solr/cloud/BasicDistributedZkTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/BasicDistributedZkTest.java
@@ -57,10 +57,15 @@ import org.apache.solr.client.solrj.request.QueryRequest;
import org.apache.solr.client.solrj.request.StreamingUpdateRequest;
import org.apache.solr.client.solrj.request.UpdateRequest;
import org.apache.solr.client.solrj.response.CollectionAdminResponse;
+import org.apache.solr.client.solrj.response.FacetField;
+import org.apache.solr.client.solrj.response.Group;
+import org.apache.solr.client.solrj.response.GroupCommand;
+import org.apache.solr.client.solrj.response.GroupResponse;
import org.apache.solr.client.solrj.response.QueryResponse;
import org.apache.solr.client.solrj.response.UpdateResponse;
import org.apache.solr.cloud.api.collections.OverseerCollectionMessageHandler;
import org.apache.solr.common.SolrDocument;
+import org.apache.solr.common.SolrDocumentList;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.SolrInputDocument;
import org.apache.solr.common.cloud.ClusterState;
@@ -102,6 +107,7 @@ public class BasicDistributedZkTest extends AbstractFullDistribZkTestBase {
String t1="a_t";
String i1="a_i1";
String tlong = "other_tl1";
+ String tsort="t_sortable";
String oddField="oddField_s";
String missingField="ignore_exception__missing_but_valid_field_t";
@@ -216,23 +222,25 @@ public class BasicDistributedZkTest extends AbstractFullDistribZkTestBase {
}
indexr(id,1, i1, 100, tlong, 100,t1,"now is the time for all good men"
- ,"foo_f", 1.414f, "foo_b", "true", "foo_d", 1.414d);
- indexr(id,2, i1, 50 , tlong, 50,t1,"to come to the aid of their country."
- );
- indexr(id,3, i1, 2, tlong, 2,t1,"how now brown cow"
- );
- indexr(id,4, i1, -100 ,tlong, 101,t1,"the quick fox jumped over the lazy dog"
- );
- indexr(id,5, i1, 500, tlong, 500 ,t1,"the quick fox jumped way over the lazy dog"
- );
- indexr(id,6, i1, -600, tlong, 600 ,t1,"humpty dumpy sat on a wall");
- indexr(id,7, i1, 123, tlong, 123 ,t1,"humpty dumpy had a great fall");
- indexr(id,8, i1, 876, tlong, 876,t1,"all the kings horses and all the kings men");
- indexr(id,9, i1, 7, tlong, 7,t1,"couldn't put humpty together again");
- indexr(id,10, i1, 4321, tlong, 4321,t1,"this too shall pass");
- indexr(id,11, i1, -987, tlong, 987,t1,"An eye for eye only ends up making the whole world blind.");
- indexr(id,12, i1, 379, tlong, 379,t1,"Great works are performed, not by strength, but by perseverance.");
- indexr(id,13, i1, 232, tlong, 232,t1,"no eggs on wall, lesson learned", oddField, "odd man out");
+ ,"foo_f", 1.414f, "foo_b", "true", "foo_d", 1.414d, tsort, "now is the time for all good men");
+ indexr(id, 2, i1, 50, tlong, 50, t1, "to come to the aid of their country."
+ , tsort, "to come to the aid of their country.");
+ indexr(id, 3, i1, 2, tlong, 2, t1, "how now brown cow", tsort, "how now brown cow");
+ indexr(id, 4, i1, -100, tlong, 101, t1, "the quick fox jumped over the lazy dog"
+ , tsort, "the quick fox jumped over the lazy dog");
+ indexr(id, 5, i1, 500, tlong, 500, t1, "the quick fox jumped way over the lazy dog"
+ , tsort, "the quick fox jumped over the lazy dog");
+ indexr(id, 6, i1, -600, tlong, 600, t1, "humpty dumpy sat on a wall", tsort, "the quick fox jumped over the lazy dog");
+ indexr(id, 7, i1, 123, tlong, 123, t1, "humpty dumpy had a great fall", tsort, "the quick fox jumped over the lazy dog");
+ indexr(id,8, i1, 876, tlong, 876,t1,"all the kings horses and all the kings men",tsort,"all the kings horses and all the kings men");
+ indexr(id, 9, i1, 7, tlong, 7, t1, "couldn't put humpty together again", tsort, "the quick fox jumped over the lazy dog");
+ indexr(id,10, i1, 4321, tlong, 4321,t1,"this too shall pass",tsort,"this too shall pass");
+ indexr(id,11, i1, -987, tlong, 987,t1,"An eye for eye only ends up making the whole world blind."
+ ,tsort,"An eye for eye only ends up making the whole world blind.");
+ indexr(id,12, i1, 379, tlong, 379,t1,"Great works are performed, not by strength, but by perseverance.",
+ tsort,"Great works are performed, not by strength, but by perseverance.");
+ indexr(id,13, i1, 232, tlong, 232,t1,"no eggs on wall, lesson learned", oddField, "odd man out",
+ tsort,"no eggs on wall, lesson learned");
indexr(id, 14, "SubjectTerms_mfacet", new String[] {"mathematical models", "mathematical analysis"});
indexr(id, 15, "SubjectTerms_mfacet", new String[] {"test 1", "test 2", "test3"});
@@ -248,6 +256,12 @@ public class BasicDistributedZkTest extends AbstractFullDistribZkTestBase {
}
commit();
+
+ testTokenizedGrouping();
+ testSortableTextFaceting();
+ testSortableTextSorting();
+ testSortableTextGrouping();
+
queryAndCompareShards(params("q", "*:*",
"sort", "id desc",
"distrib", "false",
@@ -427,6 +441,96 @@ public class BasicDistributedZkTest extends AbstractFullDistribZkTestBase {
testStopAndStartCoresInOneInstance();
}
+ private void testSortableTextFaceting() throws Exception {
+ SolrQuery query = new SolrQuery("*:*");
+ query.addFacetField(tsort);
+ query.setFacetMissing(false);
+ QueryResponse resp = queryServer(query);
+ List<FacetField> ffs = resp.getFacetFields();
+ for (FacetField ff : ffs) {
+ if (ff.getName().equals(tsort) == false) continue;
+ for (FacetField.Count count : ff.getValues()) {
+ long num = count.getCount();
+ switch (count.getName()) {
+ case "all the kings horses and all the kings men":
+ case "An eye for eye only ends up making the whole world blind.":
+ case "Great works are performed, not by strength, but by perseverance.":
+ case "how now brown cow":
+ case "no eggs on wall, lesson learned":
+ case "now is the time for all good men":
+ case "this too shall pass":
+ case "to come to the aid of their country.":
+ assertEquals("Should have exactly one facet count for field " + ff.getName(), 1, num);
+ break;
+ case "the quick fox jumped over the lazy dog":
+ assertEquals("Should have 5 docs for the lazy dog", 5, num);
+ break;
+ default:
+ fail("No case for facet '" + ff.getName() + "'");
+
+ }
+ }
+ }
+ }
+
+ private void testSortableTextSorting() throws Exception {
+ SolrQuery query = new SolrQuery("*:*");
+ query.addSort(tsort, SolrQuery.ORDER.desc);
+ query.addField("*");
+ query.addField("eoe_sortable");
+ query.addField(tsort);
+ QueryResponse resp = queryServer(query);
+
+ SolrDocumentList docs = resp.getResults();
+
+ String title = docs.get(0).getFieldValue(tsort).toString();
+ for (SolrDocument doc : docs) {
+ assertTrue("Docs should be back in sorted order, descending", title.compareTo(doc.getFieldValue(tsort).toString()) >= 0);
+ title = doc.getFieldValue(tsort).toString();
+ }
+ }
+
+ private void testSortableTextGrouping() throws Exception {
+ SolrQuery query = new SolrQuery("*:*");
+ query.add("group", "true");
+ query.add("group.field", tsort);
+ QueryResponse resp = queryServer(query);
+ GroupResponse groupResp = resp.getGroupResponse();
+ List<GroupCommand> grpCmds = groupResp.getValues();
+ for (GroupCommand grpCmd : grpCmds) {
+ if (grpCmd.getName().equals(tsort) == false) continue;
+ for (Group grp : grpCmd.getValues()) {
+ long count = grp.getResult().getNumFound();
+ if (grp.getGroupValue() == null) continue; // Don't count the groups without an entry as the numnber is variable
+ switch (grp.getGroupValue()) {
+ case "all the kings horses and all the kings men":
+ case "An eye for eye only ends up making the whole world blind.":
+ case "Great works are performed, not by strength, but by perseverance.":
+ case "how now brown cow":
+ case "no eggs on wall, lesson learned":
+ case "now is the time for all good men":
+ case "this too shall pass":
+ case "to come to the aid of their country.":
+ assertEquals("Should have exactly one facet count for field " + grpCmd.getName(), 1, count);
+ break;
+ case "the quick fox jumped over the lazy dog":
+ assertEquals("Should have 5 docs for the lazy dog", 5, count);
+ break;
+ default:
+ fail("No case for facet '" + grpCmd.getName() + "'");
+
+ }
+ }
+ }
+ }
+
+ private void testTokenizedGrouping() throws Exception {
+ SolrException ex = expectThrows(SolrException.class, () -> {
+ query(false, new String[]{"q", "*:*", "group", "true", "group.field", t1});
+ });
+ assertTrue("Expected error from server that SortableTextFields are required", ex.getMessage().contains("Sorting on a tokenized field that is not a SortableTextField is not supported in cloud mode"));
+ }
+
private void assertSliceCounts(String msg, long expected, DocCollection dColl) throws Exception {
long found = checkSlicesSameCounts(dColl);