aokolnychyi commented on code in PR #4566: URL: https://github.com/apache/iceberg/pull/4566#discussion_r850880497
########## core/src/main/java/org/apache/iceberg/AllManifestsTable.java: ########## @@ -85,58 +85,41 @@ MetadataTableType metadataTableType() { public static class AllManifestsTableScan extends BaseAllMetadataTableScan { AllManifestsTableScan(TableOperations ops, Table table, Schema fileSchema) { - super(ops, table, fileSchema); + super(ops, table, fileSchema, MetadataTableType.ALL_MANIFESTS); } private AllManifestsTableScan(TableOperations ops, Table table, Schema schema, TableScanContext context) { - super(ops, table, schema, context); + super(ops, table, schema, MetadataTableType.ALL_MANIFESTS, context); } @Override - protected TableScan newRefinedScan(TableOperations ops, Table table, Schema schema, - TableScanContext context) { - return new AllManifestsTableScan(ops, table, schema, context); + protected TableScan newRefinedScan(Schema schema, TableScanContext context) { + return new AllManifestsTableScan(ops(), table(), schema, context); } @Override - public TableScan useSnapshot(long scanSnapshotId) { - throw new UnsupportedOperationException("Cannot select snapshot: all_manifests is for all snapshots"); - } - - @Override - public TableScan asOfTime(long timestampMillis) { - throw new UnsupportedOperationException("Cannot select snapshot: all_manifests is for all snapshots"); - } - - @Override - protected String tableType() { - return MetadataTableType.ALL_MANIFESTS.name(); - } - - @Override - protected CloseableIterable<FileScanTask> planFiles( - TableOperations ops, Snapshot snapshot, Expression rowFilter, - boolean ignoreResiduals, boolean caseSensitive, boolean colStats) { + protected CloseableIterable<FileScanTask> doPlanFiles() { + FileIO io = table().io(); String schemaString = SchemaParser.toJson(schema()); String specString = PartitionSpecParser.toJson(PartitionSpec.unpartitioned()); Map<Integer, PartitionSpec> specs = Maps.newHashMap(table().specs()); + Expression filter = shouldIgnoreResiduals() ? Expressions.alwaysTrue() : filter(); Review Comment: Moved it from the loop. Don't think there is a reason to create these instances for each snapshot. ########## core/src/main/java/org/apache/iceberg/AllManifestsTable.java: ########## @@ -85,58 +85,41 @@ MetadataTableType metadataTableType() { public static class AllManifestsTableScan extends BaseAllMetadataTableScan { AllManifestsTableScan(TableOperations ops, Table table, Schema fileSchema) { - super(ops, table, fileSchema); + super(ops, table, fileSchema, MetadataTableType.ALL_MANIFESTS); } private AllManifestsTableScan(TableOperations ops, Table table, Schema schema, TableScanContext context) { - super(ops, table, schema, context); + super(ops, table, schema, MetadataTableType.ALL_MANIFESTS, context); } @Override - protected TableScan newRefinedScan(TableOperations ops, Table table, Schema schema, - TableScanContext context) { - return new AllManifestsTableScan(ops, table, schema, context); + protected TableScan newRefinedScan(Schema schema, TableScanContext context) { + return new AllManifestsTableScan(ops(), table(), schema, context); } @Override - public TableScan useSnapshot(long scanSnapshotId) { - throw new UnsupportedOperationException("Cannot select snapshot: all_manifests is for all snapshots"); - } - - @Override - public TableScan asOfTime(long timestampMillis) { - throw new UnsupportedOperationException("Cannot select snapshot: all_manifests is for all snapshots"); - } - - @Override - protected String tableType() { - return MetadataTableType.ALL_MANIFESTS.name(); - } - - @Override - protected CloseableIterable<FileScanTask> planFiles( - TableOperations ops, Snapshot snapshot, Expression rowFilter, - boolean ignoreResiduals, boolean caseSensitive, boolean colStats) { + protected CloseableIterable<FileScanTask> doPlanFiles() { + FileIO io = table().io(); String schemaString = SchemaParser.toJson(schema()); String specString = PartitionSpecParser.toJson(PartitionSpec.unpartitioned()); Map<Integer, PartitionSpec> specs = Maps.newHashMap(table().specs()); + Expression filter = shouldIgnoreResiduals() ? Expressions.alwaysTrue() : filter(); Review Comment: Moved it out of the loop. Don't think there is a reason to create these instances for each snapshot. -- 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