fix(commit): re-parent children when fix_schema renumbers a nested field

[shoemoney]

Closes #9424

Mechanism

fix_schema in rust/lance/src/io/commit.rs remaps duplicate field ids, then applies the mapping to the schema:

for (old_field_id, new_field_id) in &old_field_id_mapping {
    let field = manifest.schema.mut_field_by_id(*old_field_id).unwrap();
    field.id = *new_field_id;
}

Only id is written. Field carries an independent pub parent_id (rust/lance-core/src/datatypes/field.rs), and the protobuf conversion in rust/lance-file/src/datatypes.rs writes parent_id: field.parent_id verbatim rather than recomputing it from the tree. Schema::mut_field_by_id recurses into children, so nested fields are reachable targets of this loop.

The consequence is durable. Once a struct field is renumbered, its children still name the old parent id, that value is persisted into the manifest, and the read path in rust/lance-file/src/datatypes.rs hard-errors on reopen:

Field 'item' (id=8) references parent id 2, which must appear earlier in the protobuf field list

The tolerant branch immediately above only covers duplicate parent ids that still exist, not a parent id that was renumbered away. The commit succeeds and the table can never be read back. fix_schema is called unconditionally on both live commit paths; the check at the top of the function is a fast-path short-circuit, not a guard.

Fix

Propagate the new id to the field's direct children alongside the id write. Field::set_id already establishes this convention (it sets parent_id and recurses), but it is not directly reusable here because it also assigns ids from a seed for fields with id < 0, which is not what this loop wants.

The update is order-independent. A grandchild names its own parent's id, which that parent's own mapping entry handles, so no entry depends on another having run first. New ids start at max_field_id() + 1, so a new id can never collide with an old id still pending in the mapping.

Why #8658 does not cover this

PR #8658 (feat: stabilize field IDs across schema evolution) touches fix_schema, but only to make it pub(crate), to early-return when stable field ids are active, and to widen the id arithmetic to i64. It does not re-parent children, so it neither fixes nor obsoletes this. It is also currently CONFLICTING with CHANGES_REQUESTED.

On the validation alternative

The issue notes that the trigger requires a writer committing duplicate field coverage across two data files in one fragment, and concedes that such a writer is itself buggy. It offers an alternative of rejecting duplicate coverage at validation instead. That alternative is reasonable and this change does not preclude it. But under either approach, a fixup that runs and then emits an unopenable manifest is indefensible: if duplicate coverage is later rejected outright, this loop becomes unreachable and harmless, and until then it should not corrupt the manifest it is meant to repair.

Tests

New test_fix_schema_nested covers struct s { text, struct inner { ordinal } } with duplicate coverage on both structs, asserting children and grandchildren follow their renumbered parents, plus a closing sweep that every non-root field names a parent id that still exists.

Before, on unmodified source:

---- io::commit::tests::test_fix_schema_nested stdout ----
thread 'io::commit::tests::test_fix_schema_nested' panicked at rust/lance/src/io/commit.rs:2584:9:
assertion `left == right` failed: child of a renumbered struct kept a stale parent_id
  left: 1
 right: 5

test result: FAILED. 131 passed; 1 failed; 0 ignored; 0 measured; 3733 filtered out; finished in 0.34s

After:

test io::commit::tests::test_fix_schema ... ok
test io::commit::tests::test_fix_schema_nested ... ok

test result: ok. 132 passed; 0 failed; 0 ignored; 0 measured; 3733 filtered out; finished in 0.31s

cargo fmt --all --check and cargo clippy -p lance --tests -- -D warnings are both clean on the pinned 1.97.0 toolchain. Note that cargo clippy --all fails on lance-core on current main independently of this change (three chunks_exact findings in utils/bloomfilter/sbbf.rs and utils/stable_partition.rs), but only under a newer clippy than the pin, so it should not affect CI.

[lance-gatekeeper]

✅ Gate recommendation: approve.

This repairs the durable schema corruption from #9424 by re-parenting each remapped field’s direct children to its fresh ID. That matches the serialized schema-tree contract and remains correct when an ancestor and descendant are remapped together. Rejecting duplicate file coverage would prevent the trigger but is a broader behavior change; preserving the existing fixup and making its output readable is the narrower solution.

[Xuanwo]

Thank you for this fix!