Skip to content

fix(encoding): support sparse boolean lists in full-zip encoding - #6723

Merged
Xuanwo merged 2 commits into
lance-format:mainfrom
shenganzhang:fix/6681-sparse-boolean-fullzip-panic
May 19, 2026
Merged

fix(encoding): support sparse boolean lists in full-zip encoding#6723
Xuanwo merged 2 commits into
lance-format:mainfrom
shenganzhang:fix/6681-sparse-boolean-fullzip-panic

Conversation

@shenganzhang

@shenganzhang shenganzhang commented May 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #6681.

Writing a list<bool> column where most rows are null (sparse) panicked with:

Non-byte aligned full-zip compression not yet supported

The root cause: #6234 added a fallback from miniblock to full-zip encoding when rep/def levels are too sparse for a miniblock chunk. The full-zip path assumed all values are byte-aligned, but booleans are stored as 1-bit packed values in Arrow — triggering the assert in serialize_full_zip_fixed.

Fix (two small, paired changes):

  • Encoder (encode_full_zip): when the data block is 1-bit boolean, expand each bit to a full byte before compression. A new helper expand_boolean_to_bytes uses BooleanBuffer::value() to read bits correctly.
  • Decoder (FixedWidthDataBlock::do_into_arrow): when a Boolean field is decoded with bits_per_value == 8 (the expanded representation), re-pack the bytes back to Arrow's bit-packed format before building the ArrayData.

Test plan

  • Added test_sparse_boolean_list_roundtrip — 1000-row list<bool> with ~1/64 non-null rows and mixed true/false values; exercises both V2.1 and V2.2 paths. Test was failing before the fix (panic) and passes after.
  • Full lance-encoding test suite: 370 tests, 0 failures.

Booleans encoded as 1-bit packed values cannot be interleaved byte-by-byte
in the full-zip layout, which caused a panic ("Non-byte aligned full-zip
compression not yet supported") when a list of booleans was sparse enough
to trigger the miniblock → full-zip fallback (introduced in lance-format#6234).

Fix: expand 1-bit boolean values to 1 byte each before full-zip encoding,
and re-pack bytes to Arrow's bit-packed format on decode.

Closes lance-format#6681

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@github-actions github-actions Bot added the bug Something isn't working label May 9, 2026
@codecov

codecov Bot commented May 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@hamersaw

Copy link
Copy Markdown
Contributor

I believe this is the same fix as #6777. @Xuanwo maybe you can elaborate on why this is not a great fix and the other proposed solution?

@westonpace westonpace left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi, thanks for looking into this! I'd rather not fallback to full-zip in these cases (see #6841) but this is a good workaround for the current solution.

@westonpace

Copy link
Copy Markdown
Member

With #6723 we probably won't hit this case anymore but I think it might still be nice to merge this in for completeness sake (a user could always opt-in to full-zip encoding with boolean data for some reason)

@Xuanwo

Xuanwo commented May 19, 2026

Copy link
Copy Markdown
Member

I believe this is the same fix as #6777. @Xuanwo maybe you can elaborate on why this is not a great fix and the other proposed solution?

Yes, this is the same fix as #6777. I don't like those two because I believe those two PRs are just workaround that trying to address this issue at the wrong level.

So I proposed the change at #6787. However, I did agree that #6787 is much more complex than this one thus need more time for reviewing and testing.

I second with @westonpace's judgement and decided to merge this in first and give #6787 more time.

@Xuanwo
Xuanwo merged commit 5be79b3 into lance-format:main May 19, 2026
28 checks passed
wjones127 pushed a commit that referenced this pull request May 20, 2026
## Summary

Fixes #6681.

Writing a `list<bool>` column where most rows are null (sparse) panicked
with:

```
Non-byte aligned full-zip compression not yet supported
```

The root cause: #6234 added a fallback from miniblock to full-zip
encoding when rep/def levels are too sparse for a miniblock chunk. The
full-zip path assumed all values are byte-aligned, but booleans are
stored as 1-bit packed values in Arrow — triggering the assert in
`serialize_full_zip_fixed`.

**Fix (two small, paired changes):**

- **Encoder** (`encode_full_zip`): when the data block is 1-bit boolean,
expand each bit to a full byte before compression. A new helper
`expand_boolean_to_bytes` uses `BooleanBuffer::value()` to read bits
correctly.
- **Decoder** (`FixedWidthDataBlock::do_into_arrow`): when a `Boolean`
field is decoded with `bits_per_value == 8` (the expanded
representation), re-pack the bytes back to Arrow's bit-packed format
before building the `ArrayData`.

## Test plan

- Added `test_sparse_boolean_list_roundtrip` — 1000-row `list<bool>`
with ~1/64 non-null rows and mixed `true`/`false` values; exercises both
V2.1 and V2.2 paths. Test was failing before the fix (panic) and passes
after.
- Full `lance-encoding` test suite: 370 tests, 0 failures.

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
(cherry picked from commit 5be79b3)
zhangyue19921010 added a commit that referenced this pull request Sep 2, 2026
…ack (#8934)

When a single top-level row carries more rep/def levels than one
mini-block chunk can hold, the primitive encoder falls back to full-zip
after a pre-check that the value block is something full-zip can
serialize. That pre-check rejected 1-bit booleans as non-byte-aligned,
although `encode_full_zip` widens them to bytes before compressing
(#6723).

A sparse `List<List<Boolean>>` row therefore failed with "Mini-block
cannot encode N rep/def levels in one top-level row" even though it
encodes fine, and even when the user explicitly requested
`structural_encoding=fullzip`. Before #6787 the same row was written
through full-zip.

The pre-check now lets 1-bit fixed-width blocks through, matching what
`encode_full_zip` accepts. The boolean test that asserted the error now
asserts a full-zip round trip alongside the existing string case.
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.

Writing sparse boolean arrays using 2.1 and 2.2 format versions panics with Non-byte aligned full-zip compression not yet supported

4 participants