[wjones127]

Stacked on the action vocabulary PR. Implements the three index-management actions the Transaction V2 draft left unwritten — AddIndexSegment, RemoveIndexSegment, and AdjustIndexCoverage — the lowering of the legacy CreateIndex operation onto them, and the conflict rule that lets two writers extend one index at once. No format change: the wire shapes these need land in #7954.

Supersedes #8630 and #8641, which are folded together here and whose heads were on a fork, so they could not be part of a GitHub stack.

The format has no first-class "index" separate from its segments — a logical index is the set of segments sharing a name — so the actions operate on segments. Creating an index and extending one are the same action; dropping an index is one removal per segment. Because a segment's fields, coverage, and base path are all Refs, one commit can now append data and index what it just appended, which previously took two versions.

Two writers can extend one index

Conflict detection previously treated a logical index as a single thing only one writer could touch: any two commits adding a segment to the same index collided. That was a faithful port of the legacy CreateIndex rule, which conflicts on index name alone.

But the query path unions the segments, so two writers indexing different fragments are both right and both should land. The name-level rule is replaced with a claim on the index — the same kind of pairwise footprint entry key assertions use. Two claims on one index conflict when they cover overlapping committed fragments, when either does not state what it covers (the system indices), or when they disagree about what the index is: its fields, its config, or its version. Fragments the operation mints itself are left out of the comparison, since they have no id yet and no concurrent writer can be covering one.

RemoveIndexSegment and AdjustIndexCoverage carry a name. The coverage adjustment needs it to say which index it is widening; both use it when applying, to reject an action whose segment turns out to belong to a different index than the one it was planned against.

Example

Appending a fragment and covering it with a new index segment, in one atomic commit — the index names the fragment by the token it was minted under, since the fragment has no id until the commit lands:

CompositeOperation::new(vec![UserStep::new("append and index", vec![
    Action::AddFragment(AddFragment { local: 0, physical_rows: 5, .. }),
    Action::AddDataFile(AddDataFile { fragment: Ref::Local(0), file, .. }),
    Action::AddIndexSegment(AddIndexSegment {
        name: "by_a".into(),
        covered_fragments: Some(vec![Ref::Committed(0), Ref::Local(0)]),
        ..
    }),
])])

Two writers each doing this against different fragments now both commit, and the index is the union of their segments.

Not included

AdjustIndexCoverage keeps the shape the draft gave it, including the note that coverage representation is still an open design area. It rejects a segment that records no coverage rather than treating "unknown" as an empty set to add to.

The remaining drafted actions (AddOverlays, RefreshRowVersionMetadata, UpdateCompactedSsTables, AssertUniqueKeys) are still unimplemented and still rejected on load.

Behavior differences from the legacy path

The CreateIndex lowering is verified by building the same manifest both ways and asserting the resulting index metadata is identical, but two edges the legacy path silently tolerates are rejected here: removing a segment the manifest does not have, and adding one whose uuid an existing segment already uses. Either means the operation was planned against a different set of segments than it is landing on.

[wjones127]

Self-review pass (polish-pr), 2026-09-20

Decision

AddIndexSegment was the one index action whose decode did not check its name. RemoveIndexSegment and AdjustIndexCoverage both call non_empty; AddIndexSegment took message.name raw. It is the action that creates the name, and Footprint::build_index keys claims on it, so an empty-named segment created a nameless logical index that no removal could then name and that shared one pseudo-index — and inherited its conflicts — with every other empty-named segment. Fixed, with a third #[case::addition] on test_an_index_action_without_a_name_is_rejected, proven to fail without the change.

The covered-index fence hunk that this PR carried in assemble_manifest has moved down to #8644, where the action path first assembles a manifest. The commit message no longer claims the move, and this PR no longer touches manifest_build.rs.

Status

Settled and implemented. fmt clean, clippy --all --tests --benches -- -D warnings clean, scoped pre-commit run --files clean. lance-table transaction tests 246 pass; lance dataset_transactions + io::commit 172 pass.

Open questions

The one worth a decision before this merges:

