feat(table): add Transaction.AssertRefSnapshotID (Java UpdateRequirements parity) - #1786
Conversation
9f5cd2d to
41ae181
Compare
zeroshade
left a comment
There was a problem hiding this comment.
This lands the Java UpdateRequirements contract properly: ref assertions built from the base state the writer actually read (with the staged-ref exemption reasoned correctly at both RollbackToSnapshot and ExpireSnapshots), same-ref/different-id pairs rejected at apply time instead of one silently dropping, and the dedup-validation reorder that makes base-state re-assertion possible at all. The pin-before-visibility mutex discipline on AssertRefSnapshotID — so a concurrent commit's retry-rewrite can't void the CAS the caller just asked for — is the kind of detail that's easy to miss and wasn't. Documenting the empty-append workaround as a test is a nice touch for anyone migrating.
Heads-up only: #1637 (open) touches the same requirement-building paths, so whichever merges second will want a careful rebase.
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.
…irements from the base state Transaction.apply deduplicated assert-ref-snapshot-id requirements by (type, ref) but silently kept the first when two assertions for the same ref required different snapshot ids. Both can never hold against the catalog: keeping either silently drops a check the caller registered, so reject the pair at apply time with a named error instead of a confusing catalog rejection. Only requirements actually appended are validated against the staged metadata — a deduplicated twin re-asserts base state the staged metadata has intentionally moved past; its kept twin was validated when first added. Snapshot producers built their assertion from the staged metadata's current snapshot. For the second producer commit in one transaction that is an intermediate snapshot the catalog has never seen, which the new conflict check would correctly reject (previously masked by dedup silently keeping the first assertion). Producers now assert the BASE table's branch head — the catalog state the writer actually read, and the only assertion that can hold at commit time. This matches Java's UpdateRequirements, which accumulates one assertion per ref built from the base table state. RollbackToSnapshot and ExpireSnapshots built their assertions from the staged refs the same way and are moved to base-state requirements too; refs staged by the transaction itself (absent on the base) get no assertion from them, since the update that created such a ref already carries its own base-state requirement. Co-authored-by: Cursor <cursoragent@cursor.com>
… commits
Metadata-only transactions (e.g. exactly-once bookkeeping that records
ingestion progress in table properties) cannot detect that the branch
changed between their read and their commit: the branch assertion
Commit builds implicitly is rewritten to the fresh head between
retries and the commit is replayed, so two such committers silently
interleave and the later one clobbers the earlier one's properties.
Expose the existing requirement so a committer can require that the
branch is unchanged from the head it read:
func (t *Transaction) AssertRefSnapshotID(branch string) error
An empty branch defaults to the transaction's target branch; a branch
that does not exist on the base is required to stay absent. The
requirement asserts the BASE table's branch head — the same state
snapshot producers assert, matching Java's UpdateRequirements — so an
explicit requirement and a producer-built one for the same branch
collapse to one regardless of registration order. The branch is
marked pinned before the requirement becomes visible so a concurrent
Commit can never observe the assertion without its pinned status.
doCommit's refresh-and-replay must not rewrite these explicit
assertions to the fresh head between retries — the caller asked for
compare-and-swap semantics — so pinned branches are exempted from
rewriteRefSnapshotRequirements, and once the refreshed metadata shows
a pinned assertion can no longer hold the commit fails fast with
ErrCommitFailed instead of burning the remaining retries.
The empty-append test pins the workaround this API replaces: an empty
AddFiles commits a legal empty snapshot that moves the branch head
(Java newFastAppend().commit() parity), which writers can use to
serialize metadata-only commits through a producer's assertion at the
cost of an empty snapshot per commit.
Co-authored-by: Cursor <cursoragent@cursor.com>
41ae181 to
131a421
Compare
Adds
Transaction.AssertRefSnapshotID(branch): the sameassert-ref-snapshot-idrequirement Java'sUpdateRequirementsbuilds for a changed ref, exposed so metadata-only commits can use it too. A transaction that only sets properties (e.g. exactly-once bookkeeping that records ingestion offsets) can require that the branch is unchanged from the state it read; if another writer committed in between, the commit fails withErrCommitFailedinstead of being silently replayed onto the new head.Two commits:
Align ref requirement accumulation with Java.
UpdateRequirementsaccumulates one assertion per ref, built from the base table state. Our producers built theirs from the staged metadata's current snapshot instead — an id the catalog has never seen when a transaction carries multiple operations. Producers,RollbackToSnapshot, andExpireSnapshotsnow assert the base state, and two same-ref assertions requiring different snapshot ids are rejected at apply time rather than one being silently dropped.Expose the requirement on
Transaction. Matches Java semantics: an explicit requirement and a producer-built one for the same branch collapse to one. The one Go-specific wrinkle is the commit retry loop, which rewrites ref assertions to the fresh head between retries; explicitly registered assertions are exempt, since rewriting would void the compare-and-swap the caller asked for.Without this, Go writers have to commit an empty
AddFilesappend per metadata-only write to serialize through a producer's requirement (an included test documents that workaround, which also matches Java's zero-filenewFastAppend().commit()behavior).Made with Cursor