Skip to content

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

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

fix(encoding): normalize out-of-range null dictionary keys#8827
lance-gatefixer[bot] wants to merge 2 commits into
mainfrom
gatekeeper/fix-8826-1

Conversation

@lance-gatefixer

Copy link
Copy Markdown
Contributor

Summary

  • normalize out-of-range physical dictionary keys in null slots before structural validity extraction
  • preserve the zero-copy fast path for already-safe dictionaries
  • cover both hand-built and Arrow-concatenated reproductions

Root cause

Arrow permits arbitrary physical keys in null dictionary slots. The structural encoder records the key validity in rep-def and then removes the Arrow null buffer, which makes those previously meaningless keys appear valid and triggers dictionary bounds validation.

Fix

When a non-empty dictionary has an out-of-range key in a null slot, rebuild only its keys from the logical iterator. This writes key zero into null slots while preserving logical nullness and dictionary values. Dictionaries without the defect remain untouched.

Validation

  • cargo test -p lance-encoding test_dictionary_out_of_range_null_keys_round_trip -- --nocapture
  • cargo test -p lance-encoding
  • cargo fmt --all -- --check
  • cargo clippy --all --tests --benches -- -D warnings

Fixes #8826

@github-actions github-actions Bot added bug Something isn't working A-encoding Encoding, IO, file reader/writer and removed bug Something isn't working labels Aug 27, 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 27, 2026
@lance-gatefixer

Copy link
Copy Markdown
Contributor Author

Blocked: PR #8827 remains blocked at current remote head b855eca by three non-attributable stochastic vector-index checks: Rust linux-build job 98532150495 missed the randomized HNSW-PQ recall threshold at 117/256, while Python Linux 3.13 x86_64 job 98534008079 and Python Linux 3.14 ARM job 98532150478 failed residual-sample initialization with StopIteration on opposite index-version parameters. Current main 0ac7d22 is an ancestor of this head; the exact Rust failing test passes locally, both dictionary regression cases pass, and this PR changes only dictionary encoding. Rerun the three failed checks; alternatively, stabilize the unseeded vector tests in a separate change and then rerun this head.

@valkum

valkum commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

These test failures look unrelated.

@github-actions github-actions Bot added the bug Something isn't working 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 base merge leaves the dictionary normalization patch unchanged. The current head still repairs unsafe null-slot keys at the validity-removal boundary, and the hand-built and Arrow-concatenated round trips remain covered without changing the encoded format or safe-array allocation path.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Aug 28, 2026
valkum pushed a commit to instant-labs/lance that referenced this pull request Aug 28, 2026
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
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.

Writing a dictionary column fails when a null slot holds an out-of-range key

1 participant