You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/04/14 22:56:46 UTC
[GitHub] [iceberg] aokolnychyi opened a new pull request, #4566: Core: Refactor metadata tables and scans
aokolnychyi opened a new pull request, #4566:
URL: https://github.com/apache/iceberg/pull/4566
This PR refactors metadata tables and scans. The project has evolved since the initial implementation was done and I think that metadata tables and scans can be cleaned up a little bit.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r850879149
##########
core/src/main/java/org/apache/iceberg/AllDataFilesTable.java:
##########
@@ -53,47 +48,25 @@ MetadataTableType metadataTableType() {
return MetadataTableType.ALL_DATA_FILES;
}
- public static class AllDataFilesTableScan extends BaseFilesTableScan {
+ public static class AllDataFilesTableScan extends BaseAllFilesTableScan {
- AllDataFilesTableScan(TableOperations ops, Table table, Schema fileSchema) {
- super(ops, table, fileSchema, MetadataTableType.ALL_DATA_FILES);
+ AllDataFilesTableScan(TableOperations ops, Table table, Schema schema) {
+ super(ops, table, schema, MetadataTableType.ALL_DATA_FILES);
}
- private AllDataFilesTableScan(TableOperations ops, Table table, Schema schema, Schema fileSchema,
+ private AllDataFilesTableScan(TableOperations ops, Table table, Schema schema,
TableScanContext context) {
- super(ops, table, schema, fileSchema, context, MetadataTableType.ALL_DATA_FILES);
+ super(ops, table, schema, MetadataTableType.ALL_DATA_FILES, context);
}
@Override
- protected TableScan newRefinedScan(TableOperations ops, Table table, Schema schema, TableScanContext context) {
- return new AllDataFilesTableScan(ops, table, schema, fileSchema(), context);
- }
-
- @Override
- public TableScan useSnapshot(long scanSnapshotId) {
- throw new UnsupportedOperationException("Cannot select snapshot: all_data_files is for all snapshots");
- }
-
- @Override
- public TableScan asOfTime(long timestampMillis) {
- throw new UnsupportedOperationException("Cannot select snapshot: all_data_files is for all snapshots");
- }
-
- @Override
- public CloseableIterable<FileScanTask> planFiles() {
- return super.planFilesAllSnapshots();
+ protected TableScan newRefinedScan(Schema schema, TableScanContext context) {
+ return new AllDataFilesTableScan(ops(), table(), schema, context);
}
@Override
protected CloseableIterable<ManifestFile> manifests() {
- try (CloseableIterable<ManifestFile> iterable = new ParallelIterable<>(
Review Comment:
This logic was is in a few places. I moved it to the parent class and share in all places.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r850878770
##########
core/src/main/java/org/apache/iceberg/AllDataFilesTable.java:
##########
@@ -53,47 +48,25 @@ MetadataTableType metadataTableType() {
return MetadataTableType.ALL_DATA_FILES;
}
- public static class AllDataFilesTableScan extends BaseFilesTableScan {
+ public static class AllDataFilesTableScan extends BaseAllFilesTableScan {
- AllDataFilesTableScan(TableOperations ops, Table table, Schema fileSchema) {
- super(ops, table, fileSchema, MetadataTableType.ALL_DATA_FILES);
+ AllDataFilesTableScan(TableOperations ops, Table table, Schema schema) {
+ super(ops, table, schema, MetadataTableType.ALL_DATA_FILES);
}
- private AllDataFilesTableScan(TableOperations ops, Table table, Schema schema, Schema fileSchema,
+ private AllDataFilesTableScan(TableOperations ops, Table table, Schema schema,
TableScanContext context) {
- super(ops, table, schema, fileSchema, context, MetadataTableType.ALL_DATA_FILES);
+ super(ops, table, schema, MetadataTableType.ALL_DATA_FILES, context);
}
@Override
- protected TableScan newRefinedScan(TableOperations ops, Table table, Schema schema, TableScanContext context) {
- return new AllDataFilesTableScan(ops, table, schema, fileSchema(), context);
- }
-
- @Override
- public TableScan useSnapshot(long scanSnapshotId) {
Review Comment:
These got moved to the parent as none of the aggregated metadata tables support selecting a 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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r850881483
##########
core/src/main/java/org/apache/iceberg/BaseMetadataTableScan.java:
##########
@@ -19,39 +19,50 @@
package org.apache.iceberg;
-import org.apache.iceberg.events.Listeners;
-import org.apache.iceberg.events.ScanEvent;
-import org.apache.iceberg.io.CloseableIterable;
import org.apache.iceberg.util.PropertyUtil;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
abstract class BaseMetadataTableScan extends BaseTableScan {
- private static final Logger LOG = LoggerFactory.getLogger(BaseMetadataTableScan.class);
- protected BaseMetadataTableScan(TableOperations ops, Table table, Schema schema) {
+ private final MetadataTableType tableType;
+
+ protected BaseMetadataTableScan(TableOperations ops, Table table, Schema schema, MetadataTableType tableType) {
super(ops, table, schema);
+ this.tableType = tableType;
}
- protected BaseMetadataTableScan(TableOperations ops, Table table, Schema schema, TableScanContext context) {
+ protected BaseMetadataTableScan(TableOperations ops, Table table, Schema schema, MetadataTableType tableType,
+ TableScanContext context) {
super(ops, table, schema, context);
+ this.tableType = tableType;
+ }
+
+ /**
+ * Type of scan being performed, such as {@link MetadataTableType#ALL_DATA_FILES} when scanning
+ * a table's {@link org.apache.iceberg.AllDataFilesTable}.
+ * <p>
+ * Used for logging and error messages.
+ */
+ protected MetadataTableType tableType() {
+ return tableType;
+ }
+
+ @Override
+ public TableScan appendsBetween(long fromSnapshotId, long toSnapshotId) {
Review Comment:
None of the metadata tables support incremental scans so we can share this logic.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r850880730
##########
core/src/main/java/org/apache/iceberg/BaseAllMetadataTableScan.java:
##########
@@ -19,40 +19,56 @@
package org.apache.iceberg;
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import org.apache.iceberg.events.Listeners;
+import org.apache.iceberg.events.ScanEvent;
import org.apache.iceberg.io.CloseableIterable;
+import org.apache.iceberg.relocated.com.google.common.base.Function;
+import org.apache.iceberg.relocated.com.google.common.collect.Iterables;
+import org.apache.iceberg.relocated.com.google.common.collect.Sets;
+import org.apache.iceberg.util.ParallelIterable;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
abstract class BaseAllMetadataTableScan extends BaseMetadataTableScan {
+ private static final Logger LOG = LoggerFactory.getLogger(BaseAllMetadataTableScan.class);
- BaseAllMetadataTableScan(TableOperations ops, Table table, Schema fileSchema) {
- super(ops, table, fileSchema);
+ BaseAllMetadataTableScan(TableOperations ops, Table table, Schema schema, MetadataTableType tableType) {
+ super(ops, table, schema, tableType);
}
- BaseAllMetadataTableScan(TableOperations ops, Table table, Schema schema, TableScanContext context) {
- super(ops, table, schema, context);
+ BaseAllMetadataTableScan(TableOperations ops, Table table, Schema schema, MetadataTableType tableType,
+ TableScanContext context) {
+ super(ops, table, schema, tableType, context);
}
- /**
- * Type of scan being performed, such as {@link MetadataTableType#ALL_DATA_FILES} when scanning
- * a table's {@link org.apache.iceberg.AllDataFilesTable}.
- * <p>
- * Used for logging and error messages.
- */
- protected abstract String tableType();
Review Comment:
Moved it to the base metadata table scan as all metadata tables will need this.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r855696837
##########
core/src/main/java/org/apache/iceberg/DataTableScan.java:
##########
@@ -78,23 +76,23 @@ protected TableScan newRefinedScan(TableOperations ops, Table table, Schema sche
}
@Override
- public CloseableIterable<FileScanTask> planFiles(TableOperations ops, Snapshot snapshot,
- Expression rowFilter, boolean ignoreResiduals,
- boolean caseSensitive, boolean colStats) {
- ManifestGroup manifestGroup = new ManifestGroup(ops.io(), snapshot.dataManifests(), snapshot.deleteManifests())
- .caseSensitive(caseSensitive)
- .select(colStats ? SCAN_WITH_STATS_COLUMNS : SCAN_COLUMNS)
- .filterData(rowFilter)
- .specsById(ops.current().specsById())
+ public CloseableIterable<FileScanTask> doPlanFiles() {
+ Snapshot snapshot = snapshot();
+
+ ManifestGroup manifestGroup = new ManifestGroup(table().io(), snapshot.dataManifests(), snapshot.deleteManifests())
+ .caseSensitive(isCaseSensitive())
+ .select(colStats() ? SCAN_WITH_STATS_COLUMNS : SCAN_COLUMNS)
+ .filterData(filter())
+ .specsById(table().specs())
.ignoreDeleted();
- if (ignoreResiduals) {
+ if (shouldIgnoreResiduals()) {
Review Comment:
I cann't because of a conflicting setter method discussed [here](https://github.com/apache/iceberg/pull/4566#discussion_r855626731).
##########
core/src/main/java/org/apache/iceberg/DataTableScan.java:
##########
@@ -78,23 +76,23 @@ protected TableScan newRefinedScan(TableOperations ops, Table table, Schema sche
}
@Override
- public CloseableIterable<FileScanTask> planFiles(TableOperations ops, Snapshot snapshot,
- Expression rowFilter, boolean ignoreResiduals,
- boolean caseSensitive, boolean colStats) {
- ManifestGroup manifestGroup = new ManifestGroup(ops.io(), snapshot.dataManifests(), snapshot.deleteManifests())
- .caseSensitive(caseSensitive)
- .select(colStats ? SCAN_WITH_STATS_COLUMNS : SCAN_COLUMNS)
- .filterData(rowFilter)
- .specsById(ops.current().specsById())
+ public CloseableIterable<FileScanTask> doPlanFiles() {
+ Snapshot snapshot = snapshot();
+
+ ManifestGroup manifestGroup = new ManifestGroup(table().io(), snapshot.dataManifests(), snapshot.deleteManifests())
+ .caseSensitive(isCaseSensitive())
+ .select(colStats() ? SCAN_WITH_STATS_COLUMNS : SCAN_COLUMNS)
+ .filterData(filter())
+ .specsById(table().specs())
.ignoreDeleted();
- if (ignoreResiduals) {
+ if (shouldIgnoreResiduals()) {
Review Comment:
I can't because of a conflicting setter method discussed [here](https://github.com/apache/iceberg/pull/4566#discussion_r855626731).
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r850894951
##########
core/src/main/java/org/apache/iceberg/BaseTableScan.java:
##########
@@ -83,22 +87,23 @@ protected Collection<String> selectedColumns() {
return context.selectedColumns();
}
+ protected ExecutorService planExecutor() {
+ return context.planExecutor();
+ }
+
protected Map<String, String> options() {
return context.options();
}
- protected TableScanContext context() {
+ protected TableScanContext context() {
return context;
}
@SuppressWarnings("checkstyle:HiddenField")
- protected abstract TableScan newRefinedScan(
- TableOperations ops, Table table, Schema schema, TableScanContext context);
+ protected abstract TableScan newRefinedScan(TableOperations ops, Table table, Schema schema,
+ TableScanContext context);
- @SuppressWarnings("checkstyle:HiddenField")
- protected abstract CloseableIterable<FileScanTask> planFiles(
Review Comment:
I am not sure the signature of this method makes sense after we introduced the context object. This method passes some arguments but not all so the implementations are not entirely pure. Also, the passed arguments are useless for some metadata tables. That's why I see little value in having so many arguments. Just makes calling this method harder.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r850880998
##########
core/src/main/java/org/apache/iceberg/BaseFilesTable.java:
##########
@@ -56,72 +56,73 @@ public Schema schema() {
}
}
- abstract static class BaseFilesTableScan extends BaseMetadataTableScan {
+ private static CloseableIterable<FileScanTask> planFiles(Table table, CloseableIterable<ManifestFile> manifests,
Review Comment:
Moved the common planning logic to a static method to share in both scans.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r850880061
##########
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();
+ ResidualEvaluator residuals = ResidualEvaluator.unpartitioned(filter);
- return CloseableIterable.withNoopClose(Iterables.transform(ops.current().snapshots(), snap -> {
Review Comment:
I checked and `table()` returns the main table in all cases so `table().snapshots()` is equivalent to accessing table operations but shorter and uses a higher level API.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi merged pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
aokolnychyi merged PR #4566:
URL: https://github.com/apache/iceberg/pull/4566
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r850880061
##########
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();
+ ResidualEvaluator residuals = ResidualEvaluator.unpartitioned(filter);
- return CloseableIterable.withNoopClose(Iterables.transform(ops.current().snapshots(), snap -> {
Review Comment:
I checked and `table()` returns the main table in all cases so `table().snapshots()` is equivalent to accessing table operations but shorter and uses a higher level API. We do have other places that access `table()` already.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r855626731
##########
core/src/main/java/org/apache/iceberg/AllEntriesTable.java:
##########
@@ -88,34 +83,16 @@ protected TableScan newRefinedScan(TableOperations ops, Table table, Schema sche
}
@Override
- protected String tableType() {
- return MetadataTableType.ALL_ENTRIES.name();
- }
+ protected CloseableIterable<FileScanTask> doPlanFiles() {
+ CloseableIterable<ManifestFile> manifests = allManifests(Snapshot::allManifests);
- @Override
- protected CloseableIterable<FileScanTask> planFiles(
- TableOperations ops, Snapshot snapshot, Expression rowFilter,
- boolean ignoreResiduals, boolean caseSensitive, boolean colStats) {
- CloseableIterable<ManifestFile> manifests = allManifestFiles(
- ops.current().snapshots(), context().planExecutor());
String schemaString = SchemaParser.toJson(schema());
String specString = PartitionSpecParser.toJson(PartitionSpec.unpartitioned());
- Expression filter = ignoreResiduals ? Expressions.alwaysTrue() : rowFilter;
+ Expression filter = shouldIgnoreResiduals() ? Expressions.alwaysTrue() : filter();
Review Comment:
Yeah, it is because of a conflicting setter method. Keeping as-is.
```
@Override
public TableScan ignoreResiduals() {
return newRefinedScan(ops, table, schema, context.ignoreResiduals(true));
}
```
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#issuecomment-1105912113
Thanks for reviewing, @szehon-ho @flyrain @rdblue!
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r850878470
##########
core/src/main/java/org/apache/iceberg/AllDataFilesTable.java:
##########
@@ -53,47 +48,25 @@ MetadataTableType metadataTableType() {
return MetadataTableType.ALL_DATA_FILES;
}
- public static class AllDataFilesTableScan extends BaseFilesTableScan {
+ public static class AllDataFilesTableScan extends BaseAllFilesTableScan {
- AllDataFilesTableScan(TableOperations ops, Table table, Schema fileSchema) {
- super(ops, table, fileSchema, MetadataTableType.ALL_DATA_FILES);
+ AllDataFilesTableScan(TableOperations ops, Table table, Schema schema) {
Review Comment:
I took a closer look and it seems it is not really `fileSchema`. It is more the metadata table schema before column pruning. That's the purpose of that variable in `BaseTableScan`.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r850881237
##########
core/src/main/java/org/apache/iceberg/BaseFilesTable.java:
##########
@@ -56,72 +56,73 @@ public Schema schema() {
}
}
- abstract static class BaseFilesTableScan extends BaseMetadataTableScan {
+ private static CloseableIterable<FileScanTask> planFiles(Table table, CloseableIterable<ManifestFile> manifests,
Review Comment:
I also removed the comment about column pruning as I think it is no longer accurate. We do support column pruning for metadata tables.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r851424146
##########
core/src/main/java/org/apache/iceberg/AllEntriesTable.java:
##########
@@ -88,34 +83,16 @@ protected TableScan newRefinedScan(TableOperations ops, Table table, Schema sche
}
@Override
- protected String tableType() {
- return MetadataTableType.ALL_ENTRIES.name();
- }
+ protected CloseableIterable<FileScanTask> doPlanFiles() {
+ CloseableIterable<ManifestFile> manifests = allManifests(Snapshot::allManifests);
- @Override
- protected CloseableIterable<FileScanTask> planFiles(
- TableOperations ops, Snapshot snapshot, Expression rowFilter,
- boolean ignoreResiduals, boolean caseSensitive, boolean colStats) {
- CloseableIterable<ManifestFile> manifests = allManifestFiles(
- ops.current().snapshots(), context().planExecutor());
String schemaString = SchemaParser.toJson(schema());
String specString = PartitionSpecParser.toJson(PartitionSpec.unpartitioned());
- Expression filter = ignoreResiduals ? Expressions.alwaysTrue() : rowFilter;
+ Expression filter = shouldIgnoreResiduals() ? Expressions.alwaysTrue() : filter();
Review Comment:
I think it may be something related to having `ignoreResiduals()` that acts like a setter. I'll have to check the existing implementation. I used what was already available.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r855628692
##########
core/src/main/java/org/apache/iceberg/BaseAllMetadataTableScan.java:
##########
@@ -19,40 +19,56 @@
package org.apache.iceberg;
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import org.apache.iceberg.events.Listeners;
+import org.apache.iceberg.events.ScanEvent;
import org.apache.iceberg.io.CloseableIterable;
+import org.apache.iceberg.relocated.com.google.common.base.Function;
+import org.apache.iceberg.relocated.com.google.common.collect.Iterables;
+import org.apache.iceberg.relocated.com.google.common.collect.Sets;
+import org.apache.iceberg.util.ParallelIterable;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
abstract class BaseAllMetadataTableScan extends BaseMetadataTableScan {
+ private static final Logger LOG = LoggerFactory.getLogger(BaseAllMetadataTableScan.class);
- BaseAllMetadataTableScan(TableOperations ops, Table table, Schema fileSchema) {
- super(ops, table, fileSchema);
+ BaseAllMetadataTableScan(TableOperations ops, Table table, Schema schema, MetadataTableType tableType) {
+ super(ops, table, schema, tableType);
}
- BaseAllMetadataTableScan(TableOperations ops, Table table, Schema schema, TableScanContext context) {
- super(ops, table, schema, context);
+ BaseAllMetadataTableScan(TableOperations ops, Table table, Schema schema, MetadataTableType tableType,
+ TableScanContext context) {
+ super(ops, table, schema, tableType, context);
}
- /**
- * Type of scan being performed, such as {@link MetadataTableType#ALL_DATA_FILES} when scanning
- * a table's {@link org.apache.iceberg.AllDataFilesTable}.
- * <p>
- * Used for logging and error messages.
- */
- protected abstract String tableType();
-
@Override
- public TableScan appendsBetween(long fromSnapshotId, long toSnapshotId) {
- throw new UnsupportedOperationException(
- String.format("Cannot incrementally scan table of type %s", tableType()));
+ public TableScan useSnapshot(long scanSnapshotId) {
+ throw new UnsupportedOperationException("Cannot select snapshot in table: " + tableType());
}
@Override
- public TableScan appendsAfter(long fromSnapshotId) {
- throw new UnsupportedOperationException(
- String.format("Cannot incrementally scan table of type %s", tableType()));
+ public TableScan asOfTime(long timestampMillis) {
+ throw new UnsupportedOperationException("Cannot select snapshot in table: " + tableType());
}
@Override
public CloseableIterable<FileScanTask> planFiles() {
- return super.planFilesAllSnapshots();
+ LOG.info("Scanning metadata table {} with filter {}.", table(), filter());
+ Listeners.notifyAll(new ScanEvent(table().name(), 0L, filter(), schema()));
+
+ return doPlanFiles();
+ }
+
+ protected CloseableIterable<ManifestFile> reachableManifests(Function<Snapshot, Iterable<ManifestFile>> toManifests) {
Review Comment:
@szehon-ho, I went with `reachableManifests` as `allSnapshotsManifests` was too long to stay on one line. I think we use the term `reachableXXX` in a few places that scan through all snapshots.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r850877640
##########
core/src/main/java/org/apache/iceberg/AllDataFilesTable.java:
##########
@@ -53,47 +48,25 @@ MetadataTableType metadataTableType() {
return MetadataTableType.ALL_DATA_FILES;
}
- public static class AllDataFilesTableScan extends BaseFilesTableScan {
+ public static class AllDataFilesTableScan extends BaseAllFilesTableScan {
Review Comment:
I added `BaseAllFilesTableScan` for all aggregated file tables as I think it is slightly cleaner. That way, we can properly override the planning method in the parent class and consume in all children.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#issuecomment-1099764828
cc @kbendick @szehon-ho @karuppayya @RussellSpitzer @flyrain @rdblue
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r851423594
##########
core/src/main/java/org/apache/iceberg/BaseAllMetadataTableScan.java:
##########
@@ -19,40 +19,56 @@
package org.apache.iceberg;
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import org.apache.iceberg.events.Listeners;
+import org.apache.iceberg.events.ScanEvent;
import org.apache.iceberg.io.CloseableIterable;
+import org.apache.iceberg.relocated.com.google.common.base.Function;
+import org.apache.iceberg.relocated.com.google.common.collect.Iterables;
+import org.apache.iceberg.relocated.com.google.common.collect.Sets;
+import org.apache.iceberg.util.ParallelIterable;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
abstract class BaseAllMetadataTableScan extends BaseMetadataTableScan {
+ private static final Logger LOG = LoggerFactory.getLogger(BaseAllMetadataTableScan.class);
- BaseAllMetadataTableScan(TableOperations ops, Table table, Schema fileSchema) {
- super(ops, table, fileSchema);
+ BaseAllMetadataTableScan(TableOperations ops, Table table, Schema schema, MetadataTableType tableType) {
+ super(ops, table, schema, tableType);
}
- BaseAllMetadataTableScan(TableOperations ops, Table table, Schema schema, TableScanContext context) {
- super(ops, table, schema, context);
+ BaseAllMetadataTableScan(TableOperations ops, Table table, Schema schema, MetadataTableType tableType,
+ TableScanContext context) {
+ super(ops, table, schema, tableType, context);
}
- /**
- * Type of scan being performed, such as {@link MetadataTableType#ALL_DATA_FILES} when scanning
- * a table's {@link org.apache.iceberg.AllDataFilesTable}.
- * <p>
- * Used for logging and error messages.
- */
- protected abstract String tableType();
-
@Override
- public TableScan appendsBetween(long fromSnapshotId, long toSnapshotId) {
- throw new UnsupportedOperationException(
- String.format("Cannot incrementally scan table of type %s", tableType()));
+ public TableScan useSnapshot(long scanSnapshotId) {
+ throw new UnsupportedOperationException("Cannot select snapshot in table: " + tableType());
}
@Override
- public TableScan appendsAfter(long fromSnapshotId) {
- throw new UnsupportedOperationException(
- String.format("Cannot incrementally scan table of type %s", tableType()));
+ public TableScan asOfTime(long timestampMillis) {
+ throw new UnsupportedOperationException("Cannot select snapshot in table: " + tableType());
}
@Override
public CloseableIterable<FileScanTask> planFiles() {
- return super.planFilesAllSnapshots();
+ LOG.info("Scanning metadata table {} with filter {}.", table(), filter());
+ Listeners.notifyAll(new ScanEvent(table().name(), 0L, filter(), schema()));
+
+ return doPlanFiles();
+ }
+
+ protected CloseableIterable<ManifestFile> allManifests(Function<Snapshot, Iterable<ManifestFile>> toManifests) {
Review Comment:
Agreed. Will change.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] szehon-ho commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
szehon-ho commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r851444055
##########
core/src/main/java/org/apache/iceberg/BaseFilesTable.java:
##########
@@ -56,72 +56,73 @@ public Schema schema() {
}
}
- abstract static class BaseFilesTableScan extends BaseMetadataTableScan {
+ private static CloseableIterable<FileScanTask> planFiles(Table table, CloseableIterable<ManifestFile> manifests,
+ Schema tableSchema, Schema projectedSchema,
+ TableScanContext context) {
+ Expression rowFilter = context.rowFilter();
+ boolean caseSensitive = context.caseSensitive();
+ boolean ignoreResiduals = context.ignoreResiduals();
+
+ LoadingCache<Integer, ManifestEvaluator> evalCache = Caffeine.newBuilder().build(specId -> {
Review Comment:
Yea no problem, I can look at it later, it's part of the code but not the whole thing as PartitionsTable creates another kind of FileScanTask.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] flyrain commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
flyrain commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r855695420
##########
core/src/main/java/org/apache/iceberg/DataTableScan.java:
##########
@@ -78,23 +76,23 @@ protected TableScan newRefinedScan(TableOperations ops, Table table, Schema sche
}
@Override
- public CloseableIterable<FileScanTask> planFiles(TableOperations ops, Snapshot snapshot,
- Expression rowFilter, boolean ignoreResiduals,
- boolean caseSensitive, boolean colStats) {
- ManifestGroup manifestGroup = new ManifestGroup(ops.io(), snapshot.dataManifests(), snapshot.deleteManifests())
- .caseSensitive(caseSensitive)
- .select(colStats ? SCAN_WITH_STATS_COLUMNS : SCAN_COLUMNS)
- .filterData(rowFilter)
- .specsById(ops.current().specsById())
+ public CloseableIterable<FileScanTask> doPlanFiles() {
+ Snapshot snapshot = snapshot();
+
+ ManifestGroup manifestGroup = new ManifestGroup(table().io(), snapshot.dataManifests(), snapshot.deleteManifests())
+ .caseSensitive(isCaseSensitive())
+ .select(colStats() ? SCAN_WITH_STATS_COLUMNS : SCAN_COLUMNS)
+ .filterData(filter())
+ .specsById(table().specs())
.ignoreDeleted();
- if (ignoreResiduals) {
+ if (shouldIgnoreResiduals()) {
Review Comment:
Nit: may rename `shouldIgnoreResiduals()` to `ignoreResiduals()` to be consistent.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] szehon-ho commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
szehon-ho commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r851382921
##########
core/src/main/java/org/apache/iceberg/BaseFilesTable.java:
##########
@@ -56,72 +56,73 @@ public Schema schema() {
}
}
- abstract static class BaseFilesTableScan extends BaseMetadataTableScan {
+ private static CloseableIterable<FileScanTask> planFiles(Table table, CloseableIterable<ManifestFile> manifests,
+ Schema tableSchema, Schema projectedSchema,
+ TableScanContext context) {
+ Expression rowFilter = context.rowFilter();
+ boolean caseSensitive = context.caseSensitive();
+ boolean ignoreResiduals = context.ignoreResiduals();
+
+ LoadingCache<Integer, ManifestEvaluator> evalCache = Caffeine.newBuilder().build(specId -> {
Review Comment:
Not related to this change, but my next pr plans to use this logic as well to fix filtering for PartitionsTable. Maybe at that time will move the evalCache part of this further up to BaseMetadataTables.
##########
core/src/main/java/org/apache/iceberg/BaseAllMetadataTableScan.java:
##########
@@ -19,40 +19,56 @@
package org.apache.iceberg;
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import org.apache.iceberg.events.Listeners;
+import org.apache.iceberg.events.ScanEvent;
import org.apache.iceberg.io.CloseableIterable;
+import org.apache.iceberg.relocated.com.google.common.base.Function;
+import org.apache.iceberg.relocated.com.google.common.collect.Iterables;
+import org.apache.iceberg.relocated.com.google.common.collect.Sets;
+import org.apache.iceberg.util.ParallelIterable;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
abstract class BaseAllMetadataTableScan extends BaseMetadataTableScan {
+ private static final Logger LOG = LoggerFactory.getLogger(BaseAllMetadataTableScan.class);
- BaseAllMetadataTableScan(TableOperations ops, Table table, Schema fileSchema) {
- super(ops, table, fileSchema);
+ BaseAllMetadataTableScan(TableOperations ops, Table table, Schema schema, MetadataTableType tableType) {
+ super(ops, table, schema, tableType);
}
- BaseAllMetadataTableScan(TableOperations ops, Table table, Schema schema, TableScanContext context) {
- super(ops, table, schema, context);
+ BaseAllMetadataTableScan(TableOperations ops, Table table, Schema schema, MetadataTableType tableType,
+ TableScanContext context) {
+ super(ops, table, schema, tableType, context);
}
- /**
- * Type of scan being performed, such as {@link MetadataTableType#ALL_DATA_FILES} when scanning
- * a table's {@link org.apache.iceberg.AllDataFilesTable}.
- * <p>
- * Used for logging and error messages.
- */
- protected abstract String tableType();
-
@Override
- public TableScan appendsBetween(long fromSnapshotId, long toSnapshotId) {
- throw new UnsupportedOperationException(
- String.format("Cannot incrementally scan table of type %s", tableType()));
+ public TableScan useSnapshot(long scanSnapshotId) {
+ throw new UnsupportedOperationException("Cannot select snapshot in table: " + tableType());
}
@Override
- public TableScan appendsAfter(long fromSnapshotId) {
- throw new UnsupportedOperationException(
- String.format("Cannot incrementally scan table of type %s", tableType()));
+ public TableScan asOfTime(long timestampMillis) {
+ throw new UnsupportedOperationException("Cannot select snapshot in table: " + tableType());
}
@Override
public CloseableIterable<FileScanTask> planFiles() {
- return super.planFilesAllSnapshots();
+ LOG.info("Scanning metadata table {} with filter {}.", table(), filter());
+ Listeners.notifyAll(new ScanEvent(table().name(), 0L, filter(), schema()));
+
+ return doPlanFiles();
+ }
+
+ protected CloseableIterable<ManifestFile> allManifests(Function<Snapshot, Iterable<ManifestFile>> toManifests) {
Review Comment:
Name is a bit confusing as 'allManifests' is already used to denote data + delete manifests. How about 'allSnapshotManifests' or 'allSnapshots' , or something to denote the fact that is explores all snapshots.
##########
core/src/main/java/org/apache/iceberg/AllEntriesTable.java:
##########
@@ -88,34 +83,16 @@ protected TableScan newRefinedScan(TableOperations ops, Table table, Schema sche
}
@Override
- protected String tableType() {
- return MetadataTableType.ALL_ENTRIES.name();
- }
+ protected CloseableIterable<FileScanTask> doPlanFiles() {
+ CloseableIterable<ManifestFile> manifests = allManifests(Snapshot::allManifests);
- @Override
- protected CloseableIterable<FileScanTask> planFiles(
- TableOperations ops, Snapshot snapshot, Expression rowFilter,
- boolean ignoreResiduals, boolean caseSensitive, boolean colStats) {
- CloseableIterable<ManifestFile> manifests = allManifestFiles(
- ops.current().snapshots(), context().planExecutor());
String schemaString = SchemaParser.toJson(schema());
String specString = PartitionSpecParser.toJson(PartitionSpec.unpartitioned());
- Expression filter = ignoreResiduals ? Expressions.alwaysTrue() : rowFilter;
+ Expression filter = shouldIgnoreResiduals() ? Expressions.alwaysTrue() : filter();
Review Comment:
Should we use ignoreResiduals throughout instead of "shouldIgnoreResiduals"? (as per code style we try to avoid useless words in the getter/setter method names, ignoreResiduals seems clear enough?)
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r851801354
##########
core/src/main/java/org/apache/iceberg/AllDataFilesTable.java:
##########
@@ -53,47 +48,25 @@ MetadataTableType metadataTableType() {
return MetadataTableType.ALL_DATA_FILES;
}
- public static class AllDataFilesTableScan extends BaseFilesTableScan {
+ public static class AllDataFilesTableScan extends BaseAllFilesTableScan {
- AllDataFilesTableScan(TableOperations ops, Table table, Schema fileSchema) {
- super(ops, table, fileSchema, MetadataTableType.ALL_DATA_FILES);
+ AllDataFilesTableScan(TableOperations ops, Table table, Schema schema) {
Review Comment:
I think it was called `fileSchema` because it was the projection of the `DataFile` struct.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r855629218
##########
core/src/main/java/org/apache/iceberg/BaseFilesTable.java:
##########
@@ -56,72 +56,73 @@ public Schema schema() {
}
}
- abstract static class BaseFilesTableScan extends BaseMetadataTableScan {
+ private static CloseableIterable<FileScanTask> planFiles(Table table, CloseableIterable<ManifestFile> manifests,
+ Schema tableSchema, Schema projectedSchema,
+ TableScanContext context) {
+ Expression rowFilter = context.rowFilter();
+ boolean caseSensitive = context.caseSensitive();
+ boolean ignoreResiduals = context.ignoreResiduals();
+
+ LoadingCache<Integer, ManifestEvaluator> evalCache = Caffeine.newBuilder().build(specId -> {
Review Comment:
Let's take a look at it in the PR that will use it then.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r855627397
##########
core/src/main/java/org/apache/iceberg/AllDataFilesTable.java:
##########
@@ -53,47 +48,25 @@ MetadataTableType metadataTableType() {
return MetadataTableType.ALL_DATA_FILES;
}
- public static class AllDataFilesTableScan extends BaseFilesTableScan {
+ public static class AllDataFilesTableScan extends BaseAllFilesTableScan {
- AllDataFilesTableScan(TableOperations ops, Table table, Schema fileSchema) {
- super(ops, table, fileSchema, MetadataTableType.ALL_DATA_FILES);
+ AllDataFilesTableScan(TableOperations ops, Table table, Schema schema) {
Review Comment:
Okay, that's why. I think it still makes sense to rename as I got rid of a separate `fileSchema` var.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] szehon-ho commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
szehon-ho commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r855665894
##########
core/src/main/java/org/apache/iceberg/BaseAllMetadataTableScan.java:
##########
@@ -19,40 +19,56 @@
package org.apache.iceberg;
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import org.apache.iceberg.events.Listeners;
+import org.apache.iceberg.events.ScanEvent;
import org.apache.iceberg.io.CloseableIterable;
+import org.apache.iceberg.relocated.com.google.common.base.Function;
+import org.apache.iceberg.relocated.com.google.common.collect.Iterables;
+import org.apache.iceberg.relocated.com.google.common.collect.Sets;
+import org.apache.iceberg.util.ParallelIterable;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
abstract class BaseAllMetadataTableScan extends BaseMetadataTableScan {
+ private static final Logger LOG = LoggerFactory.getLogger(BaseAllMetadataTableScan.class);
- BaseAllMetadataTableScan(TableOperations ops, Table table, Schema fileSchema) {
- super(ops, table, fileSchema);
+ BaseAllMetadataTableScan(TableOperations ops, Table table, Schema schema, MetadataTableType tableType) {
+ super(ops, table, schema, tableType);
}
- BaseAllMetadataTableScan(TableOperations ops, Table table, Schema schema, TableScanContext context) {
- super(ops, table, schema, context);
+ BaseAllMetadataTableScan(TableOperations ops, Table table, Schema schema, MetadataTableType tableType,
+ TableScanContext context) {
+ super(ops, table, schema, tableType, context);
}
- /**
- * Type of scan being performed, such as {@link MetadataTableType#ALL_DATA_FILES} when scanning
- * a table's {@link org.apache.iceberg.AllDataFilesTable}.
- * <p>
- * Used for logging and error messages.
- */
- protected abstract String tableType();
-
@Override
- public TableScan appendsBetween(long fromSnapshotId, long toSnapshotId) {
- throw new UnsupportedOperationException(
- String.format("Cannot incrementally scan table of type %s", tableType()));
+ public TableScan useSnapshot(long scanSnapshotId) {
+ throw new UnsupportedOperationException("Cannot select snapshot in table: " + tableType());
}
@Override
- public TableScan appendsAfter(long fromSnapshotId) {
- throw new UnsupportedOperationException(
- String.format("Cannot incrementally scan table of type %s", tableType()));
+ public TableScan asOfTime(long timestampMillis) {
+ throw new UnsupportedOperationException("Cannot select snapshot in table: " + tableType());
}
@Override
public CloseableIterable<FileScanTask> planFiles() {
- return super.planFilesAllSnapshots();
+ LOG.info("Scanning metadata table {} with filter {}.", table(), filter());
+ Listeners.notifyAll(new ScanEvent(table().name(), 0L, filter(), schema()));
+
+ return doPlanFiles();
+ }
+
+ protected CloseableIterable<ManifestFile> reachableManifests(Function<Snapshot, Iterable<ManifestFile>> toManifests) {
Review Comment:
That's a good name.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r855626731
##########
core/src/main/java/org/apache/iceberg/AllEntriesTable.java:
##########
@@ -88,34 +83,16 @@ protected TableScan newRefinedScan(TableOperations ops, Table table, Schema sche
}
@Override
- protected String tableType() {
- return MetadataTableType.ALL_ENTRIES.name();
- }
+ protected CloseableIterable<FileScanTask> doPlanFiles() {
+ CloseableIterable<ManifestFile> manifests = allManifests(Snapshot::allManifests);
- @Override
- protected CloseableIterable<FileScanTask> planFiles(
- TableOperations ops, Snapshot snapshot, Expression rowFilter,
- boolean ignoreResiduals, boolean caseSensitive, boolean colStats) {
- CloseableIterable<ManifestFile> manifests = allManifestFiles(
- ops.current().snapshots(), context().planExecutor());
String schemaString = SchemaParser.toJson(schema());
String specString = PartitionSpecParser.toJson(PartitionSpec.unpartitioned());
- Expression filter = ignoreResiduals ? Expressions.alwaysTrue() : rowFilter;
+ Expression filter = shouldIgnoreResiduals() ? Expressions.alwaysTrue() : filter();
Review Comment:
Yeah, it is because of a conflicting setter method.
```
@Override
public TableScan ignoreResiduals() {
return newRefinedScan(ops, table, schema, context.ignoreResiduals(true));
}
```
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r850878470
##########
core/src/main/java/org/apache/iceberg/AllDataFilesTable.java:
##########
@@ -53,47 +48,25 @@ MetadataTableType metadataTableType() {
return MetadataTableType.ALL_DATA_FILES;
}
- public static class AllDataFilesTableScan extends BaseFilesTableScan {
+ public static class AllDataFilesTableScan extends BaseAllFilesTableScan {
- AllDataFilesTableScan(TableOperations ops, Table table, Schema fileSchema) {
- super(ops, table, fileSchema, MetadataTableType.ALL_DATA_FILES);
+ AllDataFilesTableScan(TableOperations ops, Table table, Schema schema) {
Review Comment:
I took a closer look and it seems it is not really `fileSchema`. It is more like a table schema before column pruning. That's the purpose of that variable in `BaseTableScan`.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r850878470
##########
core/src/main/java/org/apache/iceberg/AllDataFilesTable.java:
##########
@@ -53,47 +48,25 @@ MetadataTableType metadataTableType() {
return MetadataTableType.ALL_DATA_FILES;
}
- public static class AllDataFilesTableScan extends BaseFilesTableScan {
+ public static class AllDataFilesTableScan extends BaseAllFilesTableScan {
- AllDataFilesTableScan(TableOperations ops, Table table, Schema fileSchema) {
- super(ops, table, fileSchema, MetadataTableType.ALL_DATA_FILES);
+ AllDataFilesTableScan(TableOperations ops, Table table, Schema schema) {
Review Comment:
I took a closer look and it seems it is not really `fileSchema`. It is more like a metadata table schema before column pruning. That's the purpose of that variable in `BaseTableScan`.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r851424812
##########
core/src/main/java/org/apache/iceberg/BaseFilesTable.java:
##########
@@ -56,72 +56,73 @@ public Schema schema() {
}
}
- abstract static class BaseFilesTableScan extends BaseMetadataTableScan {
+ private static CloseableIterable<FileScanTask> planFiles(Table table, CloseableIterable<ManifestFile> manifests,
+ Schema tableSchema, Schema projectedSchema,
+ TableScanContext context) {
+ Expression rowFilter = context.rowFilter();
+ boolean caseSensitive = context.caseSensitive();
+ boolean ignoreResiduals = context.ignoreResiduals();
+
+ LoadingCache<Integer, ManifestEvaluator> evalCache = Caffeine.newBuilder().build(specId -> {
Review Comment:
Do you plan to reuse as-is? If so, maybe, I can move it around in this PR.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r851376908
##########
core/src/main/java/org/apache/iceberg/AllDataFilesTable.java:
##########
@@ -53,47 +48,25 @@ MetadataTableType metadataTableType() {
return MetadataTableType.ALL_DATA_FILES;
}
- public static class AllDataFilesTableScan extends BaseFilesTableScan {
+ public static class AllDataFilesTableScan extends BaseAllFilesTableScan {
- AllDataFilesTableScan(TableOperations ops, Table table, Schema fileSchema) {
- super(ops, table, fileSchema, MetadataTableType.ALL_DATA_FILES);
+ AllDataFilesTableScan(TableOperations ops, Table table, Schema schema) {
+ super(ops, table, schema, MetadataTableType.ALL_DATA_FILES);
}
- private AllDataFilesTableScan(TableOperations ops, Table table, Schema schema, Schema fileSchema,
Review Comment:
I no longer pass both `schema` and `fileSchema` as those are same and can be accessed by `schema()` from the common parent class.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] szehon-ho commented on a diff in pull request #4566: Core: Refactor metadata tables and scans
Posted by GitBox <gi...@apache.org>.
szehon-ho commented on code in PR #4566:
URL: https://github.com/apache/iceberg/pull/4566#discussion_r851444361
##########
core/src/main/java/org/apache/iceberg/AllEntriesTable.java:
##########
@@ -88,34 +83,16 @@ protected TableScan newRefinedScan(TableOperations ops, Table table, Schema sche
}
@Override
- protected String tableType() {
- return MetadataTableType.ALL_ENTRIES.name();
- }
+ protected CloseableIterable<FileScanTask> doPlanFiles() {
+ CloseableIterable<ManifestFile> manifests = allManifests(Snapshot::allManifests);
- @Override
- protected CloseableIterable<FileScanTask> planFiles(
- TableOperations ops, Snapshot snapshot, Expression rowFilter,
- boolean ignoreResiduals, boolean caseSensitive, boolean colStats) {
- CloseableIterable<ManifestFile> manifests = allManifestFiles(
- ops.current().snapshots(), context().planExecutor());
String schemaString = SchemaParser.toJson(schema());
String specString = PartitionSpecParser.toJson(PartitionSpec.unpartitioned());
- Expression filter = ignoreResiduals ? Expressions.alwaysTrue() : rowFilter;
+ Expression filter = shouldIgnoreResiduals() ? Expressions.alwaysTrue() : filter();
Review Comment:
Yea I see, never mind if its too big a change.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org