-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Introduce IndexSearcher#searchLeaf(LeafReaderContext, Weight, Collector) method #13603
Conversation
…or) method There's a couple of places in the codebase where we extend IndexSearcher to customize per leaf behaviour, and in order to do that, we need to override the entire search method that loops through the leaves. A good example is ScorerIndexSearcher. Adding a searchLeaf method that provides the per leaf behaviour makes those cases a little easier to deal with. Also, it paves the road for apache#13542 where we will want to replace search(List<LeafReaderContext>, Weight, Collector) with something along the lines of search(LeafReaderContextPartition[], Weight, Collector).
} | ||
// Note: this is called if collection ran successfully, including the above special cases of | ||
// CollectionTerminatedException and TimeExceededException, but no other exception. | ||
leafCollector.finish(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The diff is misleading: this is a pure cut and paste of the code from within the existing loop, to the new method that's called for every entry.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a reasonable API extension point to me. LGTM (minus a CHANGES entry).
Thanks a lot for your review @gsmiller , much appreciated, and for the reminder around the CHANGES entry. Would you like to review the wording perhaps? Otherwise, this should be good to go. My thinking is that it can be backported to branch_9x as well. |
Thank you again for the review @gsmiller ! |
…or) method (apache#13603) There's a couple of places in the codebase where we extend `IndexSearcher` to customize per leaf behaviour, and in order to do that, we need to override the entire search method that loops through the leaves. A good example is `ScorerIndexSearcher`. Adding a `searchLeaf` method that provides the per leaf behaviour makes those cases a little easier to deal with.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
@javanna yeah no problem. All looks good to me. +1 to backporting to 9.12. Thanks! |
…or) method (#13603) There's a couple of places in the codebase where we extend `IndexSearcher` to customize per leaf behaviour, and in order to do that, we need to override the entire search method that loops through the leaves. A good example is `ScorerIndexSearcher`. Adding a `searchLeaf` method that provides the per leaf behaviour makes those cases a little easier to deal with.
There's a couple of places in the codebase where we extend IndexSearcher to customize per leaf behaviour, and in order to do that, we need to override the entire search method that loops through the leaves. A good example is ScorerIndexSearcher.
Adding a searchLeaf method that provides the per leaf behaviour makes those cases a little easier to deal with. Also, it paves the road for #13542 where we will want to replace
search(List<LeafReaderContext>, Weight, Collector)
with something along the lines ofsearch(LeafReaderContextPartition[], Weight, Collector)
. Such switch will be made easier by no longer having extensions ofIndexSearcher
that provide per-leaf behaviour by overriding the former method.