From 0e3a66be436c3ec2a8d9691f38de99d98f325e2c Mon Sep 17 00:00:00 2001 From: Munendra S N Date: Tue, 29 Oct 2019 13:52:49 +0530 Subject: [PATCH] SOLR-13823: fix ClassCastEx in group.query when score is requested * This makes sures score computed for standalone and distributed is same for group.query. This is done by using mainQuery to compute scores --- solr/CHANGES.txt | 7 ++- .../handler/component/QueryComponent.java | 2 + .../java/org/apache/solr/search/Grouping.java | 5 +- .../distributed/command/QueryCommand.java | 53 +++++++++++++++---- .../TopGroupsResultTransformer.java | 2 +- .../apache/solr/TestDistributedGrouping.java | 13 +++++ .../org/apache/solr/TestGroupingSearch.java | 15 ++++++ 7 files changed, 84 insertions(+), 13 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 8ce244ef6d1..ada1227c75d 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -30,7 +30,9 @@ Jetty 9.4.19.v20190610 Upgrade Notes --------------------- -(No changes) + +* org.apache.solr.search.grouping.distributed.command.QueryCommand.Builder has new method 'setMainQuery' which is used + to set top-level query. build() would fail if called without setting mainQuery New Features --------------------- @@ -64,6 +66,9 @@ Bug Fixes * SOLR-13877: Fix NPE in expand component when matched docs have fewer unique values. (Munendra S N) +* SOLR-13823: Fix ClassCastEx when score is requested with group.query. This also fixes score not being generated + for distributed group.query case. (Uwe Jäger, Munendra S N) + 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 7ebe7d1490f..5f17a103c39 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 @@ -1392,6 +1392,8 @@ public class QueryComponent extends SearchComponent .setSort(groupSortSpec.getSort()) .setQuery(query, rb.req) .setDocSet(searcher) + .setMainQuery(rb.getQuery()) + .setNeedScores(needScores) .build() ); } diff --git a/solr/core/src/java/org/apache/solr/search/Grouping.java b/solr/core/src/java/org/apache/solr/search/Grouping.java index 06a7ed0bdce..2e43da10822 100644 --- a/solr/core/src/java/org/apache/solr/search/Grouping.java +++ b/solr/core/src/java/org/apache/solr/search/Grouping.java @@ -887,12 +887,13 @@ public class Grouping { @Override protected void finish() throws IOException { - TopDocsCollector topDocsCollector = (TopDocsCollector) collector.getDelegate(); - TopDocs topDocs = topDocsCollector.topDocs(); + TopDocs topDocs = topCollector.topDocs(); float maxScore; if (withinGroupSort == null || withinGroupSort.equals(Sort.RELEVANCE)) { maxScore = topDocs.scoreDocs.length == 0 ? Float.NaN : topDocs.scoreDocs[0].score; } else if (needScores) { + // use top-level query to populate the scores + TopFieldCollector.populateScores(topDocs.scoreDocs, searcher, Grouping.this.query); maxScore = maxScoreCollector.getMaxScore(); } else { maxScore = Float.NaN; diff --git a/solr/core/src/java/org/apache/solr/search/grouping/distributed/command/QueryCommand.java b/solr/core/src/java/org/apache/solr/search/grouping/distributed/command/QueryCommand.java index 6503d86357f..06b49f6c9fa 100644 --- a/solr/core/src/java/org/apache/solr/search/grouping/distributed/command/QueryCommand.java +++ b/solr/core/src/java/org/apache/solr/search/grouping/distributed/command/QueryCommand.java @@ -16,7 +16,19 @@ */ package org.apache.solr.search.grouping.distributed.command; -import org.apache.lucene.search.*; +import java.io.IOException; +import java.util.Arrays; +import java.util.List; + +import org.apache.lucene.search.Collector; +import org.apache.lucene.search.IndexSearcher; +import org.apache.lucene.search.MultiCollector; +import org.apache.lucene.search.Query; +import org.apache.lucene.search.Sort; +import org.apache.lucene.search.TopDocs; +import org.apache.lucene.search.TopDocsCollector; +import org.apache.lucene.search.TopFieldCollector; +import org.apache.lucene.search.TopScoreDocCollector; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.search.DocSet; import org.apache.solr.search.MaxScoreCollector; @@ -26,10 +38,6 @@ import org.apache.solr.search.SyntaxError; import org.apache.solr.search.grouping.Command; import org.apache.solr.search.grouping.collector.FilterCollector; -import java.io.IOException; -import java.util.Arrays; -import java.util.List; - /** * */ @@ -40,6 +48,7 @@ public class QueryCommand implements Command { private Sort sort; private String queryString; private Query query; + private Query mainQuery; private DocSet docSet; private Integer docsToCollect; private boolean needScores; @@ -49,11 +58,28 @@ public class QueryCommand implements Command { return this; } + /** + * Sets the group query. + * + * @param query The {@link Query} used for grouping + * @return this + */ public Builder setQuery(Query query) { this.query = query; return this; } + /** + * Sets the main query used for fetching results. This is mainly used for computing the scores. + * + * @param mainQuery The top-level query + * @return this + */ + public Builder setMainQuery(Query mainQuery) { + this.mainQuery = mainQuery; + return this; + } + /** * Sets the group query from the specified groupQueryString. * The groupQueryString is parsed into a query. @@ -95,11 +121,11 @@ public class QueryCommand implements Command { } public QueryCommand build() { - if (sort == null || query == null || docSet == null || docsToCollect == null) { + if (sort == null || query == null || docSet == null || docsToCollect == null || mainQuery == null) { throw new IllegalStateException("All fields must be set"); } - return new QueryCommand(sort, query, docsToCollect, needScores, docSet, queryString); + return new QueryCommand(sort, query, docsToCollect, needScores, docSet, queryString, mainQuery); } } @@ -110,19 +136,27 @@ public class QueryCommand implements Command { private final int docsToCollect; private final boolean needScores; private final String queryString; + private final Query mainQuery; private TopDocsCollector topDocsCollector; private FilterCollector filterCollector; private MaxScoreCollector maxScoreCollector; private TopDocs topDocs; - private QueryCommand(Sort sort, Query query, int docsToCollect, boolean needScores, DocSet docSet, String queryString) { + private QueryCommand(Sort sort, + Query query, + int docsToCollect, + boolean needScores, + DocSet docSet, + String queryString, + Query mainQuery) { this.sort = sort; this.query = query; this.docsToCollect = docsToCollect; this.needScores = needScores; this.docSet = docSet; this.queryString = queryString; + this.mainQuery = mainQuery; } @Override @@ -147,7 +181,8 @@ public class QueryCommand implements Command { public void postCollect(IndexSearcher searcher) throws IOException { topDocs = topDocsCollector.topDocs(); if (needScores) { - TopFieldCollector.populateScores(topDocs.scoreDocs, searcher, query); + // use mainQuery to populate the scores + TopFieldCollector.populateScores(topDocs.scoreDocs, searcher, mainQuery); } } diff --git a/solr/core/src/java/org/apache/solr/search/grouping/distributed/shardresultserializer/TopGroupsResultTransformer.java b/solr/core/src/java/org/apache/solr/search/grouping/distributed/shardresultserializer/TopGroupsResultTransformer.java index e736a9b35ad..fb5bd32ef9e 100644 --- a/solr/core/src/java/org/apache/solr/search/grouping/distributed/shardresultserializer/TopGroupsResultTransformer.java +++ b/solr/core/src/java/org/apache/solr/search/grouping/distributed/shardresultserializer/TopGroupsResultTransformer.java @@ -52,7 +52,7 @@ import static org.apache.solr.common.params.CommonParams.ID; /** * Implementation for transforming {@link TopGroups} and {@link TopDocs} into a {@link NamedList} structure and - * visa versa. + * vice versa. */ public class TopGroupsResultTransformer implements ShardResultTransformer, Map> { diff --git a/solr/core/src/test/org/apache/solr/TestDistributedGrouping.java b/solr/core/src/test/org/apache/solr/TestDistributedGrouping.java index 1cba72f2ff1..7b759d01913 100644 --- a/solr/core/src/test/org/apache/solr/TestDistributedGrouping.java +++ b/solr/core/src/test/org/apache/solr/TestDistributedGrouping.java @@ -324,6 +324,19 @@ public class TestDistributedGrouping extends BaseDistributedSearchTestCase { query("q", "{!func}id_i1", "rows", 100, "fl", "score,id," + i1, "group", "true", "group.field", i1, "group.limit", -1, "sort", "score desc, _docid_ asc, id asc"); query("q", "{!func}id_i1", "rows", 100, "fl", "score,id," + i1, "group", "true", "group.field", i1, "group.limit", -1); + query("q", "*:*", + "group", "true", + "group.query", t1 + ":kings OR " + t1 + ":eggs", "group.limit", "3", + "fl", "id,score", "sort", i1 + " asc, id asc"); + query("q", "*:*", + "group", "true", + "group.query", t1 + ":kings OR " + t1 + ":eggs", "group.limit", "3", + "fl", "id,score", "group.format", "simple", "sort", i1 + " asc, id asc"); + query("q", "*:*", + "group", "true", + "group.query", t1 + ":kings OR " + t1 + ":eggs", "group.limit", "3", + "fl", "id,score", "group.main", "true", "sort", i1 + " asc, id asc"); + // grouping shouldn't care if there are multiple fl params, or what order the fl field names are in variantQuery(params("q", "*:*", "group", "true", "group.field", i1dv, "group.limit", "10", diff --git a/solr/core/src/test/org/apache/solr/TestGroupingSearch.java b/solr/core/src/test/org/apache/solr/TestGroupingSearch.java index d885484c9f2..35ef3c60245 100644 --- a/solr/core/src/test/org/apache/solr/TestGroupingSearch.java +++ b/solr/core/src/test/org/apache/solr/TestGroupingSearch.java @@ -575,6 +575,21 @@ public class TestGroupingSearch extends SolrTestCaseJ4 { ,"/grouped/id:1000=={'matches':10,'doclist':{'numFound':0,'start':0,'docs':[]}}" ); + // group.query and sort + assertJQ(req("fq",filt, "q","{!func}"+f2, "group","true", "group.query",f+":1", "fl","id,score", "rows","2", "group.limit","2", "sort",f+" desc, score desc", "indent","off") + ,"/grouped/"+f+":1==" + + "{'matches':10,'doclist':{'numFound':3,'start':0,'maxScore':10.0,'docs':[{'id':'8','score':10.0},{'id':'10','score':3.0}]}}," + ); + // group.query with fl=score and default sort + assertJQ(req("fq",filt, "q","{!func}"+f2, "group","true", "group.query",f+":1", "fl","id,score", "rows","2", "group.limit","2", "sort", "score desc", "indent","off") + ,"/grouped/"+f+":1==" + + "{'matches':10,'doclist':{'numFound':3,'start':0,'maxScore':10.0,'docs':[{'id':'8','score':10.0},{'id':'10','score':3.0}]}}," + ); + assertJQ(req("fq",filt, "q","{!func}"+f2, "group","true", "group.query",f+":1", "fl","id", "rows","2", "group.limit","2", "indent","off") + ,"/grouped/"+f+":1==" + + "{'matches':10,'doclist':{'numFound':3,'start':0,'docs':[{'id':'8'},{'id':'10'}]}}," + ); + // group.query and offset assertJQ(req("fq",filt, "q","{!func}"+f2, "group","true", "group.query","id:[2 TO 5]", "fl","id", "group.limit","3", "group.offset","2") ,"/grouped=={'id:[2 TO 5]':{'matches':10," +