stevenzwu commented on code in PR #10735:
URL: https://github.com/apache/iceberg/pull/10735#discussion_r1685644920
##########
core/src/main/java/org/apache/iceberg/BaseEntriesTable.java:
##########
@@ -283,19 +276,28 @@ static class ManifestReadTask extends BaseFileScanTask
implements DataTask {
private final ManifestFile manifest;
private final Map<Integer, PartitionSpec> specsById;
+ ManifestReadTask(Table table, ManifestFile manifest, Schema projection,
Expression filter) {
+ this(table.schema(), table.io(), table.specs(), manifest, projection,
filter);
+ }
+
ManifestReadTask(
- Table table,
Review Comment:
did a bit refactoring. this constructor can be used by the JSON parser. the
other constructor above is used for the previous purpose in the `planFiles`
method above.
##########
core/src/main/java/org/apache/iceberg/BaseEntriesTable.java:
##########
@@ -304,11 +306,6 @@ static class ManifestReadTask extends BaseFileScanTask
implements DataTask {
: new Schema();
}
- @VisibleForTesting
Review Comment:
moved it down with other getters
##########
core/src/main/java/org/apache/iceberg/GenericManifestFile.java:
##########
@@ -105,6 +105,42 @@ public GenericManifestFile(Schema avroSchema) {
this.keyMetadata = null;
}
+ /** Adjust the arg order to avoid conflict with the public constructor below
*/
+ GenericManifestFile(
Review Comment:
used by `ManifestFileParser` introduced in this PR
##########
core/src/main/java/org/apache/iceberg/hadoop/HadoopFileIO.java:
##########
@@ -71,6 +72,13 @@ public HadoopFileIO(Configuration hadoopConf) {
public HadoopFileIO(SerializableSupplier<Configuration> hadoopConf) {
this.hadoopConf = hadoopConf;
+ Map<String, String> props =
Maps.newHashMapWithExpectedSize(hadoopConf.get().size());
Review Comment:
These bugs were discovered during serialization test (don't remember in core
module or flink module)
##########
core/src/main/java/org/apache/iceberg/ScanTaskParser.java:
##########
@@ -30,7 +30,10 @@ public class ScanTaskParser {
private enum TaskType {
FILE_SCAN_TASK("file-scan-task"),
- DATA_TASK("data-task");
+ DATA_TASK("data-task"),
+ FILES_TABLE_TASK("files-table-task"),
+ ALL_MANIFESTS_TABLE_TASK("all-manifests-table-task"),
+ MANIFEST_ENTRIES_TABLE_TASK("manifest-entries-task");
Review Comment:
This is for the `BaseEntriesTable` task, which is the base class for
`ManifestEntriesTable` and `AllEntriesTable`. Don't want to call it
`base-entries-task` as it wouldn't be clear what kind of task.
--
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]
