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

feat: finish up variable-length encodings in the full-zip path #3344

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

Conversation

westonpace
Copy link
Contributor

This adds the last structural path for 2.1, full zip encoding of variable length data. Scheduling this turned out to be a little trickier than I had planned. There is no easy way to know where to slice the fully-zipped buffer when doing decoding. Currently we settle this problem by unzipping in the indirect scheduling task. There are some alternative possibilities that I have documented but for now I think this will be good enough and we can iterate on this going forwards.

@westonpace westonpace marked this pull request as draft January 7, 2025 15:14
@github-actions github-actions bot added the enhancement New feature or request label Jan 7, 2025
@westonpace
Copy link
Contributor Author

westonpace commented Jan 7, 2025

Leaving in draft until #3335 merges

@westonpace westonpace force-pushed the feat/full-zip-rep-index-variable branch from 76c37bd to cd740a3 Compare January 7, 2025 22:06
@westonpace westonpace marked this pull request as ready for review January 7, 2025 22:06
@codecov-commenter
Copy link

codecov-commenter commented Jan 7, 2025

Codecov Report

Attention: Patch coverage is 85.71429% with 71 lines in your changes missing coverage. Please review.

Project coverage is 78.73%. Comparing base (4cb37c1) to head (82dd08c).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
.../lance-encoding/src/encodings/logical/primitive.rs 90.09% 26 Missing and 7 partials ⚠️
rust/lance-encoding/src/encodings/physical/fsst.rs 60.52% 13 Missing and 2 partials ⚠️
rust/lance-encoding/src/data.rs 0.00% 9 Missing ⚠️
rust/lance-file/src/v2/reader.rs 0.00% 7 Missing ⚠️
rust/lance-encoding/src/encoder.rs 85.00% 3 Missing ⚠️
rust/lance-encoding/src/decoder.rs 85.71% 2 Missing ⚠️
rust/lance-encoding/src/encodings/physical.rs 0.00% 1 Missing ⚠️
...st/lance-encoding/src/encodings/physical/binary.rs 90.90% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3344      +/-   ##
==========================================
+ Coverage   78.68%   78.73%   +0.05%     
==========================================
  Files         250      250              
  Lines       90734    91243     +509     
  Branches    90734    91243     +509     
==========================================
+ Hits        71391    71842     +451     
- Misses      16406    16453      +47     
- Partials     2937     2948      +11     
Flag Coverage Δ
unittests 78.73% <85.71%> (+0.05%) ⬆️

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.

@westonpace westonpace force-pushed the feat/full-zip-rep-index-variable branch from 99b0064 to b2f7ebc Compare January 20, 2025 13:06
@westonpace westonpace force-pushed the feat/full-zip-rep-index-variable branch from b2f7ebc to 82dd08c Compare January 20, 2025 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants