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


##########
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:
   Thought about this a bit more, and here is my simplified, more general 
explanation:
   
   The above happens when the referenced manifest for a delete is different 
than the actual manifest which needs to be rewritten. The cherrypick overwrite 
case is one example since the referenced manifest is read from the snapshot 
being cherry-picked, but the actual manifest that needs to be rewritten is the 
manifest from the original append of FILE_A. Worth thinking through if there's 
more such cases and if it's possible to make this more generalizable. 
   
   In the end, the main aspect to ensure is manifests which need to be 
rewritten are rewritten. Then we aim to minimize false positives on top of that.



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