Skip to content

fix(encoding): handle empty all-null dictionaries - #8841

Open
lance-gatefixer[bot] wants to merge 3 commits into
mainfrom
gatekeeper/fix-8281-1
Open

fix(encoding): handle empty all-null dictionaries#8841
lance-gatefixer[bot] wants to merge 3 commits into
mainfrom
gatekeeper/fix-8281-1

Conversation

@lance-gatefixer

@lance-gatefixer lance-gatefixer Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • defer empty-dictionary reconstruction until buffered chunks are grouped for a page
  • canonicalize hidden null-key payloads and reuse a neighboring real dictionary when available
  • cover mixed valued/all-null chunks, sliced arbitrary key payloads, and v2.3 compaction

Root cause

A metadata-only all-null Dictionary<Int32, Utf8> column is synthesized with null keys and an empty values array. The structural encoder records key validity as rep/def levels and then removes the Arrow null bitmap. That exposes key payloads which may be arbitrary under Arrow nulls and cannot reference an empty dictionary.

Adding a nullable placeholder per input chunk avoids the initial bounds error but fails when the accumulation queue combines that chunk with valued dictionary data, because the merged dictionary values become an unsupported nullable block.

Fix

Empty dictionary chunks now retain key validity until the buffered page is formed. At flush time their keys are rebuilt as validity-free zeroes and attached to an adjacent non-empty dictionary without adding values. An entirely empty page receives one zeroed non-null placeholder value. Rep/def preserves the logical nullness, so replacement keys are never exposed.

The flush-time reconstruction follows the validated approach in #8010.

Scope

This repairs the independently reproduced re-encode precursor discovered while investigating #8281. It does not resolve the reported post-compaction projection mismatch; #8479 addresses the separate binary-copy mapping path, so #8281 should remain open.

Validation

  • cargo test -p lance-encoding encodings::logical::primitive::tests:: (79 passed)
  • cargo test -p lance test_compact_metadata_only_all_null_dictionary
  • cargo fmt --all -- --check
  • cargo clippy --all --tests --benches -- -D warnings

Refs #8281
Refs #7906

@github-actions github-actions Bot added A-encoding Encoding, IO, file reader/writer bug Something isn't working labels Aug 28, 2026
lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added the K-changes Latest Gatekeeper recommendation requests changes. label Aug 28, 2026
Port the page-grouping approach from #8010 so empty chunks reuse neighboring dictionary values and canonicalize hidden key payloads.

Co-authored-by: Michal Matczuk <mmatczuk@gmail.com>

Co-authored-by: Beau Hartshorne <beau@hartshorne.ca>
@lance-gatefixer

Copy link
Copy Markdown
Contributor Author

Addressed in 9538d28: reconstruction now runs after page grouping, following #8010, with validity-free zeroed keys, neighboring dictionary reuse, and a non-null placeholder only for all-empty pages. The PR now uses Refs #8281 and explicitly leaves the projection issue open.

@lance-gatekeeper lance-gatekeeper Bot removed the K-changes Latest Gatekeeper recommendation requests changes. label Aug 28, 2026
lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Aug 28, 2026
@lance-gatekeeper lance-gatekeeper Bot removed the K-approved Latest Gatekeeper recommendation permits acceptance. label Aug 28, 2026

@lance-gatekeeper lance-gatekeeper Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Gate recommendation: approve.

The rebase leaves the reviewed flush-time empty-dictionary reconstruction unchanged: it preserves rep/def nullness, canonicalizes empty keys after page grouping, reuses neighboring values, and adds a placeholder only for all-empty pages. The focused encoder and v2.3 compaction regressions still pass on this head, while the separate #8281 projection defect remains open.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Aug 28, 2026
@lance-gatefixer

Copy link
Copy Markdown
Contributor Author

Blocked: The Ready PR at caa84cb remains blocked by Python / Python Linux 3.10 x86_64 (run 33157082331, job 98803747218) failing with StopIteration in test_torch_index_with_nans[Legacy].

The current main tip 29c4d59 is an ancestor of this head. That exact main tip passed the same Python 3.10 job (run 33155824061, job 98799400609); this repair changes only dictionary encoding and its regression coverage, while the focused failing test passed four consecutive local runs in the prescribed uv environment. No attributable repair change is supported by the available evidence.

Please rerun the failed Python 3.10 check. If it fails again, the practical alternative is to isolate and repair the randomized Torch PQ sampling failure independently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-encoding Encoding, IO, file reader/writer bug Something isn't working K-approved Latest Gatekeeper recommendation permits acceptance.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants