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

fix: handle null vectors in flat search #3422

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

wjones127
Copy link
Contributor

No description provided.

@github-actions github-actions bot added the bug Something isn't working label Jan 28, 2025
Comment on lines +108 to +110
if code.is_empty() {
return Vec::new();
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change was needed to get the existing test to pass in CI. It does seem suspicious though: why would we get an empty code book in PQ if we have nulls?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it would be fine to merge this for now, to fix the immediate bug, but to come back to this an investigate later.

@wjones127 wjones127 marked this pull request as ready for review January 28, 2025 18:20
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 98.59155% with 1 line in your changes missing coverage. Please review.

Project coverage is 78.89%. Comparing base (bfacd7c) to head (98e0f88).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
rust/lance-index/src/vector/pq/distance.rs 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3422      +/-   ##
==========================================
+ Coverage   78.85%   78.89%   +0.03%     
==========================================
  Files         250      250              
  Lines       91474    91602     +128     
  Branches    91474    91602     +128     
==========================================
+ Hits        72134    72265     +131     
+ Misses      16379    16375       -4     
- Partials     2961     2962       +1     
Flag Coverage Δ
unittests 78.89% <98.59%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@wjones127 wjones127 merged commit 7aa7d94 into lancedb:main Jan 28, 2025
26 of 27 checks passed
@wjones127 wjones127 deleted the fix/index-nulls branch January 28, 2025 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants