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


##########
core/src/main/java/org/apache/iceberg/ManifestFilterManager.java:
##########
@@ -325,7 +341,15 @@ private ManifestFile filterManifest(Schema tableSchema, 
ManifestFile manifest) {
     }
   }
 
+  @SuppressWarnings("checkstyle:CyclomaticComplexity")
   private boolean canContainDeletedFiles(ManifestFile manifest) {
+    boolean manifestReferencedInDelete = 
manifestsWithDeletedFiles.contains(manifest.path());
+    if (manifest.content() == ManifestContent.DELETES && 
allDeletesReferenceManifests) {

Review Comment:
   Updated so that `MergingSnapshotProducer` implementations can surface if 
it's possible to use the referenced manifest. I did it a bit differently than 
"filter manifests prior to passing them to ManifestFIlterManager". The reason 
is there's a few cases that need to be upleveled from `ManifestFIlterManager` 
(e.g. are there live files?) and it actually ends up making the diff quite a 
bit bigger and the logic to stitch together the pre-filtered via referenced 
manifest and additional filtered becomes a bit harder to understand. It's 
possible to be clear, it just requires refactoring quite a lot and in the end 
feels less readable. Instead manifest filter manager exposes a 
`useReferencedMnaifests` method which will get set accordingly in 
`MergingSnapshotProducer`.
   
    I also don't know if we need to pass in `TableMetadata` because for 
detecting if there was a concurrent write, I think we just need to compare the 
parent snapshot ID to the starting snapshot ID and make sure it's 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: 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