RussellSpitzer commented on code in PR #4674:
URL: https://github.com/apache/iceberg/pull/4674#discussion_r862971514
##########
spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/actions/BaseDeleteReachableFilesSparkAction.java:
##########
@@ -125,8 +126,9 @@ private Result doExecute() {
private Dataset<Row> buildReachableFileDF(TableMetadata metadata) {
Table staticTable = newStaticTable(metadata, io);
- return withFileType(buildValidContentFileDF(staticTable), CONTENT_FILE)
- .union(withFileType(buildManifestFileDF(staticTable), MANIFEST))
+ Dataset<Row> allManifests = loadMetadataTable(staticTable, ALL_MANIFESTS);
+ return withFileType(buildValidContentFileDF(staticTable, allManifests),
CONTENT_FILE)
+ .union(withFileType(buildManifestFileDF(allManifests), MANIFEST))
Review Comment:
One other thing to worry about here is what is the additional cost of the
persist operation here. Running a persist is not free and we need to check for
sure that doing this cache is cost effective. In my experience 3 uses of a
persisted df is most of the time worth it, but 2 sometimes is not (Very much
dependent on the computation leading up to the cached df)
--
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]
