mehtaashish23 commented on a change in pull request #802: Update
RemoveSnapshots to protect cherry-picked data files
URL: https://github.com/apache/incubator-iceberg/pull/802#discussion_r381564069
##########
File path: core/src/main/java/org/apache/iceberg/RemoveSnapshots.java
##########
@@ -188,16 +189,33 @@ private void cleanExpiredFiles(List<Snapshot> snapshots,
Set<Long> validIds, Set
// physically deleting files that were logically deleted in a commit that
was rolled back.
Set<Long> ancestorIds =
Sets.newHashSet(SnapshotUtil.ancestorIds(base.currentSnapshot(),
base::snapshot));
Review comment:
@rdblue +1, I am too in the opinion, that we should deprecate the older API
of expiring a particular snapshotId, supporting it with cherry-pick (or more
complex linking) is going to be tricky.
Apologies I didn't followed on this issue, I was only stuck, since I also
didn't had any solution for supporting expiry of a particular snapshot. I can
help with the UTs though, I attempted to add some with my PR
https://github.com/rominparekh/incubator-iceberg/pull/3/files
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
