szehon-ho commented on code in PR #9335: URL: https://github.com/apache/iceberg/pull/9335#discussion_r1444982918
########## core/src/main/java/org/apache/iceberg/MetricsUtil.java: ########## @@ -174,6 +175,8 @@ public static MetricsModes.MetricsMode metricsMode( field.type(), file.upperBounds().get(field.fieldId())))); public static final String READABLE_METRICS = "readable_metrics"; + public static final String REF_SNAPSHOT_ID = "reference_snapshot_id"; Review Comment: I feel like this constant is not too suitable in this file, which is for Metrics. I think if we have to have a constant, should be in BaseMetadataTable. Else we can just actually hard code it, it seems simpler. ########## core/src/main/java/org/apache/iceberg/AllEntriesTable.java: ########## @@ -64,9 +84,18 @@ protected TableScan newRefinedScan(Table table, Schema schema, TableScanContext @Override protected CloseableIterable<FileScanTask> doPlanFiles() { - CloseableIterable<ManifestFile> manifests = - reachableManifests(snapshot -> snapshot.allManifests(table().io())); - return BaseEntriesTable.planFiles(table(), manifests, tableSchema(), schema(), context()); + return new ParallelIterable<>( Review Comment: Looks like we don't close the ParallelIterable like in the original reachableManifests method. Also looks like we are going to call planFiles many more times than the original. My first thought is to keep the logic but use a Pair<ManifestFile, Snapshot> where ManifestFile is used now. How about adapting the correct reachableManifest code into a new more generic method like T traverse(Function<Snapshot, T> func) and have reachableManifests use 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: 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