Make APIs work per-segment like Lucene's Collector. #10389
Conversation
|
Hmm, I had not realized this PR was getting so large... If you need any guidance for the reviews please let me know. There is one place where the refactoring was not straightforward: For the record, this PR will break existing script plugins as it splits SearchScript into SearchScript and LeafSearchScript, the former being only responsible for creating per-leaf LeafSearchScript objects. I will fix them when this PR gets in. /cc @dadoonet |
|
Thank you so much @jpountz! |
|
@rjernst FYI I assigned you for the review like we discussed. |
| if (this.reader == context.reader()) { // if we are called with the same reader, don't invalidate source | ||
| public void setSource(LeafReaderContext context, int docId) { | ||
| if (this.reader == context.reader() && this.docId == docId) { | ||
| // if we are called with the same docId, don't invalidate source |
There was a problem hiding this comment.
maybe say document instead of docId? since the check is technically same docId in the same segment?
|
LGTM, just a couple minor comments. |
|
I pushed a new commit that addresses comments. Will merge soon. |
We still have a lot of APIs that use setNextReader in order to change the current segment that should be considered. This commit moves such APIs to getLeafXXX() instead to be more in-line with Lucene 5's collector API. I also renamed setDocId to setDocument to be more in-line with the doc values APIs. Close elastic#10389
jpountz
force-pushed
the
enhancement/per_segment_apis
branch
from
85792a8
to
967a4e2
Compare
Internal: Make APIs work per-segment like Lucene's Collector. Close #10389
See elastic/elasticsearch#10389 for background.
See elastic/elasticsearch#10389 for background.
See elastic/elasticsearch#10389 for background.
clintongormley
changed the title
We still have a lot of APIs that use setNextReader in order to change the
current segment that should be considered. This commit moves such APIs to
getLeafXXX() instead to be more in-line with Lucene 5's collector API.
I also renamed setDocId to setDocument to be more in-line with the doc values
APIs.