smaheshwar-pltr commented on code in PR #16796:
URL: https://github.com/apache/iceberg/pull/16796#discussion_r3407195016
##########
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()
- .map(TableMetadata.MetadataLogEntry::file)
- .collect(Collectors.toSet()),
- "metadata");
+ // base tracks its own metadata file too, so when
previous-versions-max=0 the superseded
+ // file is deleted here rather than left as an orphan
Review Comment:
The diff in this comment is a bit significant can we stick to the older
comment a bit closer maybe?
--
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]
