-
Notifications
You must be signed in to change notification settings - Fork 126
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
Added null checks for fieldInfo in ExactSearcher to avoid NPE while running exact search for segments with no vector field #2278
Conversation
0fd48b2
to
ea65786
Compare
dade02b
to
ffd90fc
Compare
src/main/java/org/opensearch/knn/index/query/ExactSearcher.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/knn/index/query/ExactSearcher.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/knn/index/query/nativelib/NativeEngineKnnVectorQuery.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/knn/index/query/nativelib/NativeEngineKnnVectorQuery.java
Outdated
Show resolved
Hide resolved
2b6a3b1
to
6b758f6
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.
Please fix the CI
seems to be related to new dependency upgrades. Will check |
ddc5ffd
to
33f05a5
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.
LGTM, minor comment around testing
src/test/java/org/opensearch/knn/index/query/ExactSearcherTests.java
Outdated
Show resolved
Hide resolved
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.
Looks good to me :)
…unning exact search for segments with no vector field Signed-off-by: Navneet Verma <navneev@amazon.com>
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.
LGTM
@heemin32 , @jmazanec15 and @shatejas can you guys review this PR. |
Description
Added null checks for fieldInfo in ExactSearcher to avoid NPE while running exact search for segments with no vector field
Ensured that in ExactSearcher class, if fieldInfo is null for a field no results are returned. This is similar to what we have to ANNSearch ref: https://github.com/opensearch-project/k-NN/blob/main/src/main/java/org/opensearch/knn/index/query/KNNWeight.java#L232-L236.
Related Issues
Resolves #2277
Check List
New functionality has been documented.API changes companion pull request created.--signoff
.Public documentation issue/PR created.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.