jasonf20 commented on code in PR #10962:
URL: https://github.com/apache/iceberg/pull/10962#discussion_r1860926978
##########
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:
@rdblue Sure, the filtered manifest list could exclude manifests of a
certain sequence number because their data files were re-written.
For example the current re-write replaces the manifest that contained the
min sequence number, but preserves it. The `filtered` list won't contain that
manifest, but the new manifest created will still have that same sequence
number if the rewrite kept the sequence number small enough.
So the existing logic here that just took the the min from the filtered
could miss the actual minimum since the manifest containing the actual minimum
was re-written as part of this apply. So we need to consider also the minimum
SN that will be present in the manifest that will get created with this
operation.
--
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
