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

Relax check on patch_bits overflows in delta decoding #118

Merged
merged 1 commit into from
Aug 6, 2024

Conversation

progval
Copy link
Contributor

@progval progval commented Aug 6, 2024

For some reason, some files written with pyorc have a patch_bit_width larger than needing, causing the previous check to fail, even when decoding to u64.

This changes the check to only fail when the patch bits actually overflow, instead of checking whether they may overflow.

Unfortunately, I can't provide a test. Even when writing the exact same values with the same version of pyorc, the generated file does not trigger the error like https://softwareheritage.s3.amazonaws.com/graph/2024-05-16/orc/revision/revision-0c45576a-59f7-48d1-a9a8-2e5c64098905.orc

Closes #97

For some reason, some files written with pyorc have a `patch_bit_width`
larger than needing, causing the previous check to fail, even when
decoding to `u64`.

This changes the check to only fail when the patch bits actually
overflow, instead of checking whether they may overflow.

Closes datafusion-contrib#97
Copy link

codecov bot commented Aug 6, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 4 lines in your changes missing coverage. Please review.

Project coverage is 84.27%. Comparing base (424b021) to head (86e12ca).
Report is 125 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #118      +/-   ##
==========================================
+ Coverage   77.22%   84.27%   +7.04%     
==========================================
  Files          34       36       +2     
  Lines        3302     3898     +596     
==========================================
+ Hits         2550     3285     +735     
+ Misses        752      613     -139     

Copy link
Collaborator

@WenyXu WenyXu left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks!

@WenyXu WenyXu merged commit dc4e342 into datafusion-contrib:main Aug 6, 2024
10 checks passed
@progval progval deleted the patch-overflow branch August 28, 2024 19:40
waynexia pushed a commit that referenced this pull request Oct 24, 2024
For some reason, some files written with pyorc have a `patch_bit_width`
larger than needing, causing the previous check to fail, even when
decoding to `u64`.

This changes the check to only fail when the patch bits actually
overflow, instead of checking whether they may overflow.

Closes #97
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants