Skip to content

feat(transaction): implement the index management actions - #8630

Closed
wjones127 wants to merge 31 commits into
lance-format:will/transaction-v2-actionsfrom
wjones127:will/transaction-v2-index-actions
Closed

feat(transaction): implement the index management actions#8630
wjones127 wants to merge 31 commits into
lance-format:will/transaction-v2-actionsfrom
wjones127:will/transaction-v2-index-actions

Conversation

@wjones127

Copy link
Copy Markdown
Contributor

Stacked on #8554. Implements the three index-management actions the Transaction V2 draft left unwritten: AddIndexSegment, RemoveIndexSegment, and AdjustIndexCoverage, plus the lowering of the legacy CreateIndex operation onto them.

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.

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![UserAction::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)]),
        ..
    }),
])])

Breaking changes

Three changes to AddIndexSegment in the (unstable, pre-vote) draft wire format:

  • covered_fragments becomes an optional wrapper message rather than a bare repeated. A bare repeated field cannot distinguish "no coverage recorded" — what the MemWAL and fragment-reuse indices carry, and what the query path treats as "serve this segment" — from "covers no fragment", which it treats as "skip it". Collapsing the two would silently change which segments answer a query.
  • Added base, without which a segment imported from another dataset cannot be expressed at all.
  • Added created_at and dataset_version. Both describe the build rather than where it lands, so replaying the action onto a different version must not rewrite them. dataset_version in particular is a correctness gate rather than provenance: an overlay committed at or before it counts as already folded into the index, and a segment merged from several older ones reflects only as much as its oldest input. That puts it genuinely below the version the operation reads, so it cannot be derived. It defaults to the read version and may not exceed it.

Per the plan for this stack, these get folded into the first PR at the end so the format change is a single vote.

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.

@github-actions

Copy link
Copy Markdown
Contributor

Important

This PR touches the Lance format specification.

Substantive changes to the format specification — the .proto definitions
and the spec docs under docs/src/format/ — require a PMC vote before merge.
Minor edits such as typo fixes, wording, or formatting are excluded; use your
judgment.

If this is a meaningful format change:

  • Start a vote following the Lance community voting process.
    Format specification modifications need 3 binding +1 votes (excluding the
    proposer), held on GitHub Discussions, with a minimum voting period of 1 week.
  • Once the vote passes, link the completed vote in this PR. It should not be
    merged until the vote is linked.

