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

Implement Weight#count for vector values in the FieldExistsQuery #13322

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

bugmakerrrrrr
Copy link
Contributor

Description

Considering only one vector value can be indexed per doc, we can use the Float/ByteVectorValues#size method to implement Weight#count for vector values in the FieldExistsQuery

@benwtrent
Copy link
Member

I like this. Could you add an entry under "Optimizations" in "CHANGES.txt"?

Copy link
Contributor

@jpountz jpountz left a comment

Choose a reason for hiding this comment

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

I don't remember how vectors handle ghost fields, could this trigger a NPE if a field indexes vectors, then all docs that have vectors get merged away?

This looks like something that could go into 9.11, let's move the CHANGES entry under 9.11?

@bugmakerrrrrr
Copy link
Contributor Author

I don't remember how vectors handle ghost fields, could this trigger a NPE if a field indexes vectors, then all docs that have vectors get merged away?

The test shows that vectors will return a empty instance for ghost fields, but for safety reasons, I added a null checks.

This looks like something that could go into 9.11, let's move the CHANGES entry under 9.11?

Done.

Copy link
Contributor

@jpountz jpountz left a comment

Choose a reason for hiding this comment

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

I'm not comfortable with using DocIdSetIterator#cost since it has no accuracy guarantee, could you use FloatVectorValues#size() instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants