feat: add CherryPickOperation and wire SnapshotManager::Cherrypick - #926
shangxinli wants to merge 2 commits into
Conversation
05e5a08 to
2ef558f
Compare
|
The Meson build has been removed. I'll review this PR once the conflicts are resolved. Thanks! |
Implements cherry-picking an append or dynamic partition overwrite onto the current state, with source-snapshot-id and WAP publish tracking, non-ancestor and replaced-partition validation. SnapshotManager routes fast-forwardable picks to SetSnapshot, since those produce no snapshot. Closes the CherryPickOperation item in apache#637.
2ef558f to
a79f110
Compare
|
@zhjwpku thanks. it is resolved now. |
|
|
||
| Status CherryPickOperation::ValidateNonAncestor(const TableMetadata& metadata, | ||
| int64_t snapshot_id) const { | ||
| ICEBERG_ASSIGN_OR_RAISE(bool is_ancestor, |
There was a problem hiding this comment.
If we create a new branch on an empty table, append some data to it, and then cherry-pick that append to main, this check fails with No current snapshot[1]. ISTM that main has no history yet, should we allow this case?
[1] https://github.com/apache/iceberg-cpp/blob/main/src/iceberg/table_metadata.cc#L284-L286
There was a problem hiding this comment.
Good catch — fixed in ecf7262. The bug was one level down: SnapshotUtil::IsAncestorOf(metadata, id) raised on no current snapshot instead of returning false like Java's ancestorsOf(null, ...) does, so I fixed it there rather than special-casing this call site.
…shot metadata.Snapshot() raised "No current snapshot" when the ancestor check had nothing to compare against, which surfaced as a cherry-pick failure when picking onto a branch with no history yet. Java's equivalent (SnapshotUtil.ancestorsOf(null, ...)) treats this as an empty ancestor list instead of an error, so nothing can be an ancestor of it; mirror that here.
This branch has not been deployed
2 participants
Adds
CherryPickOperationand implementsSnapshotManager::Cherrypick(), which was a TODO stub.Follows Java's
CherryPickOperation: appends re-apply their added files, dynamic overwrites (replace-partitions) re-apply adds and deletes and are rejected if a replaced partition changed, and anything else can only be fast-forward. Recordssource-snapshot-idandpublished-wap-id, and rejects a WAP id that was already published.Like Java, the operation is reachable only through
Transaction, notTable.One divergence worth a look:
Apply()here always builds a new snapshot, so it cannot express Java's fast-forward, which returns an existing one.SnapshotManagerinstead routes a fast-forward toSetSnapshot, with the predicate inSnapshotUtil::CanFastForward()so there is a single definition.SetSnapshot::RequireFastForward()re-checks it at apply time so a commit retry cannot silently turn the fast-forward into a branch move.14 tests ported from Java
TestSnapshotManagerandTestWapWorkflow.Part of #637.