rdblue commented on code in PR #4071:
URL: https://github.com/apache/iceberg/pull/4071#discussion_r841293698
##########
core/src/main/java/org/apache/iceberg/TableMetadata.java:
##########
@@ -1007,6 +1007,49 @@ public Builder setBranchSnapshot(long snapshotId, String
branch) {
return this;
}
+ public Builder setTag(String tag, SnapshotRef ref) {
+ SnapshotRef existingRef = refs.get(tag);
+ if (existingRef != null && existingRef.equals(ref)) {
+ return this;
+ }
+ long snapshotId = ref.snapshotId();
+ Snapshot snapshot = snapshotsById.get(snapshotId);
+ ValidationException.check(snapshot != null, "Cannot set %s to unknown
snapshot: %s", tag, snapshotId);
+ ValidationException.check(ref.isTag(), "Cannot create tag: %s is a
branch", tag);
+ refs.put(tag, ref);
+ changes.add(new MetadataUpdate.SetSnapshotRef(tag, snapshotId,
SnapshotRefType.TAG, null, null, null));
+ return this;
+ }
+
+ public Builder setBranch(String branch, SnapshotRef ref) {
+ SnapshotRef existingRef = refs.get(branch);
+ if (existingRef != null && existingRef.equals(ref)) {
+ return this;
+ }
+ long snapshotId = ref.snapshotId();
+ Snapshot snapshot = snapshotsById.get(snapshotId);
+ ValidationException.check(snapshot != null, "Cannot set %s to unknown
snapshot: %s", branch, snapshotId);
+ ValidationException.check(ref.isBranch(), "Cannot create branch: %s is a
tag", branch);
+ refs.put(branch, ref);
+ MetadataUpdate.SetSnapshotRef refUpdate =
+ new MetadataUpdate.SetSnapshotRef(branch, snapshotId,
SnapshotRefType.BRANCH);
+ boolean newRef = existingRef == null;
+ boolean updateMinSnapshots = newRef ||
!Objects.equal(ref.minSnapshotsToKeep(), existingRef.minSnapshotsToKeep());
Review Comment:
I think this logic is incorrect. These properties should be set if they were
explicitly set for the ref. This expects the ref to already have the required
changes, so it is sufficient to test whether the incoming `ref` has a non-null
`minSnapshotsToKeep`. And since the action for a non-null value is to set the
value, you can just pass all of these through.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