• IndexIdentity compares Ref::Local tokens across transactions. fields and covering_fields are Vec<Ref> compared with PartialEq, while the coverage side of the same claim deliberately strips local refs (committed_fragments, with a comment saying why). So two writers that each mint a field and index it under one name both present fields: [Local(0)], compare equal, pass the identity gate and are treated as building the same index. Ordering is a second unstated assumption — [Committed(1), Committed(2)] vs [Committed(2), Committed(1)] reads as a disagreement. Either normalize local tokens out the way coverage does (and treat a local field as unverifiable, i.e. conflict), or state why comparing them here is safe. The same shape appears in feat(transaction): implement the remaining drafted actions #8646's KeyAssertion.key_fields, which is also compared raw across footprints; whichever way this goes should go the same way there.

Smaller ones:

• AddIndexSegment::files collapses "no files recorded" and "recorded, empty" via (!self.files.is_empty()).then(...), one line away from FragmentCoverage, which exists purely to keep exactly that distinction for coverage.
• created_at.timestamp_millis() as u64 on encode has no counterpart to the fallible from_timestamp_millis on decode; a pre-1970 timestamp wraps to a nonsense wire value that only Rust round-trips.
• AddIndexSegment.base and ApplyState::resolve_base have no apply-level test — every fixture sets base: None, so the AddBase + Ref::Local composition that motivates the ref is untested.
• Covering fields are tested only on the rejection path; nothing asserts a valid covering_fields list resolves and lands in IndexMetadata. The test_two_writers_extending_one_index matrix also has no disagreeing-covering_fields case, so that field could be dropped from IndexIdentity with the suite still green.
• ResetTable now clears state.indices in place during apply rather than at assembly, so an AddIndexSegment sequenced after it survives where it used to be wiped. No test pins the new behaviour.

🤖 Generated with Claude Code

[wjones127]

Legacy conflict characterization run against this PR (burn-down item 14)

Decision

#9220's characterization suite passes unchanged on this PR: 42 passed, 0 failed, 6 ignored, ~2s.

Status

Verified 2026-09-20. Nothing in the stack changes legacy conflict behaviour.

Options and criteria

The suite's branch (will/legacy-conflict-characterization, base 8cdffd30e) sits 56 commits ahead of this stack's base (93ba1d20c), so the suite does not compile on the stack as-is — FileFragment::write_overlay, which scenarios.rs uses, does not exist at the older base. That is base skew, not a behavioural difference.

Method: for each PR, a throwaway branch off the PR head, merge 8cdffd30e (clean, no conflicts), cherry-pick the suite's five commits, cargo test -p lance --lib conflict_matrix. Throwaway branches deleted afterwards; nothing here is pushed.

Results, all four layers:

PR	Result
#7954	42 passed, 0 failed, 6 ignored
#8644	42 passed, 0 failed, 6 ignored
#8645	42 passed, 0 failed, 6 ignored
#8646	42 passed, 0 failed, 6 ignored

#8644 is the layer that matters most here — it is the one that touches conflict_resolver.rs, adding the fail-closed CompositeOperation arm to every check_*_txn and spelling out check_add_bases_txn's wildcard. The suite covers exactly that file and is unaffected.

Open questions

• The suite's 6 ignored cases are ignored on its own branch too; this run does not change them.
• The stack will need a rebase onto current upstream/main before merge regardless; the clean merge above is evidence that rebase is not expected to be eventful.

🤖 Generated with Claude Code

[wjones127]

Restacked onto the new #8644 head and added 77c2d8885: an index segment now requires the definitions of its keyed and covering fields, this side's requirements are also checked against the committed set's removed fragments and replaced maps, and AdjustIndexCoverage requires the fragments it widens onto. Decision record and the residual gaps are in #8644 (comment).

[lance-gatekeeper]

❌ Gate recommendation: request changes.

The restack leaves the nested-field build-freshness finding unchanged. Keep the composable actions and make those dependencies ancestry-aware, or conservatively conflict data-changing rewrites of covered fragments; final segment-set consistency cannot establish freshness.

The declared-field checks narrow the author-accepted coverage-widening risk, but widening over committed fragments can still race nested-field rewrites and yield incorrect indexed answers. Widening onto fragments minted in the same operation avoids that race.

Please mark this PR with the breaking-change label.