szehon-ho commented on code in PR #6661: URL: https://github.com/apache/iceberg/pull/6661#discussion_r1185548566
########## core/src/main/java/org/apache/iceberg/PartitionsTable.java: ########## @@ -93,31 +129,55 @@ private static StaticDataTask.Row convertPartition(Partition partition) { partition.partitionData, partition.specId, partition.dataRecordCount, - partition.dataFileCount); + partition.dataFileCount, + partition.posDeleteRecordCount, + partition.posDeleteFileCount, + partition.eqDeleteRecordCount, + partition.eqDeleteFileCount); } private static Iterable<Partition> partitions(Table table, StaticTableScan scan) { Types.StructType partitionType = Partitioning.partitionType(table); PartitionMap partitions = new PartitionMap(partitionType); - CloseableIterable<DataFile> datafiles = planDataFiles(scan); - for (DataFile dataFile : datafiles) { - StructLike partition = - PartitionUtil.coercePartition( - partitionType, table.specs().get(dataFile.specId()), dataFile.partition()); - partitions.get(partition).update(dataFile); + try (CloseableIterable<ContentFile<?>> files = planFiles(scan)) { + for (ContentFile<?> file : files) { + StructLike partition = + PartitionUtil.coercePartition( + partitionType, table.specs().get(file.specId()), file.partition()); + partitions.get(partition).update(file); + } + } catch (IOException e) { + throw new UncheckedIOException(e); } return partitions.all(); } @VisibleForTesting - static CloseableIterable<DataFile> planDataFiles(StaticTableScan scan) { + static CloseableIterable<ContentFile<?>> planFiles(StaticTableScan scan) { Table table = scan.table(); - Snapshot snapshot = scan.snapshot(); - CloseableIterable<ManifestFile> dataManifests = - CloseableIterable.withNoopClose(snapshot.dataManifests(table.io())); + CloseableIterable<ManifestFile> filteredManifests = + filteredManifests(scan, table, scan.snapshot().allManifests(table.io())); + + Iterable<CloseableIterable<ContentFile<?>>> tasks = + CloseableIterable.transform( + filteredManifests, + manifest -> + CloseableIterable.transform( + ManifestFiles.open(manifest, table.io(), table.specs()) + .caseSensitive(scan.isCaseSensitive()) + .select(BaseScan.DELETE_SCAN_COLUMNS), // don't select stats columns Review Comment: Nit: can do a switch on content to get either DELETE_SCAN_COLUMNS or SCAN_COLUMNS to make it clearer. ########## core/src/main/java/org/apache/iceberg/PartitionsTable.java: ########## @@ -58,7 +81,13 @@ public TableScan newScan() { @Override public Schema schema() { if (table().spec().fields().size() < 1) { - return schema.select("record_count", "file_count"); Review Comment: I think there's actually a bug here, and it only checks latest spec for Unpartitioned. if we have partition fields before but removed them, we will not show them. See other metadata tables like BaseFilesTable.schema. Anyway its unrelated, but made https://github.com/apache/iceberg/issues/7533 to track it. -- 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