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

Reply via email to