Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Search: Reuse Lucene's MultiCollector. #9549

Closed

Conversation

jpountz
Copy link
Contributor

@jpountz jpountz commented Feb 3, 2015

We could reuse Lucene's MultiCollector instead of implementing our own.

We could reuse Lucene's MultiCollector instead of implementing our own.
@rmuir
Copy link
Contributor

rmuir commented Feb 3, 2015

I dont understand how this is safe. is invoking that postCollection() method in XCollector not important?

@jpountz
Copy link
Contributor Author

jpountz commented Feb 3, 2015

This is the reason why I thought we needed it, but when reading this code, I noticed that postCollection is actually never called directly on the main collector, always on the sub collectors if they implement XCollector. The MultiCollector is only created in the method that I modified and the only place where it escapes is through IndexSearcher which does not know about XCollector. Tests also happen to pass with this patch applied.

@jpountz jpountz added the review label Feb 3, 2015
@rmuir
Copy link
Contributor

rmuir commented Feb 3, 2015

ok, +1

@jpountz jpountz closed this in 6cdde31 Feb 3, 2015
@javanna
Copy link
Member

javanna commented Feb 4, 2015

Can you label this please @jpountz ? ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants