fix(table): fail commits with delete-file removals instead of replaying

[ryanworl]

What

Commits whose staged snapshot carries delete-file removals — the deleteFilesToRemove argument of Transaction.ReplaceFiles, and the deletion-vector supersession a v3 merge-on-read delete performs when the data file already carries a DV — no longer enter doCommit's refresh-and-replay retry loop. On a CAS conflict they fail immediately with ErrCommitFailed (annotated so it's distinguishable from an exhausted retry budget), and the caller re-resolves the removals against the current snapshot in a new transaction. The mechanism is one flag set by the snapshot producer, one early return in the retry loop, docs, and regression tests.

Why

Removal identity is snapshot-relative: the removed entries were resolved against the snapshot the writer built on, so a replayed removal is semantically a different operation. Concretely: writer A supersedes DV₁ with its merged DV₂ (what any v3 MoR delete does on a file that already has a DV). Writer B supersedes DV₁ with DV₁′ and wins the CAS. A's replay inherits DV₁′ from the fresh base while A's stale removal of DV₁ replays as a no-op (checkRemovedFiles deliberately treats a superseded DV as absent — expunging by ref alone would discard the peer's newer deletes). The committed snapshot has two live DVs on one data file, which the v3 spec forbids. The same shape hits path-keyed ReplaceFiles removals today: a compaction replay can remove the data file a peer's freshly added delete file references, silently no-oping that delete and resurrecting its rows in the compacted replacement.

Java does not have this bug because SnapshotProducer.commit() re-runs the whole apply-validate-commit cycle after refresh(), re-resolving removals against the fresh base. Go's replay patches the already-built updates instead. The correct long-term fix is Java-style re-apply on retry; until that lands, removal-carrying commits must fail fast rather than replay incorrectly (once re-apply exists, this flag simply becomes unnecessary and can be removed).

Behavior changes

Only for removal-carrying commits with commit.retry.num-retries > 0, which today retry incorrectly. Such commits now fail on the first conflict even when the peer's change is unrelated and a replay would have been harmless — the library cannot tell the cases apart, and failing loudly beats corrupting silently. Callers of the removal APIs (compaction/maintenance) already handle commit failure by rebuilding against a fresh table. Data-file-only removals are unaffected and keep replaying: they are path-keyed, and the retry rebuild already fails terminally when the path is gone from the fresh base.

Tests

• TestMoRDeleteSupersedingDVFailsInsteadOfReplaying: the two-live-DV shape through Transaction.Delete on a v3 MoR table — exactly one CommitTable attempt, ErrCommitFailed, exactly one live DV (the peer's) afterward, and the transaction stays retriable.
• TestReplaceFilesWithRemovalsFailsInsteadOfReplaying: the delete-loss shape through ReplaceFiles, where the peer's new position delete is invisible to the serializable added-data-files validator.
• TestReplaceFilesDataOnlyStillReplays: regression guard that a data-file-only replace still replays to success.

Both removal tests fail on the pre-fix code — the corrupting replay commits successfully. go test ./table/... and golangci-lint run are clean.

Made with Cursor

[zeroshade]

The two failure narratives — stranded double DVs from replayed supersession, and resurrected rows from a compaction replay removing a peer-referenced data file — are exactly why removal identity can't survive refresh-and-replay, and the fix draws the line at precisely the right place: delete-file removals and DV supersessions fail fast with an annotated ErrCommitFailed, while path-keyed data-file removals stay replayable under their existing terminal check. One flag, one early return, thorough docs at both the property and the flag site. This is how to handle a correctness hole in a retry loop.

---

This review was drafted with an AI-assisted tool and may contain mistakes; an Apache Iceberg Go maintainer has reviewed and confirmed the submission. See the contributing docs for what the project considers a maintainer review.