smaheshwar-pltr commented on code in PR #16796:
URL: https://github.com/apache/iceberg/pull/16796#discussion_r3406385916
##########
core/src/main/java/org/apache/iceberg/CatalogUtil.java:
##########
@@ -590,19 +590,20 @@ public static void deleteRemovedMetadataFiles(
TableProperties.METADATA_DELETE_AFTER_COMMIT_ENABLED_DEFAULT);
if (deleteAfterCommit) {
- Set<TableMetadata.MetadataLogEntry> removedPreviousMetadataFiles =
- Sets.newHashSet(base.previousFiles());
// TableMetadata#addPreviousFile builds up the metadata log and uses
// TableProperties.METADATA_PREVIOUS_VERSIONS_MAX to determine how many
files should stay in
// the log, thus we don't include metadata.previousFiles() for deletion
- everything else can
- // be removed
- removedPreviousMetadataFiles.removeAll(metadata.previousFiles());
- deleteFiles(
- io,
- removedPreviousMetadataFiles.stream()
+ // be removed. The metadata file being superseded
(base.metadataFileLocation()) is normally
+ // re-added to the new log, but when previous-versions-max is 0 it is
not tracked, so it is
+ // included here as well and deleted instead of being orphaned.
+ Set<String> removedPreviousMetadataFiles =
Review Comment:
Simplified in ff177d0d49: the explicit add/forEach/remove sequence is now a
symmetric set difference — `removed = trackedMetadataFiles(base) -
trackedMetadataFiles(metadata)`, where the helper collects a metadata file plus
its log entries. Behaviour is unchanged (no-op for max >= 1, since the
superseded file is still tracked there), it just reads more clearly.
--
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]
