jbonofre commented on code in PR #10246:
URL: https://github.com/apache/iceberg/pull/10246#discussion_r1589368924
##########
core/src/main/java/org/apache/iceberg/FastAppend.java:
##########
@@ -156,6 +156,8 @@ public List<ManifestFile> apply(TableMetadata base,
Snapshot snapshot) {
manifests.addAll(snapshot.allManifests(ops.io()));
}
+ manifests.forEach(summaryBuilder::addedManifestStats);
Review Comment:
Generally speaking, I'm in favor of adding metrics in manifest list if it
helps engines during planification.
I don't see how `total-data-manifest-files` and
`total-delete-manifest-files` would be helpful for the planner, but you know
better than me here 😄
I would understand straight if the planner would have actually the location
of the delete manifest files, as he would be able to "bypass" reading them. So
maybe this PR is the first on a series where the next ones would add concrete
location. Just thinking loud again.
To summarize: ok with this change generally speaking even if I don't see how
it would help the planner 😄
--
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
