From e69a819c9d4aa137df8c1b26522f5d78e51016b8 Mon Sep 17 00:00:00 2001 From: dceccarelli4 Date: Thu, 25 Apr 2019 17:41:29 +0100 Subject: [PATCH] Fix issues with rows --- .../java/org/apache/solr/search/Grouping.java | 23 ++++++++++----- .../collector/ReRankTopGroupsCollector.java | 3 +- .../command/TopGroupsFieldCommand.java | 4 +-- .../solr/search/TestReRankQParserPlugin.java | 29 +++++++++++++++++++ 4 files changed, 47 insertions(+), 12 deletions(-) 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 c784dfb86ed8..353704a04169 100644 --- a/solr/core/src/java/org/apache/solr/search/Grouping.java +++ b/solr/core/src/java/org/apache/solr/search/Grouping.java @@ -729,7 +729,7 @@ public class CommandField extends Command { @Override protected void prepare() throws IOException { if (reRankGroups > 0){ - actualGroupsToFind = getMax(offset, reRankGroups, maxDoc); + actualGroupsToFind = getMax(offset, Math.max(numGroups, reRankGroups), maxDoc); } else { actualGroupsToFind = getMax(offset, numGroups, maxDoc); } @@ -774,7 +774,7 @@ protected Collector createSecondPassCollector() throws IOException { if (query instanceof RankQuery) { secondPass = new ReRankTopGroupsCollector<>(new TermGroupSelector(groupBy), - topGroups, groupSort, withinGroupSort, groupedDocsToCollect, needScores, needScores, false, (RankQuery) query, searcher); + topGroups, groupSort, withinGroupSort, groupedDocsToCollect, needScores, needScores, (RankQuery) query, searcher); } else { secondPass = new TopGroupsCollector<>(new TermGroupSelector(groupBy), @@ -803,11 +803,16 @@ protected void finish() throws IOException { result = secondPass.getTopGroups(0); populateScoresIfNecessary(); } - if (result != null && query instanceof RankQuery && groupSort == Sort.RELEVANCE) { + if (result != null && query instanceof AbstractReRankQuery && groupSort == Sort.RELEVANCE) { // if we are sorting for relevance and query is a RankQuery, it may be that // the order of the groups changed, we need to reorder GroupDocs[] groups = result.groups; - Arrays.sort(groups, new Comparator() { + // we don't want to rerank all the time starting from the current page + + int rerankGroups = Math.max(((AbstractReRankQuery)query).getReRankDocs()-cmd.getOffset(), 0); + rerankGroups = Math.min(rerankGroups, groups.length); + + Arrays.sort(groups, 0, rerankGroups, new Comparator() { @Override public int compare(GroupDocs o1, GroupDocs o2) { if (o1.maxScore > o2.maxScore) return -1; @@ -978,7 +983,7 @@ protected void prepare() throws IOException { context = ValueSource.newContext(searcher); groupBy.createWeight(context, searcher); if (reRankGroups > 0){ - actualGroupsToFind = getMax(offset, reRankGroups, maxDoc); + actualGroupsToFind = getMax(offset, Math.max(numGroups, reRankGroups), maxDoc); } else { actualGroupsToFind = getMax(offset, numGroups, maxDoc); } @@ -1023,7 +1028,7 @@ protected Collector createSecondPassCollector() throws IOException { if (query instanceof RankQuery){ secondPass = new ReRankTopGroupsCollector<>(newSelector(), - topGroups, groupSort, withinGroupSort, groupdDocsToCollect, needScores, needScores, false, (RankQuery)query, searcher); + topGroups, groupSort, withinGroupSort, groupdDocsToCollect, needScores, needScores, (RankQuery)query, searcher); } else { secondPass = new TopGroupsCollector<>(newSelector(), topGroups, groupSort, withinGroupSort, groupdDocsToCollect, needScores @@ -1051,11 +1056,13 @@ protected void finish() throws IOException { populateScoresIfNecessary(); } - if (result != null && query instanceof RankQuery && groupSort == Sort.RELEVANCE) { + if (result != null && query instanceof AbstractReRankQuery && groupSort == Sort.RELEVANCE) { // if we are sorting for relevance and query is a RankQuery, it may be that // the order of the groups changed, we need to reorder GroupDocs[] groups = result.groups; - Arrays.sort(groups, new Comparator() { + int rerankGroups = Math.max(((AbstractReRankQuery)query).getReRankDocs()-cmd.getOffset(), 0); + rerankGroups = Math.min(rerankGroups, groups.length); + Arrays.sort(groups, 0, rerankGroups, new Comparator() { @Override public int compare(GroupDocs o1, GroupDocs o2) { if (o1.maxScore > o2.maxScore) return -1; diff --git a/solr/core/src/java/org/apache/solr/search/grouping/collector/ReRankTopGroupsCollector.java b/solr/core/src/java/org/apache/solr/search/grouping/collector/ReRankTopGroupsCollector.java index b15c2cd72470..09e916ee2892 100644 --- a/solr/core/src/java/org/apache/solr/search/grouping/collector/ReRankTopGroupsCollector.java +++ b/solr/core/src/java/org/apache/solr/search/grouping/collector/ReRankTopGroupsCollector.java @@ -55,12 +55,11 @@ public class ReRankTopGroupsCollector extends TopGroupsCollector { * @param maxDocsPerGroup the maximum number of docs to collect for each group * @param getScores if true, record the scores of all docs in each group * @param getMaxScores if true, record the maximum score for each group - * @param fillSortFields if true, record the sort field values for all docs * @param query the rankQuery if provided by the user, null otherwise * @param searcher an index searcher */ public ReRankTopGroupsCollector(GroupSelector groupSelector, Collection> groups, Sort groupSort, Sort withinGroupSort, - int maxDocsPerGroup, boolean getScores, boolean getMaxScores, boolean fillSortFields, RankQuery query, IndexSearcher searcher) { + int maxDocsPerGroup, boolean getScores, boolean getMaxScores, RankQuery query, IndexSearcher searcher) { super(new ReRankTopGroupsCollector.TopDocsReducer<>(withinGroupSort, maxDocsPerGroup, getScores, getMaxScores, query, searcher), groupSelector, groups, groupSort, withinGroupSort, maxDocsPerGroup); this.groupSort = Objects.requireNonNull(groupSort); this.withinGroupSort = Objects.requireNonNull(withinGroupSort); 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 b30a4cbf85ab..3689418a41d2 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 @@ -161,7 +161,7 @@ public List create() throws IOException { Collection> v = GroupConverter.toMutable(field, firstPhaseGroups); if (query instanceof RankQuery){ secondPassCollector = new ReRankTopGroupsCollector<>(new ValueSourceGroupSelector(vs, new HashMap<>()), v, - groupSort, withinGroupSort, maxDocPerGroup, needScores, needMaxScore, true, (RankQuery)query, searcher); + groupSort, withinGroupSort, maxDocPerGroup, needScores, needMaxScore, (RankQuery)query, searcher); } else { secondPassCollector = new TopGroupsCollector<>(new ValueSourceGroupSelector(vs, new HashMap<>()), v, groupSort, withinGroupSort, maxDocPerGroup, needMaxScore @@ -169,7 +169,7 @@ public List create() throws IOException { } } else { if (query instanceof RankQuery) { - secondPassCollector = new ReRankTopGroupsCollector(new TermGroupSelector(field.getName()),firstPhaseGroups, groupSort, withinGroupSort, maxDocPerGroup, needScores, needMaxScore, true, (RankQuery)query, searcher); + secondPassCollector = new ReRankTopGroupsCollector(new TermGroupSelector(field.getName()),firstPhaseGroups, groupSort, withinGroupSort, maxDocPerGroup, needScores, needMaxScore, (RankQuery)query, searcher); } else { secondPassCollector = new TopGroupsCollector<>(new TermGroupSelector(field.getName()), diff --git a/solr/core/src/test/org/apache/solr/search/TestReRankQParserPlugin.java b/solr/core/src/test/org/apache/solr/search/TestReRankQParserPlugin.java index 9a0fe34fb9b0..da5f2703851b 100644 --- a/solr/core/src/test/org/apache/solr/search/TestReRankQParserPlugin.java +++ b/solr/core/src/test/org/apache/solr/search/TestReRankQParserPlugin.java @@ -743,6 +743,35 @@ public void testRerankQueryAndGroupingRerankGroups() throws Exception { "//arr/lst[2]/result/doc/str[@name='id'][.='3']", "//arr/lst[3]/result/doc/str[@name='id'][.='2']" ); + + // checks reranking a small number of groups (2) + params.remove("rq"); + params.add("rq", "{!" + ReRankQParserPlugin.NAME + " " + ReRankQParserPlugin.RERANK_QUERY + "=$rqq " + + ReRankQParserPlugin.RERANK_DOCS + "=2}"); + params.remove("rows"); + params.add("rows", "3"); + + assertQ(req(params),"*[count(//doc)=3]", + "//arr/lst[1]/result/doc/str[@name='id'][.='3']", + "//arr/lst[2]/result/doc/str[@name='id'][.='2']", + "//arr/lst[3]/result/doc/str[@name='id'][.='5']" + ); + + // The underlying test will fail, but fixing it would require to perform + // to full rounds of grouping and reranking (one to retrieve the top groups + // and one to retrieve the top docs for each doc. + + // params.remove("start"); + // params.add("start", "1"); + // params.remove("rows"); + // params.add("rows", "2"); + // assertQ(req(params),"*[count(//doc)=2]", + // "//arr/lst[1]/result/doc/str[@name='id'][.='2']", + // "//arr/lst[2]/result/doc/str[@name='id'][.='5']" + // ); + + + // test grouping by function: // documents are // 1. firstly scored by their test_tl score and then