amogh-jahagirdar commented on code in PR #13614:
URL: https://github.com/apache/iceberg/pull/13614#discussion_r2222832174
##########
core/src/main/java/org/apache/iceberg/IncrementalFileCleanup.java:
##########
@@ -50,7 +50,7 @@ class IncrementalFileCleanup extends FileCleanupStrategy {
@Override
@SuppressWarnings({"checkstyle:CyclomaticComplexity", "MethodLength"})
public void cleanFiles(TableMetadata beforeExpiration, TableMetadata
afterExpiration) {
- if (afterExpiration.refs().size() > 1) {
+ if (beforeExpiration.refs().size() > 1 || afterExpiration.refs().size() >
1) {
Review Comment:
I think in the current implementation we're safe against staged snapshots
being cleaned up since the incremental cleanup itself will delete if the
manifests are not referenced in an ancestor.
That said, I generally agree that the clearest way to determine if a
reachability analysis should be performed is by doing an analysis of if any of
the snapshots being removed are outside of the main ancestry.
--
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-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
