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