Skip to content

feat: add ManageSnapshots.fast_forward_branch#3649

Open
JeroenSchmidt wants to merge 1 commit into
apache:mainfrom
bookingcom:jschmidt/add-fast-forward
Open

feat: add ManageSnapshots.fast_forward_branch#3649
JeroenSchmidt wants to merge 1 commit into
apache:mainfrom
bookingcom:jschmidt/add-fast-forward

Conversation

@JeroenSchmidt

Copy link
Copy Markdown

Rationale for this change

Adds ManageSnapshots.fast_forward_branch(from_branch, to_ref) to advance a
branch to the snapshot referenced by another branch or tag, provided the
current snapshot of from_branch is an ancestor of to_ref's snapshot.

The method almost matches the behavior of Apache Iceberg's Java reference
implementation (SnapshotManager.fastForwardBranch /
UpdateSnapshotReferencesOperation.replaceBranch(..., fastForward=true))
and, by extension, the Spark system.fast_forward stored procedure that
depends on it.

However, while the implementation is very close there is one exception;

Unlike Java's `ManageSnapshots.fastForwardBranch`, this method does not
provide Java-parity for intra-chain semantics.

1) Java maintains a mutable `updatedRefs` map that reflects prior operations
      in the same chain, so calling `createBranch` and then `fastForwardBranch`
      on the same ref in one chain observes the freshly-created state.

2) pyiceberg's `ManageSnapshots` accumulates `SetSnapshotRefUpdate` values without
      mutating `table_metadata.refs` between chained calls;
      -> The no-op and ancestry checks below operate on committed metadata only.
      -> Callers that need Java-parity behavior should commit between chain steps.

For the sake of scope I left out the changes needed to ensure chain sematic parity between the java and python.
Would the maintainers like a followup PR or have it included in this PR? My only worry is that it will increase the complexity of an otherwise straightforward PR.

Are these changes tested?

Yes. Both unit tests and integration tests were writen

Are there any user-facing changes?

Yes. fast_forward_branch.
See mkdocs/docs/api.md for more details.

Related Issues / Pull Requests

LLM Disclosure

PR was written with the assistance of Claude Opus 4.7 (1M context) (effort: high)

It implemented the majority of the tests I defined. Specifically to assist with what fixtures to use and how best to write the unit tests in an atomic fashion appropriate for pyIceberg.

test: fast_forward_branch auto-creates missing from_branch
test: fast_forward_branch is a no-op when snapshots already equal
test: fast_forward_branch rejects a tag as the source ref
test: fast_forward_branch rejects missing to_ref
test: fast_forward_branch rejects non-ancestor target
test: fast_forward_branch preserves retention fields
test: fast_forward_branch composes in a manage_snapshots chain
test(integration): fast_forward_branch on real catalogs

docs: add fast_forward_branch section with WAP example
chore: apply ruff-format and add None-checks for mypy
@rambleraptor

Copy link
Copy Markdown
Collaborator

I'm personally in favor of making those changes. I understand how a follow-up PR would be cleaner, but I worry about a situation where we merge this in + don't get around to the follow-up.

I'm happy to do a review as-is. Which works better for you?

@JeroenSchmidt

Copy link
Copy Markdown
Author

I'm personally in favor of making those changes. I understand how a follow-up PR would be cleaner, but I worry about a situation where we merge this in + don't get around to the follow-up.

I'm happy to do a review as-is. Which works better for you?

Hi @rambleraptor
Thank you for the response. Would it be possible for you to review it as is while I look into the open issue with chaining? Depending on the size of the change I can then add it as a commit or raise another PR while this one gets reviewed.

I'm still getting familiar with the inner workings of this library so some guidance would help.

Do you know if it's safe for me to construct the refs to point to in fast_forward by inspecting self._transaction.table_metadata.refs before any commits occur? In other words, are there any fail states or edge cases that I would need to consider if I start referencing refs mentioned in self._transaction.table_metadata.refs that are not committed yet?

@rambleraptor

Copy link
Copy Markdown
Collaborator

There's almost certainly a lot of edge cases, but we'll probably have to discover them as we go. I'll do a review and we can go from there.

@JeroenSchmidt

JeroenSchmidt commented Jul 13, 2026

Copy link
Copy Markdown
Author

Sounds good. I'll try come up with something and then see where it leads us. 😄
I think I'll have something to share EoW / early next week depending on my other commitments and your review comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants