diff --git a/changelog/unreleased/SOLR-7520-grouping-postfilter-complete.yml b/changelog/unreleased/SOLR-7520-grouping-postfilter-complete.yml new file mode 100644 index 000000000000..8007c6d5d80d --- /dev/null +++ b/changelog/unreleased/SOLR-7520-grouping-postfilter-complete.yml @@ -0,0 +1,9 @@ +title: > + A post filter collector is now completed on shards when a distributed grouping request uses it, + so its complete() results (for example an AnalyticsQuery) are included in the shard response. +type: fixed +authors: + - name: Nick Shanin +links: + - name: SOLR-7520 + url: https://issues.apache.org/jira/browse/SOLR-7520 diff --git a/solr/core/src/java/org/apache/solr/search/grouping/CommandHandler.java b/solr/core/src/java/org/apache/solr/search/grouping/CommandHandler.java index 4c11e6eca47f..901a9f103ba2 100644 --- a/solr/core/src/java/org/apache/solr/search/grouping/CommandHandler.java +++ b/solr/core/src/java/org/apache/solr/search/grouping/CommandHandler.java @@ -257,7 +257,15 @@ private void searchWithTimeLimiter(Query query, ProcessedFilter filter, Collecto } try { - searcher.search(query, collector); + try { + searcher.search(query, collector); + } finally { + // let a post filter (e.g. an AnalyticsQuery collector) report its results, as + // SolrIndexSearcher does for non grouped searches + if (filter.postFilter != null) { + filter.postFilter.complete(); + } + } } catch (ExitableDirectoryReader.ExitingReaderException x) { log.warn("Query: {}; ", query, x); } diff --git a/solr/core/src/test/org/apache/solr/search/AnalyticsMergeStrategyTest.java b/solr/core/src/test/org/apache/solr/search/AnalyticsMergeStrategyTest.java index c2b255824651..93821846308b 100644 --- a/solr/core/src/test/org/apache/solr/search/AnalyticsMergeStrategyTest.java +++ b/solr/core/src/test/org/apache/solr/search/AnalyticsMergeStrategyTest.java @@ -20,6 +20,7 @@ import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope.Scope; import org.apache.solr.BaseDistributedSearchTestCase; import org.apache.solr.SolrTestCaseJ4; +import org.apache.solr.client.solrj.request.QueryRequest; import org.apache.solr.client.solrj.response.QueryResponse; import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.util.NamedList; @@ -45,21 +46,7 @@ public static void setUpBeforeClass() throws Exception { @Test @ShardsFixed(num = 3) public void test() throws Exception { - del("*:*"); - - index_specific(0, "id", "1", "sort_i", "5"); - index_specific(0, "id", "2", "sort_i", "50"); - index_specific(1, "id", "5", "sort_i", "4"); - index_specific(1, "id", "6", "sort_i", "10"); - index_specific(0, "id", "7", "sort_i", "1"); - index_specific(1, "id", "8", "sort_i", "2"); - index_specific(2, "id", "9", "sort_i", "1000"); - index_specific(2, "id", "10", "sort_i", "1500"); - index_specific(2, "id", "11", "sort_i", "1300"); - index_specific(1, "id", "12", "sort_i", "15"); - index_specific(1, "id", "13", "sort_i", "16"); - - commit(); + indexDocs(); /* * The count qparser plugin is pointing to AnalyticsTestQParserPlugin. This class defines a @@ -88,6 +75,74 @@ public void test() throws Exception { setDistributedParams(params); rsp = queryRandomShard(params); assertCount(rsp, 4); + + // Merge strategies do not run for grouped responses at the coordinator, so the grouped + // analytics checks read shard responses. Shard 0 holds docs 1, 2 and 7. + params = new ModifiableSolrParams(); + params.add("q", "*:*"); + params.add("fq", "{!count}"); + params.add("group", "true"); + params.add("group.field", "sort_i"); + params.add("group.distributed.first", "true"); + QueryResponse shardRsp = new QueryRequest(params).process(clients.get(0)); + assertShardCount(shardRsp, "analytics section missing for grouped shard query", 3); + + params = new ModifiableSolrParams(); + params.add("q", "*:*"); + params.add("fq", "{!count}"); + params.add("group", "true"); + params.add("group.field", "sort_i"); + params.add("group.ngroups", "true"); + setDistributedParams(params); + rsp = queryRandomShard(params); + assertEquals(11, rsp.getGroupResponse().getValues().get(0).getNGroups().intValue()); + assertEquals(11, rsp.getGroupResponse().getValues().get(0).getMatches()); + } + + @Test + @ShardsFixed(num = 3) + public void testGroupedSecondPhase() throws Exception { + indexDocs(); + + ModifiableSolrParams params = new ModifiableSolrParams(); + params.add("q", "*:*"); + params.add("fq", "{!count}"); + params.add("group", "true"); + params.add("group.field", "sort_i"); + params.add("group.distributed.second", "true"); + params.add("group.topgroups.sort_i", "5"); + params.add("group.topgroups.sort_i", "50"); + QueryResponse shardRsp = new QueryRequest(params).process(clients.get(0)); + assertShardCount(shardRsp, "analytics section missing for grouped second phase shard query", 3); + } + + private void indexDocs() throws Exception { + del("*:*"); + + index_specific(0, "id", "1", "sort_i", "5"); + index_specific(0, "id", "2", "sort_i", "50"); + index_specific(1, "id", "5", "sort_i", "4"); + index_specific(1, "id", "6", "sort_i", "10"); + index_specific(0, "id", "7", "sort_i", "1"); + index_specific(1, "id", "8", "sort_i", "2"); + index_specific(2, "id", "9", "sort_i", "1000"); + index_specific(2, "id", "10", "sort_i", "1500"); + index_specific(2, "id", "11", "sort_i", "1300"); + index_specific(1, "id", "12", "sort_i", "15"); + index_specific(1, "id", "13", "sort_i", "16"); + + commit(); + } + + private void assertShardCount(QueryResponse shardRsp, String missingMessage, int count) + throws Exception { + @SuppressWarnings({"rawtypes"}) + NamedList analytics = (NamedList) shardRsp.getResponse().get("analytics"); + assertNotNull(missingMessage, analytics); + Integer c = (Integer) analytics.get("mycount"); + if (c != count) { + throw new Exception("Shard count is not correct:" + count + ":" + c); + } } private void assertCountOnly(QueryResponse rsp, int count) throws Exception {