rdblue commented on code in PR #11131:
URL: https://github.com/apache/iceberg/pull/11131#discussion_r1813944812
##########
core/src/main/java/org/apache/iceberg/ManifestFilterManager.java:
##########
@@ -323,11 +345,15 @@ private ManifestFile filterManifest(Schema tableSchema,
ManifestFile manifest) {
PartitionSpec spec = reader.spec();
PartitionAndMetricsEvaluator evaluator =
new PartitionAndMetricsEvaluator(tableSchema, spec,
deleteExpression);
+ boolean hasDeletedFiles =
manifestsReferencedForDeletes.contains(manifest.path());
+ if (hasDeletedFiles) {
Review Comment:
I think this structure makes the code harder to reason about and misses an
optimization.
This currently checks whether the manifest is known to have a file to
remove. If it is unknown, then the logic below is used, which scans the
manifest to determine whether it needs to be rewritten. But if the set of
referenced manifests is trusted, then there is no need to scan these manifests
because the manifest to rewrite for every removal is known.
--
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
