[GitHub] [iceberg] jackye1995 commented on a change in pull request #4089: Core: Replace setCurrentSnapshot with setBranchSnapshot in metadata builder

[GitBox <gi...@apache.org>]

jackye1995 commented on a change in pull request #4089:
URL: https://github.com/apache/iceberg/pull/4089#discussion_r805601862



##########
File path: core/src/main/java/org/apache/iceberg/TableMetadata.java
##########
@@ -972,10 +987,17 @@ public Builder removeSnapshots(List<Snapshot> snapshotsToRemove) {
       }
 
       this.snapshots = retainedSnapshots;
-      if (!snapshotsById.containsKey(currentSnapshotId)) {
-        setCurrentSnapshot(null, System.currentTimeMillis());
+
+      // remove any refs that are no longer valid

Review comment:
       Based on the current usage, we are only calling this when expiring snapshots. At that time the snapshots send in here should already be the ones that have to be removed after evaluating all retention policies. I think that's why we directly remove the refs instead of failing.




-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org