apache / lucene

Apache Lucene open-source search software
https://lucene.apache.org/
Apache License 2.0
2.72k stars 1.04k forks source link

Remove all deprecated IndexSearcher#search(Query, Collector) usage / methods in the next major release #12892

Open zacharymorn opened 11 months ago

zacharymorn commented 11 months ago

Description

As a follow-up of https://github.com/apache/lucene/issues/11041, we would like to remove all deprecated IndexSearcher#search(Query, Collector) methods in the next major release, preferably 10.0.0.

A list of the leftover usages follows:

javanna commented 4 months ago

I have been looking into this, there are unfortunately ~85 leftover usages of this method. Would be great to clean this up for Lucene 10. I opened a few small PRs around this.

I think though that FacetsCollector needs extensive rework to leverage collector managers instead of collectors. I think it would be much simpler if the static search methods that FacetsCollector exposes took a FacetsCollector as last argument instead of a generic Collector. That seems to cover all of our internal usages, but I am not sure what the expectations are around users usage for such methods. We should discuss how we want to revisit the facets API as a whole.

javanna commented 3 months ago

I edited the description of this issue to include an extensive list of the leftover usages that need fixing.

gsmiller commented 2 months ago

Opened #13735 for the monitor usage

msfroh commented 2 months ago

Opened https://github.com/apache/lucene/pull/13747 for join/join.test.

gsmiller commented 2 months ago

Note that we added a factory method in CollectorManager that might make these migration tasks a little easier for cases where we don't need to leverage the concurrency offered by IndexSearcher (or if there are cases where we want to follow up later to add the concurrency support). See CollectorManager#forSequentialExecution (and thanks to @romseygeek for the idea!).

EDIT: After more consideration, this is probably too trappy for users and is likely getting removed (#13790) (FYI @msfroh)