You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ds...@apache.org on 2015/11/26 05:56:48 UTC
svn commit: r1716569 - in /lucene/dev/trunk: lucene/
lucene/grouping/src/java/org/apache/lucene/search/grouping/
lucene/grouping/src/java/org/apache/lucene/search/grouping/term/
lucene/grouping/src/test/org/apache/lucene/search/grouping/ solr/ solr/cor...
Author: dsmiley
Date: Thu Nov 26 04:56:47 2015
New Revision: 1716569
URL: http://svn.apache.org/viewvc?rev=1716569&view=rev
Log:
LUCENE-6900: Grouping sortWithinGroup shouldn't be null; use Sort.RELEVANCE.
Enhanced related Solr side a bit.
Modified:
lucene/dev/trunk/lucene/CHANGES.txt
lucene/dev/trunk/lucene/grouping/src/java/org/apache/lucene/search/grouping/AbstractSecondPassGroupingCollector.java
lucene/dev/trunk/lucene/grouping/src/java/org/apache/lucene/search/grouping/BlockGroupingCollector.java
lucene/dev/trunk/lucene/grouping/src/java/org/apache/lucene/search/grouping/GroupingSearch.java
lucene/dev/trunk/lucene/grouping/src/java/org/apache/lucene/search/grouping/TopGroups.java
lucene/dev/trunk/lucene/grouping/src/java/org/apache/lucene/search/grouping/term/TermSecondPassGroupingCollector.java
lucene/dev/trunk/lucene/grouping/src/test/org/apache/lucene/search/grouping/TestGrouping.java
lucene/dev/trunk/solr/CHANGES.txt
lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/Grouping.java
lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/grouping/distributed/responseprocessor/SearchGroupShardResponseProcessor.java
lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/grouping/distributed/responseprocessor/TopGroupsShardResponseProcessor.java
lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/grouping/distributed/shardresultserializer/TopGroupsResultTransformer.java
lucene/dev/trunk/solr/core/src/test/org/apache/solr/TestDistributedGrouping.java
lucene/dev/trunk/solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java
Modified: lucene/dev/trunk/lucene/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/CHANGES.txt?rev=1716569&r1=1716568&r2=1716569&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/CHANGES.txt (original)
+++ lucene/dev/trunk/lucene/CHANGES.txt Thu Nov 26 04:56:47 2015
@@ -104,7 +104,11 @@ Changes in Runtime Behavior
(Robert Muir, Mike McCandless)
======================= Lucene 5.5.0 =======================
-(No Changes)
+
+API Changes
+
+* LUCENE-6900: Grouping sortWithinGroup variables used to allow null to mean
+ Sort.RELEVANCE. Null is no longer permitted. (David Smiley)
======================= Lucene 5.4.0 =======================
Modified: lucene/dev/trunk/lucene/grouping/src/java/org/apache/lucene/search/grouping/AbstractSecondPassGroupingCollector.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/grouping/src/java/org/apache/lucene/search/grouping/AbstractSecondPassGroupingCollector.java?rev=1716569&r1=1716568&r2=1716569&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/grouping/src/java/org/apache/lucene/search/grouping/AbstractSecondPassGroupingCollector.java (original)
+++ lucene/dev/trunk/lucene/grouping/src/java/org/apache/lucene/search/grouping/AbstractSecondPassGroupingCollector.java Thu Nov 26 04:56:47 2015
@@ -24,6 +24,7 @@ import java.io.IOException;
import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
+import java.util.Objects;
/**
* SecondPassGroupingCollector is the second of two passes
@@ -54,29 +55,27 @@ public abstract class AbstractSecondPass
throws IOException {
//System.out.println("SP init");
- if (groups.size() == 0) {
- throw new IllegalArgumentException("no groups to collect (groups.size() is 0)");
+ if (groups.isEmpty()) {
+ throw new IllegalArgumentException("no groups to collect (groups is empty)");
}
- this.groupSort = groupSort;
- this.withinGroupSort = withinGroupSort;
- this.groups = groups;
+ this.groupSort = Objects.requireNonNull(groupSort);
+ this.withinGroupSort = Objects.requireNonNull(withinGroupSort);
+ this.groups = Objects.requireNonNull(groups);
this.maxDocsPerGroup = maxDocsPerGroup;
- groupMap = new HashMap<>(groups.size());
+ this.groupMap = new HashMap<>(groups.size());
for (SearchGroup<GROUP_VALUE_TYPE> group : groups) {
//System.out.println(" prep group=" + (group.groupValue == null ? "null" : group.groupValue.utf8ToString()));
final TopDocsCollector<?> collector;
- if (withinGroupSort == null) {
+ if (withinGroupSort.equals(Sort.RELEVANCE)) { // optimize to use TopScoreDocCollector
// Sort by score
collector = TopScoreDocCollector.create(maxDocsPerGroup);
} else {
// Sort by fields
collector = TopFieldCollector.create(withinGroupSort, maxDocsPerGroup, fillSortFields, getScores, getMaxScores);
}
- groupMap.put(group.groupValue,
- new SearchGroupDocs<>(group.groupValue,
- collector));
+ groupMap.put(group.groupValue, new SearchGroupDocs<>(group.groupValue, collector));
}
}
@@ -133,7 +132,7 @@ public abstract class AbstractSecondPass
}
return new TopGroups<>(groupSort.getSort(),
- withinGroupSort == null ? null : withinGroupSort.getSort(),
+ withinGroupSort.getSort(),
totalHitCount, totalGroupedHitCount, groupDocsResult,
maxScore);
}
Modified: lucene/dev/trunk/lucene/grouping/src/java/org/apache/lucene/search/grouping/BlockGroupingCollector.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/grouping/src/java/org/apache/lucene/search/grouping/BlockGroupingCollector.java?rev=1716569&r1=1716568&r2=1716569&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/grouping/src/java/org/apache/lucene/search/grouping/BlockGroupingCollector.java (original)
+++ lucene/dev/trunk/lucene/grouping/src/java/org/apache/lucene/search/grouping/BlockGroupingCollector.java Thu Nov 26 04:56:47 2015
@@ -231,8 +231,7 @@ public class BlockGroupingCollector exte
this.needsScores = needsScores;
this.lastDocPerGroup = lastDocPerGroup;
- // TODO: allow null groupSort to mean "by relevance",
- // and specialize it?
+
this.groupSort = groupSort;
this.topNGroups = topNGroups;
@@ -265,8 +264,7 @@ public class BlockGroupingCollector exte
* DocValues, etc.)
*
* @param withinGroupSort The {@link Sort} used to sort
- * documents within each group. Passing null is
- * allowed, to sort by relevance.
+ * documents within each group.
* @param groupOffset Which group to start from
* @param withinGroupOffset Which document to start from
* within each group
@@ -300,7 +298,7 @@ public class BlockGroupingCollector exte
// At this point we hold all docs w/ in each group,
// unsorted; we now sort them:
final TopDocsCollector<?> collector;
- if (withinGroupSort == null) {
+ if (withinGroupSort.equals(Sort.RELEVANCE)) {
// Sort by score
if (!needsScores) {
throw new IllegalArgumentException("cannot sort by relevance within group: needsScores=false");
@@ -356,7 +354,7 @@ public class BlockGroupingCollector exte
*/
return new TopGroups<>(new TopGroups<>(groupSort.getSort(),
- withinGroupSort == null ? null : withinGroupSort.getSort(),
+ withinGroupSort.getSort(),
totalHitCount, totalGroupedHitCount, groups, maxScore),
totalGroupCount);
}
Modified: lucene/dev/trunk/lucene/grouping/src/java/org/apache/lucene/search/grouping/GroupingSearch.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/grouping/src/java/org/apache/lucene/search/grouping/GroupingSearch.java?rev=1716569&r1=1716568&r2=1716569&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/grouping/src/java/org/apache/lucene/search/grouping/GroupingSearch.java (original)
+++ lucene/dev/trunk/lucene/grouping/src/java/org/apache/lucene/search/grouping/GroupingSearch.java Thu Nov 26 04:56:47 2015
@@ -58,7 +58,7 @@ public class GroupingSearch {
private final Query groupEndDocs;
private Sort groupSort = Sort.RELEVANCE;
- private Sort sortWithinGroup;
+ private Sort sortWithinGroup = Sort.RELEVANCE;
private int groupDocsOffset;
private int groupDocsLimit = 1;
Modified: lucene/dev/trunk/lucene/grouping/src/java/org/apache/lucene/search/grouping/TopGroups.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/grouping/src/java/org/apache/lucene/search/grouping/TopGroups.java?rev=1716569&r1=1716568&r2=1716569&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/grouping/src/java/org/apache/lucene/search/grouping/TopGroups.java (original)
+++ lucene/dev/trunk/lucene/grouping/src/java/org/apache/lucene/search/grouping/TopGroups.java Thu Nov 26 04:56:47 2015
@@ -132,7 +132,7 @@ public class TopGroups<GROUP_VALUE_TYPE>
final GroupDocs<T>[] mergedGroupDocs = new GroupDocs[numGroups];
final TopDocs[] shardTopDocs;
- if (docSort == null) {
+ if (docSort.equals(Sort.RELEVANCE)) {
shardTopDocs = new TopDocs[shardGroups.length];
} else {
shardTopDocs = new TopFieldDocs[shardGroups.length];
@@ -163,7 +163,7 @@ public class TopGroups<GROUP_VALUE_TYPE>
}
*/
- if (docSort == null) {
+ if (docSort.equals(Sort.RELEVANCE)) {
shardTopDocs[shardIDX] = new TopDocs(shardGroupDocs.totalHits,
shardGroupDocs.scoreDocs,
shardGroupDocs.maxScore);
@@ -179,7 +179,7 @@ public class TopGroups<GROUP_VALUE_TYPE>
}
final TopDocs mergedTopDocs;
- if (docSort == null) {
+ if (docSort.equals(Sort.RELEVANCE)) {
mergedTopDocs = TopDocs.merge(docOffset + docTopN, shardTopDocs);
} else {
mergedTopDocs = TopDocs.merge(docSort, docOffset + docTopN, (TopFieldDocs[]) shardTopDocs);
@@ -231,7 +231,7 @@ public class TopGroups<GROUP_VALUE_TYPE>
if (totalGroupCount != null) {
TopGroups<T> result = new TopGroups<>(groupSort.getSort(),
- docSort == null ? null : docSort.getSort(),
+ docSort.getSort(),
totalHitCount,
totalGroupedHitCount,
mergedGroupDocs,
@@ -239,7 +239,7 @@ public class TopGroups<GROUP_VALUE_TYPE>
return new TopGroups<>(result, totalGroupCount);
} else {
return new TopGroups<>(groupSort.getSort(),
- docSort == null ? null : docSort.getSort(),
+ docSort.getSort(),
totalHitCount,
totalGroupedHitCount,
mergedGroupDocs,
Modified: lucene/dev/trunk/lucene/grouping/src/java/org/apache/lucene/search/grouping/term/TermSecondPassGroupingCollector.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/grouping/src/java/org/apache/lucene/search/grouping/term/TermSecondPassGroupingCollector.java?rev=1716569&r1=1716568&r2=1716569&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/grouping/src/java/org/apache/lucene/search/grouping/term/TermSecondPassGroupingCollector.java (original)
+++ lucene/dev/trunk/lucene/grouping/src/java/org/apache/lucene/search/grouping/term/TermSecondPassGroupingCollector.java Thu Nov 26 04:56:47 2015
@@ -38,18 +38,19 @@ import org.apache.lucene.util.SentinelIn
*/
public class TermSecondPassGroupingCollector extends AbstractSecondPassGroupingCollector<BytesRef> {
+ private final String groupField;
private final SentinelIntSet ordSet;
+
private SortedDocValues index;
- private final String groupField;
@SuppressWarnings({"unchecked", "rawtypes"})
public TermSecondPassGroupingCollector(String groupField, Collection<SearchGroup<BytesRef>> groups, Sort groupSort, Sort withinGroupSort,
int maxDocsPerGroup, boolean getScores, boolean getMaxScores, boolean fillSortFields)
throws IOException {
super(groups, groupSort, withinGroupSort, maxDocsPerGroup, getScores, getMaxScores, fillSortFields);
- ordSet = new SentinelIntSet(groupMap.size(), -2);
this.groupField = groupField;
- groupDocs = (SearchGroupDocs<BytesRef>[]) new SearchGroupDocs[ordSet.keys.length];
+ this.ordSet = new SentinelIntSet(groupMap.size(), -2);
+ super.groupDocs = (SearchGroupDocs<BytesRef>[]) new SearchGroupDocs[ordSet.keys.length];
}
@Override
Modified: lucene/dev/trunk/lucene/grouping/src/test/org/apache/lucene/search/grouping/TestGrouping.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/grouping/src/test/org/apache/lucene/search/grouping/TestGrouping.java?rev=1716569&r1=1716568&r2=1716569&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/grouping/src/test/org/apache/lucene/search/grouping/TestGrouping.java (original)
+++ lucene/dev/trunk/lucene/grouping/src/test/org/apache/lucene/search/grouping/TestGrouping.java Thu Nov 26 04:56:47 2015
@@ -150,7 +150,7 @@ public class TestGrouping extends Lucene
final AbstractFirstPassGroupingCollector<?> c1 = createRandomFirstPassCollector(groupField, groupSort, 10);
indexSearcher.search(new TermQuery(new Term("content", "random")), c1);
- final AbstractSecondPassGroupingCollector<?> c2 = createSecondPassCollector(c1, groupField, groupSort, null, 0, 5, true, true, true);
+ final AbstractSecondPassGroupingCollector<?> c2 = createSecondPassCollector(c1, groupField, groupSort, Sort.RELEVANCE, 0, 5, true, true, true);
indexSearcher.search(new TermQuery(new Term("content", "random")), c2);
final TopGroups<?> groups = c2.getTopGroups(0);
Modified: lucene/dev/trunk/solr/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/CHANGES.txt?rev=1716569&r1=1716568&r2=1716569&view=diff
==============================================================================
--- lucene/dev/trunk/solr/CHANGES.txt (original)
+++ lucene/dev/trunk/solr/CHANGES.txt Thu Nov 26 04:56:47 2015
@@ -159,8 +159,14 @@ Other Changes
* SOLR-8179: SQL JDBC - DriverImpl loadParams doesn't support keys with no values in the connection string
(Kevin Risden, Joel Bernstein)
-================== 5.5.0 ==================
-(No Changes)
+======================= Lucene 5.5.0 =======================
+
+Other Changes
+----------------------
+
+* LUCENE-6900: Added test for score ordered grouping, and refactored TopGroupsResultTransformer.
+ (David Smiley)
+
================== 5.4.0 ==================
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/Grouping.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/Grouping.java?rev=1716569&r1=1716568&r2=1716569&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/Grouping.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/Grouping.java Thu Nov 26 04:56:47 2015
@@ -40,7 +40,6 @@ import org.apache.lucene.search.MultiCol
import org.apache.lucene.search.Query;
import org.apache.lucene.search.ScoreDoc;
import org.apache.lucene.search.Sort;
-import org.apache.lucene.search.SortField;
import org.apache.lucene.search.TimeLimitingCollector;
import org.apache.lucene.search.TopDocs;
import org.apache.lucene.search.TopDocsCollector;
@@ -74,7 +73,7 @@ import org.slf4j.LoggerFactory;
/**
* Basic Solr Grouping infrastructure.
- * Warning NOT thread save!
+ * Warning NOT thread safe!
*
* @lucene.experimental
*/
@@ -109,7 +108,7 @@ public class Grouping {
private NamedList grouped = new SimpleOrderedMap();
private Set<Integer> idSet = new LinkedHashSet<>(); // used for tracking unique docs when we need a doclist
private int maxMatches; // max number of matches from any grouping command
- private float maxScore = Float.NEGATIVE_INFINITY; // max score seen in any doclist
+ private float maxScore = Float.NaN; // max score seen in any doclist
private boolean signalCacheWarning = false;
private TimeLimitingCollector timeLimitingCollector;
@@ -311,16 +310,8 @@ public class Grouping {
boolean cacheScores = false;
// NOTE: Change this when withinGroupSort can be specified per group
if (!needScores && !commands.isEmpty()) {
- if (commands.get(0).withinGroupSort == null) {
- cacheScores = true;
- } else {
- for (SortField field : commands.get(0).withinGroupSort.getSort()) {
- if (field.getType() == SortField.Type.SCORE) {
- cacheScores = true;
- break;
- }
- }
- }
+ Sort withinGroupSort = commands.get(0).withinGroupSort;
+ cacheScores = withinGroupSort == null || withinGroupSort.needsScores();
} else if (needScores) {
cacheScores = needScores;
}
@@ -638,7 +629,7 @@ public class Grouping {
}
float score = groups.maxScore;
- maxScore = Math.max(maxScore, score);
+ maxScore = maxAvoidNaN(score, maxScore);
DocSlice docs = new DocSlice(off, Math.max(0, ids.length - off), ids, scores, groups.totalHits, score);
if (getDocList) {
@@ -661,13 +652,11 @@ public class Grouping {
List<Float> scores = new ArrayList<>();
int docsToGather = getMax(offset, numGroups, maxDoc);
int docsGathered = 0;
- float maxScore = Float.NEGATIVE_INFINITY;
+ float maxScore = Float.NaN;
outer:
for (GroupDocs group : groups) {
- if (group.maxScore > maxScore) {
- maxScore = group.maxScore;
- }
+ maxScore = maxAvoidNaN(maxScore, group.maxScore);
for (ScoreDoc scoreDoc : group.scoreDocs) {
if (docsGathered >= docsToGather) {
@@ -696,6 +685,15 @@ public class Grouping {
}
+ /** Differs from {@link Math#max(float, float)} in that if only one side is NaN, we return the other. */
+ private float maxAvoidNaN(float valA, float valB) {
+ if (Float.isNaN(valA) || valB > valA) {
+ return valB;
+ } else {
+ return valA;
+ }
+ }
+
/**
* A group command for grouping on a field.
*/
@@ -759,6 +757,7 @@ public class Grouping {
int groupedDocsToCollect = getMax(groupOffset, docsPerGroup, maxDoc);
groupedDocsToCollect = Math.max(groupedDocsToCollect, 1);
+ Sort withinGroupSort = this.withinGroupSort != null ? this.withinGroupSort : Sort.RELEVANCE;
secondPass = new TermSecondPassGroupingCollector(
groupBy, topGroups, groupSort, withinGroupSort, groupedDocsToCollect, needScores, needScores, false
);
@@ -776,7 +775,7 @@ public class Grouping {
*/
@Override
public AbstractAllGroupHeadsCollector<?> createAllGroupCollector() throws IOException {
- Sort sortWithinGroup = withinGroupSort != null ? withinGroupSort : new Sort();
+ Sort sortWithinGroup = withinGroupSort != null ? withinGroupSort : Sort.RELEVANCE;
return TermAllGroupHeadsCollector.create(groupBy, sortWithinGroup);
}
@@ -882,7 +881,7 @@ public class Grouping {
TopDocsCollector newCollector(Sort sort, boolean needScores) throws IOException {
int groupDocsToCollect = getMax(groupOffset, docsPerGroup, maxDoc);
- if (sort == null || sort == Sort.RELEVANCE) {
+ if (sort == null || sort.equals(Sort.RELEVANCE)) {
return TopScoreDocCollector.create(groupDocsToCollect);
} else {
return TopFieldCollector.create(searcher.weightSort(sort), groupDocsToCollect, false, needScores, needScores);
@@ -979,6 +978,7 @@ public class Grouping {
int groupdDocsToCollect = getMax(groupOffset, docsPerGroup, maxDoc);
groupdDocsToCollect = Math.max(groupdDocsToCollect, 1);
+ Sort withinGroupSort = this.withinGroupSort != null ? this.withinGroupSort : Sort.RELEVANCE;
secondPass = new FunctionSecondPassGroupingCollector(
topGroups, groupSort, withinGroupSort, groupdDocsToCollect, needScores, needScores, false, groupBy, context
);
@@ -993,7 +993,7 @@ public class Grouping {
@Override
public AbstractAllGroupHeadsCollector<?> createAllGroupCollector() throws IOException {
- Sort sortWithinGroup = withinGroupSort != null ? withinGroupSort : new Sort();
+ Sort sortWithinGroup = withinGroupSort != null ? withinGroupSort : Sort.RELEVANCE;
return new FunctionAllGroupHeadsCollector(groupBy, context, sortWithinGroup);
}
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/grouping/distributed/responseprocessor/SearchGroupShardResponseProcessor.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/grouping/distributed/responseprocessor/SearchGroupShardResponseProcessor.java?rev=1716569&r1=1716568&r2=1716569&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/grouping/distributed/responseprocessor/SearchGroupShardResponseProcessor.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/grouping/distributed/responseprocessor/SearchGroupShardResponseProcessor.java Thu Nov 26 04:56:47 2015
@@ -51,6 +51,10 @@ public class SearchGroupShardResponsePro
SortSpec ss = rb.getSortSpec();
Sort groupSort = rb.getGroupingSpec().getGroupSort();
String[] fields = rb.getGroupingSpec().getFields();
+ Sort sortWithinGroup = rb.getGroupingSpec().getSortWithinGroup();
+ if (sortWithinGroup == null) { // TODO prevent it from being null in the first place
+ sortWithinGroup = Sort.RELEVANCE;
+ }
Map<String, List<Collection<SearchGroup<BytesRef>>>> commandSearchGroups = new HashMap<>();
Map<String, Map<SearchGroup<BytesRef>, Set<String>>> tempSearchGroupToShards = new HashMap<>();
@@ -106,7 +110,7 @@ public class SearchGroupShardResponsePro
maxElapsedTime = (int) Math.max(maxElapsedTime, srsp.getSolrResponse().getElapsedTime());
@SuppressWarnings("unchecked")
NamedList<NamedList> firstPhaseResult = (NamedList<NamedList>) srsp.getSolrResponse().getResponse().get("firstPhase");
- final Map<String, SearchGroupsFieldCommandResult> result = serializer.transformToNative(firstPhaseResult, groupSort, null, srsp.getShard());
+ final Map<String, SearchGroupsFieldCommandResult> result = serializer.transformToNative(firstPhaseResult, groupSort, sortWithinGroup, srsp.getShard());
for (String field : commandSearchGroups.keySet()) {
final SearchGroupsFieldCommandResult firstPhaseCommandResult = result.get(field);
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/grouping/distributed/responseprocessor/TopGroupsShardResponseProcessor.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/grouping/distributed/responseprocessor/TopGroupsShardResponseProcessor.java?rev=1716569&r1=1716568&r2=1716569&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/grouping/distributed/responseprocessor/TopGroupsShardResponseProcessor.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/grouping/distributed/responseprocessor/TopGroupsShardResponseProcessor.java Thu Nov 26 04:56:47 2015
@@ -61,6 +61,9 @@ public class TopGroupsShardResponseProce
String[] fields = rb.getGroupingSpec().getFields();
String[] queries = rb.getGroupingSpec().getQueries();
Sort sortWithinGroup = rb.getGroupingSpec().getSortWithinGroup();
+ if (sortWithinGroup == null) { // TODO prevent it from being null in the first place
+ sortWithinGroup = Sort.RELEVANCE;
+ }
// If group.format=simple group.offset doesn't make sense
int groupOffsetDefault;
@@ -173,7 +176,7 @@ public class TopGroupsShardResponseProce
int topN = rb.getGroupingSpec().getOffset() + rb.getGroupingSpec().getLimit();
final TopDocs mergedTopDocs;
- if (sortWithinGroup == null) {
+ if (sortWithinGroup.equals(Sort.RELEVANCE)) {
mergedTopDocs = TopDocs.merge(topN, topDocs.toArray(new TopDocs[topDocs.size()]));
} else {
mergedTopDocs = TopDocs.merge(sortWithinGroup, topN, topDocs.toArray(new TopFieldDocs[topDocs.size()]));
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/grouping/distributed/shardresultserializer/TopGroupsResultTransformer.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/grouping/distributed/shardresultserializer/TopGroupsResultTransformer.java?rev=1716569&r1=1716568&r2=1716569&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/grouping/distributed/shardresultserializer/TopGroupsResultTransformer.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/search/grouping/distributed/shardresultserializer/TopGroupsResultTransformer.java Thu Nov 26 04:56:47 2015
@@ -17,6 +17,12 @@ package org.apache.solr.search.grouping.
* limitations under the License.
*/
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
import org.apache.lucene.document.DocumentStoredFieldVisitor;
import org.apache.lucene.index.StoredDocument;
import org.apache.lucene.search.FieldDoc;
@@ -27,8 +33,6 @@ import org.apache.lucene.search.TopField
import org.apache.lucene.search.grouping.GroupDocs;
import org.apache.lucene.search.grouping.TopGroups;
import org.apache.lucene.util.BytesRef;
-import org.apache.lucene.util.CharsRef;
-import org.apache.lucene.util.UnicodeUtil;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.handler.component.ResponseBuilder;
import org.apache.solr.handler.component.ShardDoc;
@@ -42,12 +46,6 @@ import org.apache.solr.search.grouping.d
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
/**
* Implementation for transforming {@link TopGroups} and {@link TopDocs} into a {@link NamedList} structure and
* visa versa.
@@ -110,40 +108,9 @@ public class TopGroupsResultTransformer
@SuppressWarnings("unchecked")
List<NamedList<Object>> documents = (List<NamedList<Object>>) commandResult.get("documents");
- ScoreDoc[] scoreDocs = new ScoreDoc[documents.size()];
- int j = 0;
- for (NamedList<Object> document : documents) {
- Object docId = document.get("id");
- Object uniqueId = null;
- if (docId != null)
- uniqueId = docId.toString();
- else
- log.warn("doc {} has null 'id'", document);
- Float score = (Float) document.get("score");
- if (score == null) {
- score = Float.NaN;
- }
- Object[] sortValues = null;
- Object sortValuesVal = document.get("sortValues");
- if (sortValuesVal != null) {
- sortValues = ((List) sortValuesVal).toArray();
- for (int k = 0; k < sortValues.length; k++) {
- SchemaField field = groupSort.getSort()[k].getField() != null ? schema.getFieldOrNull(groupSort.getSort()[k].getField()) : null;
- if (field != null) {
- FieldType fieldType = field.getType();
- if (sortValues[k] != null) {
- sortValues[k] = fieldType.unmarshalSortValue(sortValues[k]);
- }
- }
- }
- }
- else {
- log.warn("doc {} has null 'sortValues'", document);
- }
- scoreDocs[j++] = new ShardDoc(score, sortValues, uniqueId, shard);
- }
+ ScoreDoc[] scoreDocs = transformToNativeShardDoc(documents, groupSort, shard, schema);
final TopDocs topDocs;
- if (sortWithinGroup == null) {
+ if (sortWithinGroup.equals(Sort.RELEVANCE)) {
topDocs = new TopDocs(totalHits, scoreDocs, maxScore);
} else {
topDocs = new TopFieldDocs(totalHits, scoreDocs, sortWithinGroup.getSort(), maxScore);
@@ -167,26 +134,7 @@ public class TopGroupsResultTransformer
@SuppressWarnings("unchecked")
List<NamedList<Object>> documents = (List<NamedList<Object>>) groupResult.get("documents");
- ScoreDoc[] scoreDocs = new ScoreDoc[documents.size()];
- int j = 0;
- for (NamedList<Object> document : documents) {
- Object uniqueId = document.get("id").toString();
- Float score = (Float) document.get("score");
- if (score == null) {
- score = Float.NaN;
- }
- Object[] sortValues = ((List) document.get("sortValues")).toArray();
- for (int k = 0; k < sortValues.length; k++) {
- SchemaField field = sortWithinGroup.getSort()[k].getField() != null ? schema.getFieldOrNull(sortWithinGroup.getSort()[k].getField()) : null;
- if (field != null) {
- FieldType fieldType = field.getType();
- if (sortValues[k] != null) {
- sortValues[k] = fieldType.unmarshalSortValue(sortValues[k]);
- }
- }
- }
- scoreDocs[j++] = new ShardDoc(score, sortValues, uniqueId, shard);
- }
+ ScoreDoc[] scoreDocs = transformToNativeShardDoc(documents, groupSort, shard, schema);
BytesRef groupValueRef = groupValue != null ? new BytesRef(groupValue) : null;
groupDocs.add(new GroupDocs<>(Float.NaN, maxScore, totalGroupHits, scoreDocs, groupValueRef, null));
@@ -204,6 +152,43 @@ public class TopGroupsResultTransformer
return result;
}
+ protected ScoreDoc[] transformToNativeShardDoc(List<NamedList<Object>> documents, Sort groupSort, String shard,
+ IndexSchema schema) {
+ ScoreDoc[] scoreDocs = new ScoreDoc[documents.size()];
+ int j = 0;
+ for (NamedList<Object> document : documents) {
+ Object docId = document.get("id");
+ if (docId != null) {
+ docId = docId.toString();
+ } else {
+ log.error("doc {} has null 'id'", document);
+ }
+ Float score = (Float) document.get("score");
+ if (score == null) {
+ score = Float.NaN;
+ }
+ Object[] sortValues = null;
+ Object sortValuesVal = document.get("sortValues");
+ if (sortValuesVal != null) {
+ sortValues = ((List) sortValuesVal).toArray();
+ for (int k = 0; k < sortValues.length; k++) {
+ SchemaField field = groupSort.getSort()[k].getField() != null
+ ? schema.getFieldOrNull(groupSort.getSort()[k].getField()) : null;
+ if (field != null) {
+ FieldType fieldType = field.getType();
+ if (sortValues[k] != null) {
+ sortValues[k] = fieldType.unmarshalSortValue(sortValues[k]);
+ }
+ }
+ }
+ } else {
+ log.debug("doc {} has null 'sortValues'", document);
+ }
+ scoreDocs[j++] = new ShardDoc(score, sortValues, docId, shard);
+ }
+ return scoreDocs;
+ }
+
protected NamedList serializeTopGroups(TopGroups<BytesRef> data, SchemaField groupField) throws IOException {
NamedList<Object> result = new NamedList<>();
result.add("totalGroupedHitCount", data.totalGroupedHitCount);
@@ -211,7 +196,6 @@ public class TopGroupsResultTransformer
if (data.totalGroupCount != null) {
result.add("totalGroupCount", data.totalGroupCount);
}
- CharsRef spare = new CharsRef();
final IndexSchema schema = rb.req.getSearcher().getSchema();
SchemaField uniqueField = schema.getUniqueKeyField();
@@ -233,7 +217,7 @@ public class TopGroupsResultTransformer
document.add("score", searchGroup.scoreDocs[i].score);
}
if (!(searchGroup.scoreDocs[i] instanceof FieldDoc)) {
- continue;
+ continue; // thus don't add sortValues below
}
FieldDoc fieldDoc = (FieldDoc) searchGroup.scoreDocs[i];
@@ -264,7 +248,8 @@ public class TopGroupsResultTransformer
NamedList<Object> queryResult = new NamedList<>();
queryResult.add("matches", result.getMatches());
queryResult.add("totalHits", result.getTopDocs().totalHits);
- if (rb.getGroupingSpec().isNeedScore()) {
+ // debug: assert !Float.isNaN(result.getTopDocs().getMaxScore()) == rb.getGroupingSpec().isNeedScore();
+ if (!Float.isNaN(result.getTopDocs().getMaxScore())) {
queryResult.add("maxScore", result.getTopDocs().getMaxScore());
}
List<NamedList> documents = new ArrayList<>();
@@ -272,18 +257,17 @@ public class TopGroupsResultTransformer
final IndexSchema schema = rb.req.getSearcher().getSchema();
SchemaField uniqueField = schema.getUniqueKeyField();
- CharsRef spare = new CharsRef();
for (ScoreDoc scoreDoc : result.getTopDocs().scoreDocs) {
NamedList<Object> document = new NamedList<>();
documents.add(document);
StoredDocument doc = retrieveDocument(uniqueField, scoreDoc.doc);
document.add("id", uniqueField.getType().toExternal(doc.getField(uniqueField.getName())));
- if (rb.getGroupingSpec().isNeedScore()) {
+ if (!Float.isNaN(scoreDoc.score)) {
document.add("score", scoreDoc.score);
}
if (!FieldDoc.class.isInstance(scoreDoc)) {
- continue;
+ continue; // thus don't add sortValues below
}
FieldDoc fieldDoc = (FieldDoc) scoreDoc;
@@ -291,7 +275,7 @@ public class TopGroupsResultTransformer
for (int j = 0; j < fieldDoc.fields.length; j++) {
Object sortValue = fieldDoc.fields[j];
Sort groupSort = rb.getGroupingSpec().getGroupSort();
- SchemaField field = groupSort.getSort()[j].getField() != null
+ SchemaField field = groupSort.getSort()[j].getField() != null
? schema.getFieldOrNull(groupSort.getSort()[j].getField()) : null;
if (field != null) {
FieldType fieldType = field.getType();
Modified: lucene/dev/trunk/solr/core/src/test/org/apache/solr/TestDistributedGrouping.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/test/org/apache/solr/TestDistributedGrouping.java?rev=1716569&r1=1716568&r2=1716569&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/test/org/apache/solr/TestDistributedGrouping.java (original)
+++ lucene/dev/trunk/solr/core/src/test/org/apache/solr/TestDistributedGrouping.java Thu Nov 26 04:56:47 2015
@@ -259,10 +259,12 @@ public class TestDistributedGrouping ext
assertEquals(shardsArr.length, groupCount);
- // We cannot validate distributed grouping with scoring as first sort. since there is no global idf. We can check if no errors occur
- simpleQuery("q", "*:*", "rows", 100, "fl", "id," + i1, "group", "true", "group.field", i1, "group.limit", 10, "sort", i1 + " desc", "group.sort", "score desc"); // SOLR-2955
- simpleQuery("q", "*:*", "rows", 100, "fl", "id," + i1, "group", "true", "group.field", i1, "group.limit", 10, "sort", "score desc, _docid_ asc, id asc");
- simpleQuery("q", "*:*", "rows", 100, "fl", "id," + i1, "group", "true", "group.field", i1, "group.limit", 10);
+ // We validate distributed grouping with scoring as first sort.
+ // note: this 'q' matches all docs and returns the 'id' as the score, which is unique and so our results should be deterministic.
+ handle.put("maxScore", SKIP);// TODO see SOLR-6612
+ query("q", "{!func}id", "rows", 100, "fl", "score,id," + i1, "group", "true", "group.field", i1, "group.limit", 10, "sort", i1 + " desc", "group.sort", "score desc"); // SOLR-2955
+ query("q", "{!func}id", "rows", 100, "fl", "score,id," + i1, "group", "true", "group.field", i1, "group.limit", 10, "sort", "score desc, _docid_ asc, id asc");
+ query("q", "{!func}id", "rows", 100, "fl", "score,id," + i1, "group", "true", "group.field", i1, "group.limit", 10);
// Can't validate the response, but can check if no errors occur.
simpleQuery("q", "*:*", "rows", 100, "fl", "id," + i1, "group", "true", "group.query", t1 + ":kings OR " + t1 + ":eggs", "group.limit", 10, "sort", i1 + " asc, id asc", CommonParams.TIME_ALLOWED, 1);
Modified: lucene/dev/trunk/solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java?rev=1716569&r1=1716568&r2=1716569&view=diff
==============================================================================
--- lucene/dev/trunk/solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java (original)
+++ lucene/dev/trunk/solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java Thu Nov 26 04:56:47 2015
@@ -778,10 +778,11 @@ public abstract class BaseDistributedSea
String cmp;
int f = flags(handle, "maxScore");
- if ((f & SKIPVAL) == 0) {
+ if (f == 0) {
cmp = compare(a.getMaxScore(), b.getMaxScore(), 0, handle);
if (cmp != null) return ".maxScore" + cmp;
- } else {
+ } else if ((f & SKIP) == 0) { // so we skip val but otherwise both should be present
+ assert (f & SKIPVAL) != 0;
if (b.getMaxScore() != null) {
if (a.getMaxScore() == null) {
return ".maxScore missing";