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
This commit is contained in:
Munendra S N 2019-10-29 13:52:49 +05:30
parent 2055983d80
commit 0e3a66be43
7 changed files with 84 additions and 13 deletions

View File

@ -30,7 +30,9 @@ Jetty 9.4.19.v20190610
Upgrade Notes 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 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-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 Other Changes
--------------------- ---------------------

View File

@ -1392,6 +1392,8 @@ public class QueryComponent extends SearchComponent
.setSort(groupSortSpec.getSort()) .setSort(groupSortSpec.getSort())
.setQuery(query, rb.req) .setQuery(query, rb.req)
.setDocSet(searcher) .setDocSet(searcher)
.setMainQuery(rb.getQuery())
.setNeedScores(needScores)
.build() .build()
); );
} }

View File

@ -887,12 +887,13 @@ public class Grouping {
@Override @Override
protected void finish() throws IOException { protected void finish() throws IOException {
TopDocsCollector topDocsCollector = (TopDocsCollector) collector.getDelegate(); TopDocs topDocs = topCollector.topDocs();
TopDocs topDocs = topDocsCollector.topDocs();
float maxScore; float maxScore;
if (withinGroupSort == null || withinGroupSort.equals(Sort.RELEVANCE)) { if (withinGroupSort == null || withinGroupSort.equals(Sort.RELEVANCE)) {
maxScore = topDocs.scoreDocs.length == 0 ? Float.NaN : topDocs.scoreDocs[0].score; maxScore = topDocs.scoreDocs.length == 0 ? Float.NaN : topDocs.scoreDocs[0].score;
} else if (needScores) { } else if (needScores) {
// use top-level query to populate the scores
TopFieldCollector.populateScores(topDocs.scoreDocs, searcher, Grouping.this.query);
maxScore = maxScoreCollector.getMaxScore(); maxScore = maxScoreCollector.getMaxScore();
} else { } else {
maxScore = Float.NaN; maxScore = Float.NaN;

View File

@ -16,7 +16,19 @@
*/ */
package org.apache.solr.search.grouping.distributed.command; 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.request.SolrQueryRequest;
import org.apache.solr.search.DocSet; import org.apache.solr.search.DocSet;
import org.apache.solr.search.MaxScoreCollector; 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.Command;
import org.apache.solr.search.grouping.collector.FilterCollector; 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<QueryCommandResult> {
private Sort sort; private Sort sort;
private String queryString; private String queryString;
private Query query; private Query query;
private Query mainQuery;
private DocSet docSet; private DocSet docSet;
private Integer docsToCollect; private Integer docsToCollect;
private boolean needScores; private boolean needScores;
@ -49,11 +58,28 @@ public class QueryCommand implements Command<QueryCommandResult> {
return this; return this;
} }
/**
* Sets the group query.
*
* @param query The {@link Query} used for grouping
* @return this
*/
public Builder setQuery(Query query) { public Builder setQuery(Query query) {
this.query = query; this.query = query;
return this; 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. * Sets the group query from the specified groupQueryString.
* The groupQueryString is parsed into a query. * The groupQueryString is parsed into a query.
@ -95,11 +121,11 @@ public class QueryCommand implements Command<QueryCommandResult> {
} }
public QueryCommand build() { 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"); 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<QueryCommandResult> {
private final int docsToCollect; private final int docsToCollect;
private final boolean needScores; private final boolean needScores;
private final String queryString; private final String queryString;
private final Query mainQuery;
private TopDocsCollector topDocsCollector; private TopDocsCollector topDocsCollector;
private FilterCollector filterCollector; private FilterCollector filterCollector;
private MaxScoreCollector maxScoreCollector; private MaxScoreCollector maxScoreCollector;
private TopDocs topDocs; 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.sort = sort;
this.query = query; this.query = query;
this.docsToCollect = docsToCollect; this.docsToCollect = docsToCollect;
this.needScores = needScores; this.needScores = needScores;
this.docSet = docSet; this.docSet = docSet;
this.queryString = queryString; this.queryString = queryString;
this.mainQuery = mainQuery;
} }
@Override @Override
@ -147,7 +181,8 @@ public class QueryCommand implements Command<QueryCommandResult> {
public void postCollect(IndexSearcher searcher) throws IOException { public void postCollect(IndexSearcher searcher) throws IOException {
topDocs = topDocsCollector.topDocs(); topDocs = topDocsCollector.topDocs();
if (needScores) { if (needScores) {
TopFieldCollector.populateScores(topDocs.scoreDocs, searcher, query); // use mainQuery to populate the scores
TopFieldCollector.populateScores(topDocs.scoreDocs, searcher, mainQuery);
} }
} }

View File

@ -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 * Implementation for transforming {@link TopGroups} and {@link TopDocs} into a {@link NamedList} structure and
* visa versa. * vice versa.
*/ */
public class TopGroupsResultTransformer implements ShardResultTransformer<List<Command>, Map<String, ?>> { public class TopGroupsResultTransformer implements ShardResultTransformer<List<Command>, Map<String, ?>> {

View File

@ -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, "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", "{!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 // grouping shouldn't care if there are multiple fl params, or what order the fl field names are in
variantQuery(params("q", "*:*", variantQuery(params("q", "*:*",
"group", "true", "group.field", i1dv, "group.limit", "10", "group", "true", "group.field", i1dv, "group.limit", "10",

View File

@ -575,6 +575,21 @@ public class TestGroupingSearch extends SolrTestCaseJ4 {
,"/grouped/id:1000=={'matches':10,'doclist':{'numFound':0,'start':0,'docs':[]}}" ,"/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 // 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") 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," + ,"/grouped=={'id:[2 TO 5]':{'matches':10," +