@github-actions github-actions Bot added A-format On-disk format: protos and format spec docs enhancement New feature or request labels Aug 19, 2026
Comment on lines +137 to +139
pub(super) fn footprint(&self, footprint: &mut Footprint) {
footprint.add(Coordinate::IndexName(self.name.clone()));
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

issue(blocking): I don't see why we wouldn't allow multiple segments to be committed to the same index, as long as they have matching index config and cover disjoint fragments. However, I don't know if we can express that with footprint. Something we need to discuss.

Comment thread rust/lance-table/src/transaction/action/add_index_segment.rs Outdated
Comment thread rust/lance-table/src/transaction/action/adjust_index_coverage.rs
wjones127 and others added 25 commits August 19, 2026 12:42
Introduces the Rust side of the action-based transaction draft: `Ref`,
`UserOperation`, `UserAction`, and the eight `Action` variants this build
implements (AddFragment, AddDataFile, AddField, AddBase, TombstoneFieldData,
RemoveFragment, SetDeletionFile, AlterField).

Types only -- no wire conversion, apply, or conflict handling yet, so nothing
reaches these from the commit path.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds `Operation::UserOperation`, the Transaction V2 arm of the transaction
oneof, and its protobuf conversions in both directions. Loading a V2
transaction now yields an action set instead of an outright rejection; an
action the build does not implement is still rejected rather than skipped,
so a concurrent V2 commit can never be silently treated as a no-op.

The rest of the transaction machinery gains the variant but no behavior:
build_manifest returns NotSupported, and conflict checks route through a
single `check_action_txn` that conservatively demands a retry. Apply and
conflict rules follow in later commits.

Also extracts `From<&DeletionFile> for pb::DeletionFile`, previously inlined
in the fragment conversion and now needed by SetDeletionFile too.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Pulls the two operation-independent stages out of `build_manifest`:
`normalize_fragments` (order, drop fully-tombstoned files, check overlay
order) and `assemble_manifest` (construct the manifest and apply the tag,
feature flags, timestamp, and fragment id watermark).

The overwrite-only storage format override becomes an explicit parameter
rather than a match on the operation inside the assembly step. No behavior
change; the action-based apply path needs the same two stages.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds the action apply path: `build_manifest_from_actions` walks the action
set in order against a working copy of the read-version state, and the four
minting actions (AddFragment, AddDataFile, AddField, AddBase) allocate ids
from the target's counters as they are reached.

Each mint records its id against the action's local token, so a later action
in the same operation resolves that token to the id this apply chose. The
same action set replayed against a different version therefore lands on
different ids without any action changing -- the property branch merge needs.

An action set requires an existing dataset: it is a delta, so there is
nothing for it to be a delta against at creation time.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Applies TombstoneFieldData, RemoveFragment, SetDeletionFile, and
AlterField, completing the eight-action apply path.

Tombstoning a field's data and retyping a field both leave any index
over that field describing values the fragment no longer holds, so the
affected fragments are dropped from the index bitmap rather than the
index being discarded outright.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ions

Adds `TryFrom<&Operation> for Vec<UserAction>`, the recipe that turns a
named operation into the granular actions it decomposes into. Squashing
several operations into one commit is concatenation once both sides speak
actions.

Translation is fail-closed: an operation with no recipe yet, or one
carrying a detail the actions cannot express, is rejected.

Each case is covered by a parity test that builds the manifest twice --
once down the legacy path, once through the translation -- and asserts
the two agree.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replacing a field's data becomes TombstoneFieldData followed by
AddDataFile. The legacy path swaps the path on the existing file in
place, so the two produce the same set of data files per fragment but
not necessarily the same order; files are addressed by field, so the
order carries no meaning.

Also documents why Merge and Project are not translated: both hand over
a whole new schema rather than a delta, and Project needs a
field-removal action this draft does not define.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A footprint is the set of coordinates an action set writes. Two
concurrent sets can both commit when neither writes what the other
writes -- one structural rule instead of a matrix over operation pairs,
so adding an action means saying which coordinates it writes rather than
extending an N-by-N table.

Only committed coordinates appear. A minted fragment, field, or base has
no id in the read version, so two writers minting at the same time never
collide.

Footprints are derived from the actions at conflict time and never
serialized, so a writer cannot pin down what a reader treats as a
conflict and the rule can be tightened without a format change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replaces the conservative always-retry with a footprint comparison. A
legacy operation on the other side gets a footprint through its action
translation, so an action set can be checked against a concurrent named
operation without either needing an entry in the operation-pair matrix.
An operation with no translation still falls back to retry.

Rebasing an action set onto a newer version is a no-op: its minted ids
are allocated when the actions are applied, against whichever manifest
they land on, and its committed references name coordinates that do not
move.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replaying the same actions against the manifest a previous run produced
re-resolves their local tokens against the newer counters, so the second
run mints different fragment and field ids without any action changing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Each test is one commit doing work that would otherwise have taken
several: a fragment added and then modified, a field added and then
filled, both inside a single version. Also covers row id assignment for
minted fragments and both sides of the footprint conflict rule through
the real commit path.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Removes a field from the schema, taking its descendants with it. The
slots that backed the dropped fields in each data file are tombstoned
rather than removed, so a file's remaining columns stay at the positions
they were written at; a file left backing nothing live is pruned during
normalization.

For conflicts, a drop writes every coordinate belonging to the field --
its definition and its data in every fragment -- so it collides with a
concurrent alter or data rewrite of the same field.

Field ids come from a monotonic counter, so a dropped id is never reused
and a stale data file naming it cannot be mistaken for a later field.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The action code was split by phase -- apply, footprint, proto -- so
understanding one action meant reading four files and adding one meant
touching four match statements.

Each action now owns a module holding its definition, `apply`,
`footprint`, wire conversions, and tests. `Action` dispatches to them.
The phase modules keep what is genuinely shared: `apply` holds the
`ApplyState` the actions program against, `footprint` the coordinate
space and the conflict comparison, `proto` the envelope and dispatch.

No behavior change. The one difference is that `AddField` and `AddBase`
now check for a duplicate local token before bumping the id counter,
matching `AddFragment`; a duplicate token aborts the whole apply either
way.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Takes a contiguous range of fragment ids off the counter without minting
fragments for them, so a later (possibly distributed) writer can populate
the range and name the ids as committed. The counterpart of the legacy
ReserveFragments operation.

Nothing backs a reserved id, so the manifest assembly cannot infer it
from the fragment list; apply raises the manifest's high-water mark to
cover the range instead. The range starts wherever the counter stands,
which unlike the legacy operation does not waste an id on an empty table.

The action writes no coordinates: ids come off a monotonic counter, so
two operations reserving at once get disjoint ranges.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Empties the table -- schema, schema metadata, fragments and indices --
leaving the config, table metadata and base paths alone. This is how a
full Overwrite / CREATE OR REPLACE decomposes: reset, then write the
fresh schema and data as later actions in the same operation.

The id counters keep going across the reset, so a field or fragment
added afterwards never reuses an id a stale file might still name.

Conflict detection gains its first non-enumerable footprint. A reset
writes every coordinate there is, including ones a concurrent set would
only mint, so it takes the table exclusively: any concurrent action set
is preempted, including a pure append that writes no committed
coordinate at all.

Two proto tests used ResetTable as their example of a drafted-but-
unimplemented action; they now use RefreshRowVersionMetadata.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Edits the four string maps a manifest carries -- dataset config, table
metadata, schema metadata and per-field metadata -- with the same
UpdateMap the legacy UpdateConfig operation takes. Per-field updates are
keyed by Ref, so a field minted earlier in the same operation can be
given metadata before it has a committed id.

The unenforced primary and clustering keys keep the immutability rules
the legacy path enforces: each is rejected if changed once set, or if a
reserved key is written with a value that installs no valid key. The
check runs on every apply including a conflict rebase, so it catches the
concurrent-writer race too.

Conflicts are per key: two operations editing different keys of the same
map commute. A replacement writes keys it does not name, so like a
fragment removal it is matched by map rather than by key, and two
replacements of one map collide even when both are clears. A field's
metadata belongs to the field, so dropping the field collides with a
concurrent update to its metadata.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`UserOperation` read as a category next to `Operation::Append` rather than
naming what distinguishes it. Rename it to `CompositeOperation`, which says
the thing: a composite of granular actions committed atomically.

`UserAction` keeps its name -- it is the user-facing grouping of actions,
the level a user recognizes, as distinct from the granular actions inside it.

Also drops `CompositeOperation::description`. Nothing reads it, and every
construction site set it to something the step descriptions already imply.
The per-step `UserAction::description` is what keeps history readable; an
operation-level name can come back if squashing turns out to need one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s differ

`AddFragment`'s version metadata fields said only "stamp at apply" without
saying who would ever set them. Name the two producers that must: an update
carries each row's `created_at` forward from its source fragment, and a
compaction rechunks both sequences off the fragments it merged.

`TombstoneFieldData::data_change` deferred to `AddFragment`'s doc, which
does not cover it. State the case directly: a tombstone paired with a re-add
in the same operation moves bytes without changing values.

Strikes an over-long comment in `conflicts.rs`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adding an action meant editing five per-variant matches -- the enum, `name`,
`is_data_change`, `apply`, `footprint` -- plus both directions of the wire
encoding, in two files. Nothing but review caught a variant handled
inconsistently across them.

`for_each_action!` now holds the vocabulary as a single list of variant names,
and the enum, its four forwarding methods, and both proto conversions expand
from it. Adding an action is a module plus one line. The variant name is also
the protobuf oneof variant and the name in errors, so those cannot drift.

`is_data_change` moves into the action modules alongside `apply` and
`footprint`, so an action's module answers every question about it. The
grouped comments that used to sit in the central match move with it, one to
each action.

This is the `enum_dispatch` pattern done locally: it needs no new dependency,
and it also covers the decode direction, which dispatches on the protobuf
oneof tag rather than on `self` and so is out of `enum_dispatch`'s reach.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A dedicated integration test gets its own binary, which links every
dependency of the crate. Fold the six tests into the existing
`dataset::tests::dataset_transactions` module as a `composite` submodule and
delete the standalone target.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The field claimed a minted fragment "has no committed rows to delete", which
reads as a semantic restriction and is at best ambiguous. The actual
constraint is mechanical: a deletion file's path embeds the fragment id, so
the writer must know the committed id before it can write the file, and a
minted id does not exist until apply.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ifest

`build_manifest_from_actions` ran the actions and then assembled the manifest
in one 80-line body. Move the assembly to `ApplyState::into_manifest`, so the
entry point reads as the three steps it is: make the state, run the actions,
turn it into a manifest.

`ApplyState` now borrows the manifest it was built from rather than copying
everything it needs out of it, which is also what lets `into_manifest` reach
the read version without being handed it back.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`AlterField`, `DropField`, and `TombstoneFieldData` took raw committed field
ids, so none of them could name a field minted earlier in the same operation.
Squashing needs exactly that: collapsing "add column c" and "rename c" into
one operation leaves an `AddField` whose field has no committed id, and a
squash rewrites already-committed transactions, so it cannot re-plan the data
to avoid the reference.

All three now take a `Ref`, matching the fragment actions. Footprints follow
the rule already used there: a local reference records no coordinate, since a
field this operation mints is invisible to a concurrent writer.

`AlterField` loses its `Default` impl -- `Ref` has no meaningful zero, and a
default field reference would silently mean field 0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rebasing onto the merged transaction module split brings three API changes the
action code predates: `RowIdMeta::Inline` wraps `InlineRowIds` rather than a
byte vector, `DataFile::new`/`new_unstarted` take a `ConcreteFileVersion`
instead of a major/minor pair, and `Operation::Project` carries
`preserves_nullability`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The index actions edit the index list as they are reached, so the list has
to be part of the state actions are applied against rather than an argument
the manifest assembly receives separately. ResetTable clears it where it
stands instead of setting a flag the assembly reads back.
The format has no first-class index apart from its segments, so one action
covers both creating an index and extending one: a logical index is the set
of segments sharing a name. Its fields, coverage, and base path are Refs, so
a segment can index what the same operation just wrote.

Three format changes fall out of implementing it:

- `covered_fragments` becomes an optional wrapper message. A bare `repeated`
  cannot tell "no coverage recorded" -- what the system indices carry, and
  what the query path treats as "serve this segment" -- from "covers no
  fragment", which it treats as "skip it".
- Added `base`, without which a segment imported from another dataset cannot
  be expressed.
- Added `created_at` and `dataset_version`, both describing the build rather
  than where it lands. `dataset_version` in particular is a correctness gate
  (an overlay committed at or before it counts as folded into the index) and
  a merged segment reflects only as much as its oldest input, so it is
  genuinely below the read version and cannot be derived. It defaults to the
  read version and may not exceed it.
Dropping a logical index is one of these per segment carrying its name,
since the format knows only segments. Removing a segment the dataset does
not have is rejected rather than treated as a no-op: it means the operation
was planned against a different set of segments.

Segments are named by uuid, which the writer picks, so the footprint
coordinate is the segment itself -- a concurrent writer extending the same
logical index adds a segment of its own and does not collide.
Moves fragments in and out of a segment's coverage without rewriting the
segment, which is what lets an append and the coverage extension over what
it appended commit as one operation.

A segment recording no coverage is rejected rather than treated as an empty
set to add to: "unknown coverage" is what the query path serves everything
for, so turning it into a concrete set would silently narrow the segment.
The legacy operation already carries its removals and additions as two
lists, so the recipe is one RemoveIndexSegment per removal followed by one
AddIndexSegment per addition. Parity tests build the same manifest down both
paths and assert the resulting index metadata is identical.

Two edges the legacy path 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.
Covers the three index actions through the real commit path: one commit
that appends a fragment and adds a segment covering it by local token, one
that swaps a segment out, and one that moves coverage around.
…itions

`AddIndexSegment`'s DeepSizeOf skipped `index_details` on the grounds that
it is opaque. It is only a type url and a byte string, so both are now
measured.

`AdjustIndexCoverage` did not say when adding a fragment to a segment's
coverage is legitimate. It is one case -- a rewrite moved rows the segment
already covered into a new fragment, which the segment reaches through the
fragment-reuse remapping. Adding a fragment of new rows is a writer error
that nothing here can detect, so it is called out.
@wjones127
wjones127 force-pushed the will/transaction-v2-actions branch from f1cfb1f to 25f0610 Compare August 19, 2026 20:30
@wjones127
wjones127 force-pushed the will/transaction-v2-index-actions branch from 2d455ad to d3400f3 Compare August 19, 2026 20:30
@wjones127
wjones127 force-pushed the will/transaction-v2-actions branch from 25f0610 to 247689d Compare August 19, 2026 22:49
@wjones127

Copy link
Copy Markdown
Contributor Author

Superseded by #8645, which folds this together with #8641 (the index claim conflict rule) and moves the branch head into this repo for GitHub's native stacks. The protobuf changes described here now live in #7954.

@wjones127 wjones127 closed this Aug 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-format On-disk format: protos and format spec docs enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant