Repository navigation
Conversation
gbrgr
force-pushed
the
gb/base-snapshot-assertion-and-stage-commit
branch
from
October 14, 2026 11:45
019ef89 to
a97b6de
Compare
gbrgr
changed the title
Open
3 tasks done
gbrgr
added a commit
to RelationalAI/RustyIceberg.jl
that referenced
this pull request
Oct 15, 2026
Wraps RelationalAI/iceberg-rust#83's OverwriteAction.assert_requirements for Julia: supply arbitrary TableRequirements (as typed TableRequirement subtypes: AssertTableUuid, AssertRefSnapshotId, etc.), passed to the Rust side as a JSON array via a new iceberg_overwrite_action_assert_requirements FFI export. A supplied requirement of an auto-derived kind replaces the derived one; any other kind is added; repeated calls accumulate. This is needed because Transaction::commit's do_commit always refreshes the table from the catalog and silently rebases onto whatever is current before computing its auto-derived RefSnapshotIdMatch requirement, so a retried (or racing) commit would otherwise keep asserting whatever is current rather than the base it actually started from, and a genuine conflict could never surface. assert_requirements lets a caller pin that assertion to an externally-resolved base snapshot instead. Bumps iceberg_rust_ffi's iceberg-rust git rev to a97b6defa31aa69ecb894d8b d5204e7fcd7a51d2 (that PR's branch head) to pick up the new API; move this to a `main` rev once the PR merges. Tests: assert_requirements_tests.jl covers the override/accumulate/reject semantics, including two tests that provoke a genuine CatalogCommitConflicts by pinning a stale/wrong requirement and asserting the catalog itself rejects the commit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Lets a caller supply arbitrary TableRequirements alongside the ones an action derives automatically: a supplied requirement of an auto-derived kind (UuidMatch, RefSnapshotIdMatch for the target ref) replaces the derived one, any other kind is added, and repeated calls accumulate. This makes it possible to assert a pinned base snapshot id that differs from whatever the in-memory Table currently reports. That override is needed because Transaction::commit's do_commit always refreshes the table from the catalog and silently rebases onto whatever is current before computing its auto-derived RefSnapshotIdMatch requirement, on every attempt, not just retries -- so a retried (or racing) commit would otherwise keep asserting whatever is current rather than the base it actually started from, and a real conflict can never surface. A pinned override is exempted from that rebase and from Transaction::apply's local self-check (which is a tautology for auto-derived requirements, since those are read straight off the same table, but would be wrong to apply to a deliberately-diverging override) via ActionCommit's new with_unchecked_requirements/take_unchecked_requirements, so the catalog itself is left to enforce it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
gbrgr
force-pushed
the
gb/base-snapshot-assertion-and-stage-commit
branch
from
October 15, 2026 08:46
a97b6de to
f02c8db
Compare
gbrgr
added a commit
to RelationalAI/RustyIceberg.jl
that referenced
this pull request
Oct 15, 2026
Wraps RelationalAI/iceberg-rust#83's OverwriteAction.assert_requirements for Julia: supply arbitrary TableRequirements (as typed TableRequirement subtypes: AssertTableUuid, AssertRefSnapshotId, etc.), passed to the Rust side as a JSON array via a new iceberg_overwrite_action_assert_requirements FFI export. A supplied requirement of an auto-derived kind replaces the derived one; any other kind is added; repeated calls accumulate. This is needed because Transaction::commit's do_commit always refreshes the table from the catalog and silently rebases onto whatever is current before computing its auto-derived RefSnapshotIdMatch requirement, so a retried (or racing) commit would otherwise keep asserting whatever is current rather than the base it actually started from, and a genuine conflict could never surface. assert_requirements lets a caller pin that assertion to an externally-resolved base snapshot instead. Bumps iceberg_rust_ffi's iceberg-rust git rev to f02c8db2f890d4447a2995 738668573769aa98a3 (that PR's branch head, rebased onto main after #84's upstream sync + pipeline.rs architecture convergence merged) to pick up the new API; move this to a `main` rev once the PR merges. Tests: assert_requirements_tests.jl covers the override/accumulate/reject semantics, including two tests that provoke a genuine CatalogCommitConflicts by pinning a stale/wrong requirement and asserting the catalog itself rejects the commit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
assert_requirements's doc comment referenced commit_pinned and stage_commit, neither of which exist in this branch -- leftovers from an earlier draft that also introduced stage_commit, later split out into its own track. Point at Transaction::commit's rebase-on-every- retry behavior instead, which is the actual reason this method exists. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Lets a caller that must assert a commit against an externally-pinned base snapshot -- possibly one that no longer matches whatever the in-memory
Tablecurrently reports -- do so explicitly.OverwriteAction::assert_requirements(requirements)-- supply arbitraryTableRequirements alongside the ones this action derives automatically (currentlyUuidMatchand aRefSnapshotIdMatchfor the target branch). A supplied requirement of an auto-derived kind replaces the derived one; any other kind is added alongside; repeated calls accumulate.Why
Transaction::commit'sdo_commitalways refreshes the table from the catalog and silently rebases onto whatever is current before computing its auto-derivedRefSnapshotIdMatchrequirement, on every attempt, not just retries. That means a stale base is transparently absorbed rather than surfaced as a conflict -- by design, this is optimistic-retry-via-rebase, and a genuine conflict can never naturally surface through the plain commit path.Transaction::apply's existing local requirement check (requirement.check(Some(table.metadata()))?) validates every requirement against the in-memoryTableit's being applied to. For auto-derived requirements this is a tautology (they're read straight off that same table), but for a caller-supplied override it's actively wrong -- the whole reason to override is to assert something that may not match what's currently loaded, so the catalog -- not this local check -- can correctly reject it as a genuine conflict.ActionCommitnow carries caller-supplied requirements separately viawith_unchecked_requirements/take_unchecked_requirements, so only auto-derived requirements go through the local check; both still end up in the final commit.Test plan
cargo test -p iceberg --lib-- 1794 passed, 0 failedcargo fmt -p iceberg -- --checkcargo clippy -p iceberg --lib --all-targets-- cleanassert_requirementsoverriding an auto-derived requirement, adding a new requirement kind, accumulating across multiple calls.🤖 Generated with Claude Code