gautamworah96 commented on a change in pull request #181:
URL: https://github.com/apache/lucene/pull/181#discussion_r651418529
##########
File path: lucene/core/src/java/org/apache/lucene/search/MultiCollector.java
##########
@@ -25,13 +25,39 @@
/**
* A {@link Collector} which allows running a search with several {@link
Collector}s. It offers a
* static {@link #wrap} method which accepts a list of collectors and wraps
them with {@link
- * MultiCollector}, while filtering out the <code>null</code> null ones.
+ * MultiCollector}, while filtering out the <code>null</code> ones.
*/
public class MultiCollector implements Collector {
+ /**
+ * Defines the behavior to take when a wrapped {@link Collector} signals
early termination by
+ * throwing a {@link CollectionTerminatedException}.
+ */
+ public enum EarlyTerminationBehavior {
Review comment:
Can we name this `EarlyTerminationCollectorBehavior` everywhere? It just
makes the name a bit more explicit
##########
File path: lucene/core/src/java/org/apache/lucene/search/MultiCollector.java
##########
@@ -78,14 +107,17 @@ public static Collector wrap(Iterable<? extends Collector>
collectors) {
colls[n++] = c;
}
}
- return new MultiCollector(colls);
+ return new MultiCollector(earlyTerminationBehavior, colls);
}
}
+ private final EarlyTerminationBehavior earlyTerminationBehavior;
private final boolean cacheScores;
private final Collector[] collectors;
- private MultiCollector(Collector... collectors) {
+ private MultiCollector(
+ EarlyTerminationBehavior earlyTerminationBehavior, Collector...
collectors) {
Review comment:
How about giving different levels of `EarlyTerminationBehavior`
privileges to each collector? For example, a normal matching docs collector
could terminate all collectors but a `FacetCollector` can only terminate
itself.
This would mean this ctor would take in a `EarlyTerminationBehavior...
earlyTerminationBehavior` array I think? Then hopefully while catching the
exception we can check the collector privilege and accordingly throw the error
or just continue
##########
File path: lucene/core/src/test/org/apache/lucene/search/TestMultiCollector.java
##########
@@ -105,14 +106,38 @@ public void testCollectionTerminatedExceptionHandling()
throws IOException {
final int numCollectors = TestUtil.nextInt(random(), 1, 5);
for (int i = 0; i < numCollectors; ++i) {
final int terminateAfter = random().nextInt(numDocs + 10);
- final int expectedCount = terminateAfter > numDocs ? numDocs :
terminateAfter;
+ final int expectedCount = Math.min(terminateAfter, numDocs);
Review comment:
Neat!
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
