fix: conflict a write with a metadata change to a field it wrote

[wkalt]

A field's metadata is part of that field's definition, so a write that
produced values for the field and a concurrent change to its metadata are not
independent. Rebasing either past the other publishes a pairing neither writer
produced, and the manifest records no disagreement.

The pair now conflicts retryably, in both commit orders, for an update, a data
replacement and an overlay. Deriving the fields a write produced is the awkward
part: an in-place column rewrite names them in fields_modified, but a row
rewrite deletes the matched rows and writes them again and leaves that list
empty. The columns it recomputed are the ones index maintenance already
consults for the same question, and that signal only means anything once the
update wrote a fragment -- an update with neither only applied deletions.

The two sides also do not address a field the same way. Metadata hangs off a
struct's parent while the values under it are usually written as leaves; a
packed struct or a blob is the other way around, written as one column under
the parent's id while its metadata may name a child. The comparison walks the
field tree in both directions.

The tree comes from the transaction's read version rather than from the handle
it was submitted through, so which dataset a caller commits through cannot
change the decision. commit_transaction already checks that version out, so it
costs no extra read.

[lance-gatekeeper]

❌ Gate recommendation: request changes.

The symmetric conflict barrier is the right direction, but the current written-field model still has two verified false negatives: row-rewrite updates can report no fields_modified, and packed structs can report an ancestor field ID. A viable revision should derive exhaustive affected-field coverage for each supported write shape and compare logical field ancestry in both directions, with regressions for both commit orders.

[lance-gatekeeper]

fields_modified is not exhaustive for RewriteRows, so this check still accepts the metadata/data pairing this PR intends to reject. Normal Dataset::update writes new_fragments while setting fields_modified: vec![]; full-row merge-insert does the same. This predicate therefore sees no overlap in either commit order, allowing values prepared under the old field definition to be published with the new one.

Use an exhaustive logical written-field signal for row rewrites (or add one to the transaction) and distinguish true delete-only/no-value updates instead of treating an empty fields_modified as no write.

Reproducer

Added beside the existing conflict tests and ran cargo test -p lance gate_reproducer_rewrite_rows_with_empty_fields_modified --lib:

#[test]
fn gate_reproducer_rewrite_rows_with_empty_fields_modified() {
    let schema = struct_schema();
    let scalar = field_id(&schema, "id");
    let mut write = update_rewriting(vec![]);
    let Operation::Update {
        new_fragments,
        fields_for_preserving_frag_bitmap,
        update_mode,
        ..
    } = &mut write else { unreachable!() };
    *new_fragments = vec![Fragment::new(2)];
    *fields_for_preserving_frag_bitmap = vec![scalar as u32];
    *update_mode = Some(RewriteRows);

    assert!(conflicts(&write, &metadata_on(scalar), &schema));
    assert!(conflicts(&metadata_on(scalar), &write, &schema));
}

Observed: the first assertion fails because the operations are considered compatible.

[lance-gatekeeper]

Fixed on the current head: row-rewrite updates now derive their written-field set from fields_for_preserving_frag_bitmap when they produce new fragments, and the regression covers both commit orders. Resolving this thread.

[lance-gatekeeper]

This traversal only expands the metadata target downward, which misses the inverse nested-field representation. A packed struct data file records the struct parent ID; if metadata changes one child, field_and_descendants(child) never includes that parent, so both commit orders remain compatible even though the packed value contains the redefined child.

Compare field ancestry symmetrically (identical, ancestor, or descendant) using the schema rather than expanding only the metadata side.

Reproducer

Added beside the existing conflict tests and ran cargo test -p lance gate_reproducer_packed_parent_with_child_metadata --lib:

#[test]
fn gate_reproducer_packed_parent_with_child_metadata() {
    let schema = struct_schema();
    let parent = field_id(&schema, "a_st") as u32;
    let child = leaves_of(&schema, "a_st")[0] as i32;
    let write = replacement(&[parent]);

    assert!(conflicts(&write, &metadata_on(child), &schema));
    assert!(conflicts(&metadata_on(child), &write, &schema));
}

Observed: the first assertion fails because the operations are considered compatible.

[lance-gatekeeper]

Fixed on the current head: field_lineage now matches ancestors and descendants, including a packed parent write against child metadata, and the regression covers both commit orders. Resolving this thread.

[lance-gatekeeper]

❌ Gate recommendation: request changes.

The rebase leaves both verified false negatives unchanged. The conflict boundary still needs an exhaustive logical written-field model and symmetric field-hierarchy comparison so every metadata/data pairing covered by the stated contract is rejected in either commit order.

[lance-gatekeeper]

❌ Gate recommendation: request changes.

2 fixed / 1 new. The prior row-rewrite and packed-parent gaps are covered for a current schema, but the resolver still loses that lineage when the transaction is committed through an older Dataset handle. The rebase should use a schema guaranteed to represent the transaction's read version (or explicit transaction field effects) so conflict decisions do not depend on caller-handle freshness.

[lance-gatekeeper]

dataset here is the caller's original handle, so this can silently miss the parent/child overlap the new rule promises. commit_transaction checks out transaction.read_version for conflict discovery but still calls try_new(&original_dataset, ...); CommitBuilder supports submitting a newer-read-version transaction through an older handle. If v2 adds child C below existing packed parent P, a v2 replacement writes P, concurrent metadata changes C, and the replacement is committed through a v1 handle, field_lineage(v1, C) is only {C}; neither commit order conflicts.

Keep a dataset/schema at the transaction's read version for lineage resolution (or persist explicit semantic field effects) so caller-handle freshness cannot change the decision.

Reproducer

Added beside the resolver tests and ran cargo test -p lance gate_reproducer_stale_schema_loses_nested_lineage --lib:

#[test]
fn gate_reproducer_stale_schema_loses_nested_lineage() {
    let schema = struct_schema();
    let parent = field_id(&schema, "a_st");
    let child = leaves_of(&schema, "a_st")[0] as i32;
    let mut stale_schema = schema.clone();
    stale_schema.field_by_id_mut(parent).unwrap().children.clear();
    let write = replacement(&[parent as u32]);
    let metadata = metadata_on(child);

    assert_eq!(
        (
            conflicts(&write, &metadata, &stale_schema),
            conflicts(&metadata, &write, &stale_schema),
        ),
        (true, true),
    );
}

Observed: left: (false, false), right: (true, true).

[lance-gatekeeper]

Fixed on the current head: commit_transaction now retains the transaction read-version dataset, TransactionRebase independently resolves that version and owns its schema, and the older-handle regression passes in both commit orders. Resolving this thread.

[lance-gatekeeper]

✅ Gate recommendation: approve.

The transaction rebase now resolves lineage and schema from the transaction’s read-version snapshot, so conflict decisions no longer depend on the caller’s stale dataset handle. The regression coverage exercises both commit orders and preserves symmetric conflict behavior across retries.