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

Reply via email to