amogh-jahagirdar commented on code in PR #9354:
URL: https://github.com/apache/iceberg/pull/9354#discussion_r1433548763
##########
core/src/main/java/org/apache/iceberg/MergingSnapshotProducer.java:
##########
@@ -892,15 +892,18 @@ private void cleanUncommittedAppends(Set<ManifestFile>
committed) {
}
}
- ListIterator<ManifestFile> deleteManifestsIterator =
cachedNewDeleteManifests.listIterator();
- while (deleteManifestsIterator.hasNext()) {
- ManifestFile deleteManifest = deleteManifestsIterator.next();
- if (!committed.contains(deleteManifest)) {
- deleteFile(deleteManifest.path());
- deleteManifestsIterator.remove();
+ boolean hasDeleteDeletes = false;
+ for (ManifestFile cachedNewDeleteManifest : cachedNewDeleteManifests) {
+ if (!committed.contains(cachedNewDeleteManifest)) {
+ deleteFile(cachedNewDeleteManifest.path());
+ hasDeleteDeletes = true;
Review Comment:
Also another nit: I'd probably the same pattern we did for the data manifest
case where we just null it out and don't exercise the loop if it's null, but I
see that the logic is the same.
--
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]
