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 prevent is cases where the logic says the
manifest doesn't need to be read but it actually does. 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: [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]
