fix: reject missing merge insert update addresses - #8168
fix: reject missing merge insert update addresses#8168lance-gatekeeper[bot] wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Gate recommendation: request changes. The fail-closed address invariant must cover both partial and full-fragment update paths. Validate exact live-address coverage before selecting the full-rewrite optimization, or route ambiguous cases through reconciliation, so every writer path rejects unresolved updates.
| } | ||
| let mut updated_row_addr_iter = get_row_addr_iter(&batches).peekable(); | ||
| let mut updated_rows = | ||
| UpdatedRowAddrReconciler::new(get_row_addr_iter(&batches)); |
There was a problem hiding this comment.
Address validation is constructed only after the full-fragment rewrite branch, so equal row counts still bypass it. On a two-row fragment, updates for (frag, 0) and nonexistent (frag, 2) take that shortcut and return Ok; the second payload is written as physical row 1 instead of rejecting the missing address. Require deletion-free, exact contiguous address coverage before the shortcut, or reconcile this branch too.
Reproducer
On 67d2f61072ba11cd323fa876ebcbc730d5d190ee, I added a disposable test that calls update_fragments with those two addresses and asserts result.is_err(), then ran:
CARGO_TARGET_DIR=/home/agent/tmp/implementation-8168-target cargo test -p lance dataset::write::merge_insert::tests::test_disposable_full_fragment_rewrite_rejects_missing_row_address -- --exact --nocapture
It exited 101 with full-fragment rewrite accepted a missing row address (0 passed; 1 failed).
There was a problem hiding this comment.
Gate recommendation: approve. The revision closes the full-fragment bypass by requiring deletion-free, exact contiguous address coverage before direct rewrite; invalid sets fall back to reconciliation. This preserves the optimization without weakening the fail-closed update contract.
Summary
Root cause
Partial-fragment reconciliation used a debug-only assertion when the next update address had already been passed. Release builds replaced that update with the original row, and addresses after the final live row were never checked. The equal-row-count full-fragment shortcut also bypassed reconciliation without validating addresses, so a stale scalar-index result could silently write a nonexistent row payload into another physical position.
Validation
cargo test -p lance test_updated_row_addr_missingcargo fmt --all -- --checkcargo clippy --all --tests --benches -- -D warningsFixes #7934