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

Introduce IndexSearcher#searchLeaf(LeafReaderContext, Weight, Collector) method #13603

Merged
merged 3 commits into from
Jul 30, 2024

Conversation

javanna
Copy link
Contributor

@javanna javanna commented Jul 23, 2024

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 of search(LeafReaderContextPartition[], Weight, Collector). Such switch will be made easier by no longer having extensions of IndexSearcher that provide per-leaf behaviour by overriding the former method.

…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();
Copy link
Contributor Author

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.

Copy link
Contributor

@gsmiller gsmiller left a 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).

@javanna javanna added this to the 9.12.0 milestone Jul 30, 2024
@javanna
Copy link
Contributor Author

javanna commented Jul 30, 2024

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.

@javanna javanna merged commit 30c965e into apache:main Jul 30, 2024
3 checks passed
@javanna
Copy link
Contributor Author

javanna commented Jul 30, 2024

Thank you again for the review @gsmiller !

javanna added a commit to javanna/lucene that referenced this pull request Jul 30, 2024
…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.
Copy link
Contributor

@gsmiller gsmiller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@gsmiller
Copy link
Contributor

@javanna yeah no problem. All looks good to me. +1 to backporting to 9.12. Thanks!

javanna added a commit that referenced this pull request Jul 30, 2024
…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.
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.

2 participants