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