javanna commented on code in PR #13747: URL: https://github.com/apache/lucene/pull/13747#discussion_r1751774184
########## lucene/join/src/java/org/apache/lucene/search/join/JoinUtil.java: ########## @@ -530,49 +528,104 @@ public static Query createJoinQuery( final Query rewrittenFromQuery = searcher.rewrite(fromQuery); final Query rewrittenToQuery = searcher.rewrite(toQuery); - GlobalOrdinalsWithScoreCollector globalOrdinalsWithScoreCollector; + Supplier<GlobalOrdinalsWithScoreCollector> globalOrdinalsWithScoreCollector; + final OrdinalMap finalOrdinalMap = ordinalMap; switch (scoreMode) { case Total: globalOrdinalsWithScoreCollector = - new GlobalOrdinalsWithScoreCollector.Sum(joinField, ordinalMap, valueCount, min, max); + () -> + new GlobalOrdinalsWithScoreCollector.Sum( + joinField, finalOrdinalMap, valueCount, min, max); break; case Min: globalOrdinalsWithScoreCollector = - new GlobalOrdinalsWithScoreCollector.Min(joinField, ordinalMap, valueCount, min, max); + () -> + new GlobalOrdinalsWithScoreCollector.Min( + joinField, finalOrdinalMap, valueCount, min, max); break; case Max: globalOrdinalsWithScoreCollector = - new GlobalOrdinalsWithScoreCollector.Max(joinField, ordinalMap, valueCount, min, max); + () -> + new GlobalOrdinalsWithScoreCollector.Max( + joinField, finalOrdinalMap, valueCount, min, max); break; case Avg: globalOrdinalsWithScoreCollector = - new GlobalOrdinalsWithScoreCollector.Avg(joinField, ordinalMap, valueCount, min, max); + () -> + new GlobalOrdinalsWithScoreCollector.Avg( + joinField, finalOrdinalMap, valueCount, min, max); break; case None: if (min <= 1 && max == Integer.MAX_VALUE) { - GlobalOrdinalsCollector globalOrdinalsCollector = - new GlobalOrdinalsCollector(joinField, ordinalMap, valueCount); - searcher.search(rewrittenFromQuery, globalOrdinalsCollector); + LongBitSet collectedOrdinals = + searcher.search( + rewrittenFromQuery, + new CollectorManager<GlobalOrdinalsCollector, LongBitSet>() { Review Comment: do we expect users to benefit from this collector manager outside of `JoinUtil`? If so maybe it deserves to be in its own outer class, and include the merge method (now called combine) in it. -- 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