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: [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]
