diff --git a/changelog/unreleased/SOLR-8088-grouping-multivalued.yml b/changelog/unreleased/SOLR-8088-grouping-multivalued.yml new file mode 100644 index 000000000000..38f8eae540f1 --- /dev/null +++ b/changelog/unreleased/SOLR-8088-grouping-multivalued.yml @@ -0,0 +1,7 @@ +title: Distributed grouping on a multiValued non-numeric field now fails with a clear 400 instead of an IllegalStateException from Lucene docValues. +type: fixed +authors: + - name: Nick Shanin +links: + - name: SOLR-8088 + url: https://issues.apache.org/jira/browse/SOLR-8088 diff --git a/solr/core/src/java/org/apache/solr/search/grouping/distributed/command/SearchGroupsFieldCommand.java b/solr/core/src/java/org/apache/solr/search/grouping/distributed/command/SearchGroupsFieldCommand.java index c4d98a65a216..e915b14c78eb 100644 --- a/solr/core/src/java/org/apache/solr/search/grouping/distributed/command/SearchGroupsFieldCommand.java +++ b/solr/core/src/java/org/apache/solr/search/grouping/distributed/command/SearchGroupsFieldCommand.java @@ -31,6 +31,7 @@ import org.apache.lucene.search.grouping.ValueSourceGroupSelector; import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.mutable.MutableValue; +import org.apache.solr.common.SolrException; import org.apache.solr.schema.FieldType; import org.apache.solr.schema.SchemaField; import org.apache.solr.search.grouping.Command; @@ -70,10 +71,25 @@ public SearchGroupsFieldCommand build() { throw new IllegalStateException("All fields must be set"); } + checkGroupable(field); return new SearchGroupsFieldCommand(field, groupSort, topNGroups, includeGroupCount); } } + /** + * Term based grouping reads single valued docValues; a multiValued field fails deep inside Lucene + * ("unexpected docvalues type SORTED_SET"), so reject it with a clear message instead. + */ + static void checkGroupable(SchemaField field) { + if (field.getType().getNumberType() == null && field.multiValued()) { + throw new SolrException( + SolrException.ErrorCode.BAD_REQUEST, + "Distributed grouping requires a single valued field, but '" + + field.getName() + + "' is multiValued; group on a single valued (e.g. copyField) field instead"); + } + } + private final SchemaField field; private final Sort groupSort; private final int topNGroups; diff --git a/solr/core/src/java/org/apache/solr/search/grouping/distributed/command/TopGroupsFieldCommand.java b/solr/core/src/java/org/apache/solr/search/grouping/distributed/command/TopGroupsFieldCommand.java index dc9300c93e5f..4199f051cd8a 100644 --- a/solr/core/src/java/org/apache/solr/search/grouping/distributed/command/TopGroupsFieldCommand.java +++ b/solr/core/src/java/org/apache/solr/search/grouping/distributed/command/TopGroupsFieldCommand.java @@ -106,6 +106,7 @@ public TopGroupsFieldCommand build() { throw new IllegalStateException("All required fields must be set"); } + SearchGroupsFieldCommand.checkGroupable(field); return new TopGroupsFieldCommand( query, field, diff --git a/solr/core/src/test/org/apache/solr/TestDistributedGrouping.java b/solr/core/src/test/org/apache/solr/TestDistributedGrouping.java index 28fab47d83b9..13173db55b40 100644 --- a/solr/core/src/test/org/apache/solr/TestDistributedGrouping.java +++ b/solr/core/src/test/org/apache/solr/TestDistributedGrouping.java @@ -21,6 +21,7 @@ import java.util.List; import org.apache.solr.SolrTestCaseJ4.SuppressPointFields; import org.apache.solr.client.solrj.SolrClient; +import org.apache.solr.client.solrj.request.QueryRequest; import org.apache.solr.client.solrj.response.QueryResponse; import org.apache.solr.common.SolrDocumentList; import org.apache.solr.common.SolrException; @@ -1727,6 +1728,22 @@ public void testShardRequestFactory() throws Exception { } } + @Test + public void testMultiValuedGroupFieldIsBadRequest() throws Exception { + index("id", "1", "cat", "a"); + commit(); + + ModifiableSolrParams params = new ModifiableSolrParams(); + params.set("q", "*:*"); + params.set("group", "true"); + params.set("group.field", "cat"); + setDistributedParams(params); + + SolrException e = + expectThrows(SolrException.class, () -> new QueryRequest(params).process(clients.get(0))); + assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, e.code()); + } + private QueryResponse simpleQuery(Object... queryParams) throws Exception { ModifiableSolrParams params = new ModifiableSolrParams(); for (int i = 0; i < queryParams.length; i += 2) { diff --git a/solr/core/src/test/org/apache/solr/search/grouping/distributed/command/SearchGroupsFieldCommandTest.java b/solr/core/src/test/org/apache/solr/search/grouping/distributed/command/SearchGroupsFieldCommandTest.java new file mode 100644 index 000000000000..e8e46626a56d --- /dev/null +++ b/solr/core/src/test/org/apache/solr/search/grouping/distributed/command/SearchGroupsFieldCommandTest.java @@ -0,0 +1,61 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.search.grouping.distributed.command; + +import org.apache.lucene.search.Sort; +import org.apache.solr.SolrTestCaseJ4; +import org.apache.solr.common.SolrException; +import org.apache.solr.schema.SchemaField; +import org.junit.BeforeClass; +import org.junit.Test; + +public class SearchGroupsFieldCommandTest extends SolrTestCaseJ4 { + + @BeforeClass + public static void beforeClass() throws Exception { + initCore("solrconfig.xml", "schema.xml"); + } + + @Test + public void testMultiValuedFieldIsRejected() { + SchemaField cat = h.getCore().getLatestSchema().getField("cat"); + assertTrue(cat.multiValued()); + + SolrException e = + expectThrows( + SolrException.class, + () -> + new SearchGroupsFieldCommand.Builder() + .setField(cat) + .setGroupSort(Sort.RELEVANCE) + .setTopNGroups(10) + .build()); + assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, e.code()); + assertTrue(e.getMessage(), e.getMessage().contains("'cat' is multiValued")); + } + + @Test + public void testSingleValuedFieldIsAccepted() { + SchemaField id = h.getCore().getLatestSchema().getField("id"); + assertFalse(id.multiValued()); + new SearchGroupsFieldCommand.Builder() + .setField(id) + .setGroupSort(Sort.RELEVANCE) + .setTopNGroups(10) + .build(); + } +}