amogh-jahagirdar commented on code in PR #10962:
URL: https://github.com/apache/iceberg/pull/10962#discussion_r1862410733


##########
core/src/main/java/org/apache/iceberg/MergingSnapshotProducer.java:
##########
@@ -833,7 +833,17 @@ public List<ManifestFile> apply(TableMetadata base, 
Snapshot snapshot) {
         filterManager.filterManifests(
             SnapshotUtil.schemaFor(base, targetBranch()),
             snapshot != null ? snapshot.dataManifests(ops.io()) : null);
-    long minDataSequenceNumber =
+
+    long minNewFileSequenceNumber =

Review Comment:
   Took a look, @jasonf20 see my comment 
[here](https://github.com/apache/iceberg/pull/11131/files#r1815710103) , the 
change to not rewrite deletes in case of a manifest only having aged out 
deletes was actually intentional and I don't think we want to add that behavior 
back. Let me take a look to see how we can simplify the tests in this PR 
considering that manifests with only aged out deletes will only be rewritten 
opportunistically if some other changes prompt a rewrite of the manifest



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

Reply via email to