Skip to content

fix(encoding): normalize out-of-range null dictionary keys - #2

Merged
valkum merged 1 commit into
instant-labs:release/6.x-instantfrom
valkum:backport/dict-out-of-range-null-keys-6.x
Aug 28, 2026
Merged

fix(encoding): normalize out-of-range null dictionary keys#2
valkum merged 1 commit into
instant-labs:release/6.x-instantfrom
valkum:backport/dict-out-of-range-null-keys-6.x

Conversation

@valkum

@valkum valkum commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

Backport of upstream lance-format/lance#8827 (head b855eca8), which fixes #8826: the encoder rejects a dictionary array that holds an out-of-range physical key in a null slot.

Root cause. Arrow permits an arbitrary physical key in a null dictionary slot. The structural encoder records the key validity in rep-def and then removes the null buffer (extract_validity_buf). The meaningless key then looks valid and fails the dictionary bounds check. arrow_select::concat makes such keys when it merges a chunk that has values with an all-null chunk.

Fix. Before validity extraction, rebuild the keys from the logical iterator when a non-empty dictionary holds an out-of-range key in a null slot. This writes key zero into every null slot and keeps the logical values and the validity bitmap. A dictionary without the defect returns unchanged, so the no-allocation path stays.

Backport notes

  • The production code is byte-identical to the upstream PR head: clear_out_of_range_null_keys in dict.rs, plus one call in PrimitiveStructuralEncoder::extract_validity. git cherry-pick -x applied with no conflict.
  • One test adaptation. Upstream selects the structural encoder with .with_encoding(TestEncoding::StructuralU32). This branch has no TestEncoding selector, so the test uses .with_min_file_version(LanceFileVersion::V2_1), the same as the dictionary tests next to it.
  • No conflict with this branch's empty-dictionary work (rebuild_empty_dictionary_chunks, fix(encoding): align all-null dictionary handling with upstream #1). The new function skips an empty dictionary (num_values == 0), which do_flush continues to handle.
  • Upstream PR fix(encoding): normalize out-of-range null dictionary keys lance-format/lance#8827 is still open. It has a gatekeeper approve recommendation (review 5051200469) but no merge. If upstream changes the approach before it merges, this backport needs a follow-up.

Tests

test_dictionary_out_of_range_null_keys_round_trip covers two sources of the defect: a hand-built dictionary, and one that arrow_select::concat produces from a valued chunk and an all-null chunk.

The test is not vacuous on this branch. With the call in extract_validity removed, both cases fail:

Invalid argument error: Value at position 1 out of bounds: 7 (should be in [0, 0])
  at rust/lance-encoding/src/encodings/logical/primitive.rs:5510
  • cargo test -p lance-encoding — 382 passed, 0 failed, 5 ignored
  • cargo fmt --all -- --check — clean
  • cargo clippy --all --tests --benches -- -D warnings — clean

🤖 Generated with Claude Code

https://claude.ai/code/session_0116qdBmoaQpMLZAWoi6evmA

Arrow permits an arbitrary physical key in a null dictionary slot. The
structural encoder records the key validity in rep-def and then removes the
null buffer, which makes such a key look valid and fails the dictionary
bounds check. Rebuild the keys from the logical iterator when a non-empty
dictionary holds an out-of-range key in a null slot. A dictionary without
the defect stays untouched.

Backport of lance-format#8827 (head b855eca).

Adaptation for 6.x: the round trip test selects the structural encoder with
with_min_file_version(LanceFileVersion::V2_1). This branch has no
TestEncoding selector.

(cherry picked from commit b855eca)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0116qdBmoaQpMLZAWoi6evmA
@valkum
valkum merged commit 2b17e81 into instant-labs:release/6.x-instant Aug 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant