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: bitpacking respects patches and zero bit width cases #2368

Merged
merged 2 commits into from
Feb 17, 2025

Conversation

danking
Copy link
Member

@danking danking commented Feb 14, 2025

Fixes a few issues:

  1. When bit width is zero, we still must apply patches.

  2. When bit width is zero, we should only push zeros into the values because the nulls are already handled three lines above (the call to append_validity)

  3. Tests for many of the cases of bitpacked array.

Looks better with whitespace changes hidden.

Fixes a few issues:

1. When bit width is zero, we still must apply patches.

2. When bit width is zero, we should only push zeros into the values because the nulls are already
   handled three lines above (the call to `append_validity`)

3. Tests for many of the cases of bitpacked array.
@danking danking marked this pull request as ready for review February 14, 2025 21:41
@danking danking enabled auto-merge (squash) February 14, 2025 21:41
if bit_width == 0 {
builder.append_zeros(length);
Copy link
Member Author

Choose a reason for hiding this comment

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

NB: append_zeros also appends is_valids, so we end up with twice the number of validity entries.

Copy link
Member

@robert3005 robert3005 left a comment

Choose a reason for hiding this comment

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

I think it's slightly nicer to use spare_capacity_mut or something similar but that probably requires more changes in bitunpacking code that we can do in a separate pr

@danking danking merged commit 49f45da into develop Feb 17, 2025
23 checks passed
@danking danking deleted the dk/fix-bitpacking-compress branch February 17, 2025 15:18
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.

2 participants