kbendick commented on a change in pull request #2984: URL: https://github.com/apache/iceberg/pull/2984#discussion_r689881711
########## File path: mr/src/main/java/org/apache/iceberg/mr/mapreduce/IcebergInputFormat.java ########## @@ -141,7 +144,7 @@ // TODO: We do not support residual evaluation for HIVE and PIG in memory data model yet checkResiduals(task); } - splits.add(new IcebergSplit(conf, task, table.io(), table.encryption())); + splits.add(new IcebergSplit(SerializableTable.copyOf(table), conf, task)); Review comment: Nit: It looks like this call to `SerializableTable.copyOf(table)` is happening in a loop. Can we pull it out? ########## File path: core/src/main/java/org/apache/iceberg/MetadataColumns.java ########## @@ -38,6 +38,11 @@ private MetadataColumns() { Integer.MAX_VALUE - 2, "_pos", Types.LongType.get(), "Ordinal position of a row in the source data file"); public static final NestedField IS_DELETED = NestedField.required( Integer.MAX_VALUE - 3, "_deleted", Types.BooleanType.get(), "Whether the row has been deleted"); + public static final NestedField SPEC = NestedField.required( + Integer.MAX_VALUE - 4, "_spec", Types.IntegerType.get(), "Spec ID to which a row belongs to"); + public static final int PARTITION_COLUMN_ID = Integer.MAX_VALUE - 5; + public static final String PARTITION_COLUMN_NAME = "_partition"; + public static final String PARTITION_COLUMN_DOC = "Partition to which a row belongs to"; Review comment: Nit: I see here that this is so that we can have a later call to `Partitioning.partitionType` (and possibly because it's optional). But it took me a minute to see why this one is different. Possibly might consider making it more clear or adding a comment? Even just `// Partition type is not static so must be handled specially` might be good, though I know sometimes we prefer the less comments route due to comment rot etc. Up to you 🙂 -- 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