-
Notifications
You must be signed in to change notification settings - Fork 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
Check ahead if we can get the count #13899
Conversation
efd0115
to
980f7ea
Compare
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 logic makes sense to me but it's a bit hard to read, could we avoid touching getDocIdSetIteratorOrNull
and only have new logic in the Weight#count
impl?
980f7ea
to
105e3c2
Compare
Thank you for your feedback! @jpountz I really appreciate your suggestion. I’ve made the changes as you recommended |
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.
Thanks, the code makes sense and I see that we already have test coverage for counting for our various optimizations.
IteratorAndCount itAndCount = getDocIdSetIteratorOrNull(context); | ||
if (lowerValue > upperValue) { | ||
return 0; | ||
} |
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 could be moved before the check of whether the segment has deletes?
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.
if lowerValue > upperValue
and has deletes, PointRangeQuery
's count will reture -1.
I’ll temporarily maintain the consistency of the two queries for now. Later, we can consider whether to add this optimization to the PointRangeQuery
. @jpountz
c0b5a35
to
fa7274a
Compare
Currently, we traverse the BKD tree or perform a binary search using DocValues first, and then check whether the count can be obtained in the count() method of IndexSortSortedNumericDocValuesRangeQuery. we should consider providing a mechanism to perform this check beforehand, avoid unnecessary processing when dealing with a sparseRange
Check whether the IndexSortSortedNumericDocValuesRangeQuery's count can be obtained before traversing the BKD tree or performing binary search using DocValues.
see #13890