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. So
currently, the method just takes in the parent `Snapshot` the commit is being
applied to.
--
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]
