gsmiller commented on code in PR #13702:
URL: https://github.com/apache/lucene/pull/13702#discussion_r1742791755


##########
lucene/facet/src/java/org/apache/lucene/facet/DrillSideways.java:
##########
@@ -480,59 +462,61 @@ private void searchSequentially(
     }
     Query[] drillDownQueries = query.getDrillDownQueries();
 
-    DrillSidewaysQuery dsq =
-        new DrillSidewaysQuery(
-            baseQuery,
-            // drillDownCollectorOwner,
-            // Don't pass drill down collector because drill down is collected 
by IndexSearcher
-            // itself.
-            // TODO: deprecate drillDown collection in DrillSidewaysQuery?
-            null,
-            drillSidewaysCollectorOwners,
-            drillDownQueries,
-            scoreSubDocsAtOnce());
-
-    searcher.search(dsq, drillDownCollectorOwner);
-    // This method doesn't return results as each dimension might have its own 
result type.
-    // But we call getResult to trigger results reducing, so that users don't 
have to worry about
-    // it.
-    drillDownCollectorOwner.getResult();
-    if (drillSidewaysCollectorOwners != null) {
-      for (CollectorOwner<?, ?> sidewaysOwner : drillSidewaysCollectorOwners) {
-        sidewaysOwner.getResult();
+    DrillSidewaysQuery<K, R> dsq =
+        new DrillSidewaysQuery<>(
+            baseQuery, drillSidewaysCollectorManagers, drillDownQueries, 
scoreSubDocsAtOnce());
+
+    T collectorResult = searcher.search(dsq, drillDownCollectorManager);
+    List<R> drillSidewaysResults = new ArrayList<>(drillDownDims.size());
+    assert drillSidewaysCollectorManagers != null
+        : "Case without drill sideways dimensions is handled above";
+    int numSlices = dsq.managedDrillSidewaysCollectors.size();
+    for (int dim = 0; dim < drillDownDims.size(); dim++) {
+      List<K> collectorsForDim = new ArrayList<>(numSlices);
+      for (int slice = 0; slice < numSlices; slice++) {
+        
collectorsForDim.add(dsq.managedDrillSidewaysCollectors.get(slice).get(dim));
       }
+      drillSidewaysResults.add(
+          dim, 
drillSidewaysCollectorManagers.get(dim).reduce(collectorsForDim));
     }
+    return new Result<>(collectorResult, drillSidewaysResults);
   }
 
-  private void searchConcurrently(
+  private <C extends Collector, T, K extends Collector, R> Result<T, R> 
searchConcurrently(
       final DrillDownQuery query,
-      final CollectorOwner<?, ?> drillDownCollectorOwner,
-      final List<CollectorOwner<?, ?>> drillSidewaysCollectorOwners)
+      final CollectorManager<C, T> drillDownCollectorManager,
+      final List<CollectorManager<K, R>> drillSidewaysCollectorManagers)
       throws IOException {

Review Comment:
   Yeah, we could do that, but I think I'd be more inclined to leave it as-is 
for now and explore the idea of moving the "sideways" functionality into 
IndexSearcher as a supported pattern there, and fully deprecate `DrillSideways` 
as its own entry point. If we did that, we could leave out the concurrent 
version of this algorithm in IndexSearcher initially and it could be ported 
later if a valid use-case comes up?



-- 
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: issues-unsubscr...@lucene.apache.org

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

Reply via email to