gaobinlong commented on PR #15557: URL: https://github.com/apache/lucene/pull/15557#issuecomment-4244176649
Thanks for your comment, @javanna , > Can it be used by users as-is like such test does? Yes, I'm sure it can be used by users like the test cases. > Are there other places where Lucene itself could rely on this new collector manager in production code? it seems like GroupingSearch is a good candidate but it relies on MultiCollector which complicates things quite a bit and needs some rework Yes, I noticed that in [GroupingSearch](https://github.com/apache/lucene/blob/55b4574a20d21f7fdc783170babcdd7683ad2a3f/lucene/grouping/src/java/org/apache/lucene/search/grouping/GroupingSearch.java#L148), and we need to replace the AllGroupsCollector with AllGroupsCollectorManager, but we need to also introduce FirstPassGroupCollectorManger and AllGroupHeadsCollectorManager, then use [MultiCollectorManager](https://github.com/apache/lucene/blob/55b4574a20d21f7fdc783170babcdd7683ad2a3f/lucene/core/src/java/org/apache/lucene/search/MultiCollectorManager.java#L28) to wrap all of the collectorManager into one, then it works. > I am also not entirely sure about the Collection<?> generic type, does that need to be as broad? Yeah, this is a problem, I've refactored the code to use <T> instead, thanks! -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
