Skip to content

Conversation

jainankitk
Copy link
Contributor

Description

Reusing count() for minor refactor in SortedNumericDocValuesRangeQuery

Copy link
Contributor

This PR does not have an entry in lucene/CHANGES.txt. Consider adding one. If the PR doesn't need a changelog entry, then add the skip-changelog label to it and you will stop receiving this reminder on future updates to the PR.

@jainankitk jainankitk added the skip-changelog Apply to PRs that don't need a changelog entry, stopping the automated changelog check. label Aug 25, 2025
@jpountz
Copy link
Contributor

jpountz commented Aug 25, 2025

Javadocs of LeafReader#numDocs warn that it may run in O(maxDoc) time. I wonder if we should instead extract a method that tells whether it matches all or nothing (without calling numDocs()) and then use it from count() (plus calling numDocs()) and scorerSupplier()?

@jainankitk
Copy link
Contributor Author

That's a good point. I was initially thinking of doing that by adding relate method similar to PointRangeQuery, but thought this could be simpler. I overlooked this key detail regarding LeafReader#numDocs potentially taking O(maxDoc) time.

Signed-off-by: Ankit Jain <[email protected]>
/* Returning 1 instead of LeafReader#numDocs as it may run in O(maxDoc)
* which is unnecessary when docCount is invoked from ScorerSupplier
*/
private int docCount(LeafReaderContext context) throws IOException {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if calling it docCountIgnoringDeletes and returning maxDoc instead of 1 would make the contract of this method a bit less awkward?

@jainankitk jainankitk merged commit 7ccabaf into apache:main Aug 27, 2025
8 checks passed
@jainankitk jainankitk deleted the sorted-dv branch August 27, 2025 17:16
jainankitk added a commit that referenced this pull request Aug 27, 2025
@jainankitk jainankitk added this to the 10.3.0 milestone Aug 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module:core/other skip-changelog Apply to PRs that don't need a changelog entry, stopping the automated changelog check.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants