rdblue commented on code in PR #6090:
URL: https://github.com/apache/iceberg/pull/6090#discussion_r1053474166
##########
core/src/main/java/org/apache/iceberg/FileCleanupStrategy.java:
##########
@@ -83,4 +85,25 @@ protected void deleteFiles(Set<String> pathsToDelete, String
fileType) {
(file, thrown) -> LOG.warn("Delete failed for {} file: {}",
fileType, file, thrown))
.run(deleteFunc::accept);
}
+
+ protected Set<String> expiredStatisticsFilesLocations(
+ TableMetadata beforeExpiration, TableMetadata afterExpiration) {
+ Set<String> statisticsFilesLocationBeforeExpiry =
allStatisticsFilesLocation(beforeExpiration);
+ Set<String> statisticsFilesLocationAfterExpiry =
allStatisticsFilesLocation(afterExpiration);
+
statisticsFilesLocationBeforeExpiry.removeAll(statisticsFilesLocationAfterExpiry);
+ return statisticsFilesLocationBeforeExpiry;
Review Comment:
I think it is a best practice to avoid mutating sets or returning mutable
sets. Can you use Guava's `Sets.difference` instead of `removeAll`? It would
also be better if the set returned by `allStatisticsFilesLocations` were
immutable.
--
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
