smaheshwar-pltr commented on code in PR #16796:
URL: https://github.com/apache/iceberg/pull/16796#discussion_r3406368128


##########
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:
   These changes are a bit involved. Any way to simplify the code here?



##########
core/src/main/java/org/apache/iceberg/TableMetadata.java:
##########
@@ -1800,12 +1800,18 @@ private static List<MetadataLogEntry> addPreviousFile(
 
       int maxSize =
           Math.max(
-              1,
+              0,
               PropertyUtil.propertyAsInt(
                   properties,
                   TableProperties.METADATA_PREVIOUS_VERSIONS_MAX,
                   TableProperties.METADATA_PREVIOUS_VERSIONS_MAX_DEFAULT));
 
+      // A max of 0 means no previous metadata files are tracked, so the 
metadata log is empty and
+      // the file being superseded is not retained.

Review Comment:
   Remove comment



-- 
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]

Reply via email to