javanna commented on code in PR #15995:
URL: https://github.com/apache/lucene/pull/15995#discussion_r3167325368
##########
lucene/grouping/src/java/org/apache/lucene/search/grouping/TopGroupsCollectorManager.java:
##########
@@ -80,13 +83,15 @@ public TopGroupsCollectorManager(
Collection<SearchGroup<T>> searchGroups,
Sort groupSort,
Sort sortWithinGroup,
+ int withinGroupOffset,
Review Comment:
here too, add it to the javadocs?
##########
lucene/grouping/src/java/org/apache/lucene/search/grouping/TopGroupsCollectorManager.java:
##########
@@ -101,42 +106,25 @@ public TopGroupsCollector<T> newCollector() throws
IOException {
searchGroups,
groupSort,
sortWithinGroup,
- maxDocsPerGroup,
+ withinGroupOffset + maxDocsPerGroup,
getMaxScores);
collectors.add(collector);
return collector;
}
@Override
public TopGroups<T> reduce(Collection<TopGroupsCollector<T>> collectors)
throws IOException {
- if (collectors.isEmpty()) {
- return null;
- }
-
- if (collectors.size() == 1) {
- return collectors.iterator().next().getTopGroups(0);
- }
-
// Merge results from multiple collectors
- List<TopGroups<T>> shardGroupsList = new ArrayList<>();
- for (TopGroupsCollector<T> collector : collectors) {
- TopGroups<T> groups = collector.getTopGroups(0);
- if (groups != null) {
- shardGroupsList.add(groups);
- }
- }
+ List<TopGroups<T>> shardGroupsList = collectors.stream().map(c ->
c.getTopGroups(0)).toList();
- if (shardGroupsList.isEmpty()) {
- return null;
- }
-
- @SuppressWarnings({"unchecked", "rawtypes"})
+ @SuppressWarnings("unchecked")
TopGroups<T>[] shardGroups = (TopGroups<T>[])
shardGroupsList.toArray(TopGroups[]::new);
return TopGroups.merge(
- shardGroups, groupSort, sortWithinGroup, 0, maxDocsPerGroup,
scoreMergeMode);
- }
-
- public List<TopGroupsCollector<T>> getCollectors() {
Review Comment:
++
##########
lucene/grouping/src/java/org/apache/lucene/search/grouping/TopGroupsCollectorManager.java:
##########
@@ -101,42 +106,25 @@ public TopGroupsCollector<T> newCollector() throws
IOException {
searchGroups,
groupSort,
sortWithinGroup,
- maxDocsPerGroup,
+ withinGroupOffset + maxDocsPerGroup,
getMaxScores);
collectors.add(collector);
return collector;
}
@Override
public TopGroups<T> reduce(Collection<TopGroupsCollector<T>> collectors)
throws IOException {
- if (collectors.isEmpty()) {
- return null;
- }
-
- if (collectors.size() == 1) {
- return collectors.iterator().next().getTopGroups(0);
- }
-
// Merge results from multiple collectors
- List<TopGroups<T>> shardGroupsList = new ArrayList<>();
- for (TopGroupsCollector<T> collector : collectors) {
- TopGroups<T> groups = collector.getTopGroups(0);
- if (groups != null) {
- shardGroupsList.add(groups);
- }
- }
+ List<TopGroups<T>> shardGroupsList = collectors.stream().map(c ->
c.getTopGroups(0)).toList();
Review Comment:
thanks for simplifying this!
##########
lucene/grouping/src/java/org/apache/lucene/search/grouping/TopGroupsCollectorManager.java:
##########
@@ -52,13 +53,15 @@ public TopGroupsCollectorManager(
Collection<SearchGroup<T>> searchGroups,
Sort groupSort,
Sort sortWithinGroup,
+ int withinGroupOffset,
int maxDocsPerGroup,
boolean getMaxScores) {
this(
groupSelectorFactory,
searchGroups,
groupSort,
sortWithinGroup,
+ withinGroupOffset,
Review Comment:
could you add this new argument to the constructor params too in the
javadocs?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]