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


##########
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:
   > nit: this can no longer throw IOException so you can pull it
   
   Done, thanks!
   
   > It also drives me a little nuts that we even have this concurrent 
implementation. [...]
   
   My theory is that concurrent drill-sideways predates inter segment 
concurrency support in IndexSearcher, so it was faster some time ago, but 
probably not anymore? It might still work faster for some edge cases, e.g. if 
baseQuery is `MatchAllDocsQuery` and there is no overlap between dimensions' 
match sets ?
   
   In case someone still relies on it for some reason, we can mark 
`DrillSideways` constructors that enable `concurrentSearch` as `@Deprecated` in 
9.x branch, and deprecate it in 10, what do you think?



-- 
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