amogh-jahagirdar commented on code in PR #10983:
URL: https://github.com/apache/iceberg/pull/10983#discussion_r1739152717
##########
core/src/main/java/org/apache/iceberg/RemoveSnapshots.java:
##########
@@ -321,6 +323,15 @@ ExpireSnapshots withIncrementalCleanup(boolean
useIncrementalCleanup) {
private void cleanExpiredSnapshots() {
TableMetadata current = ops.refresh();
+ if (specifiedSnapshotId) {
+ if (incrementalCleanup != null && incrementalCleanup) {
+ throw new UnsupportedOperationException(
+ "Cannot incrementally clean files when expiring snapshots through
specified ids");
Review Comment:
"Cannot clean files incrementally when snapshot IDs are specified"?
##########
core/src/test/java/org/apache/iceberg/TestRemoveSnapshots.java:
##########
@@ -370,7 +370,7 @@ public void testRetainLastWithExpireById() {
}
// Retain last 3 snapshots, but explicitly remove the first snapshot
-
removeSnapshots(table).expireSnapshotId(firstSnapshotId).retainLast(3).commit();
+
table.expireSnapshots().expireSnapshotId(firstSnapshotId).retainLast(3).commit();
Review Comment:
So these are cases where we used to be able to do incremental cleanup before
but now will fail. I'm good with that since the default now is more safe even
though from an API perspective it may be less performant for specific cases.
And to address the performant issue, practically at larger scales there will be
some distributed procedure running and the Spark procedure does the full
reachability analysis itself anyways.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
