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 only delete if it's
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
