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 2021/01/20 11:15:14 UTC

[GitHub] [iceberg] pvary opened a new pull request #2121: Hive: New test case for writing to the table with projection and column specification

pvary opened a new pull request #2121:
URL: https://github.com/apache/iceberg/pull/2121


   During the test identified an issue:
   - `IcebergRecordObjectInspector.getStructFieldData()` should use the names to identify the columns and not the position when there is a projection (might be a real issue only in Tez execution, but I think it is good to have the fix here too)
   - Also realized that Vectorized execution will not work with the current implementation of IcebergInput/IcebergOutputFormat. Raised: #2120 for that
   
   


----------------------------------------------------------------
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.

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] pvary commented on pull request #2121: Hive: New test case for writing to the table with projection and column specification

Posted by GitBox <gi...@apache.org>.
pvary commented on pull request #2121:
URL: https://github.com/apache/iceberg/pull/2121#issuecomment-772341638


   Thanks for the review and the merge @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.

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] pvary commented on pull request #2121: Hive: New test case for writing to the table with projection and column specification

Posted by GitBox <gi...@apache.org>.
pvary commented on pull request #2121:
URL: https://github.com/apache/iceberg/pull/2121#issuecomment-767004823


   @marton-bod, @lcspinter: Could you please review?
   
   Thanks,
   Peter 


----------------------------------------------------------------
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.

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] pvary commented on a change in pull request #2121: Hive: New test case for writing to the table with projection and column specification

Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #2121:
URL: https://github.com/apache/iceberg/pull/2121#discussion_r568173029



##########
File path: mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerWithEngine.java
##########
@@ -335,6 +335,25 @@ public void testInsertFromSelectWithOrderBy() throws IOException {
     HiveIcebergTestUtils.validateData(table, records, 0);
   }
 
+  @Test
+  public void testInsertFromSelectWithProjection() throws IOException {
+    Assume.assumeTrue("Tez write is not implemented yet", executionEngine.equals("mr"));
+
+    Table table = testTables.createTable(shell, "customers", HiveIcebergStorageHandlerTestUtils.CUSTOMER_SCHEMA,
+        fileFormat, ImmutableList.of());
+    testTables.createTable(shell, "orders", ORDER_SCHEMA, fileFormat, ORDER_RECORDS);
+
+    shell.executeStatement(
+        "INSERT INTO customers (customer_id, last_name) SELECT distinct(customer_id), 'test' FROM orders");
+
+    List<Record> expected = TestHelper.RecordsBuilder.newInstance(HiveIcebergStorageHandlerTestUtils.CUSTOMER_SCHEMA)
+        .add(0L, null, "test")
+        .add(1L, null, "test")
+        .build();
+
+    HiveIcebergTestUtils.validateData(table, new ArrayList<>(expected), 0);

Review comment:
       `HiveIcebergTestUtils.validateData` uses in place sort to compare the actual and the expected records. This is needed of other uses of the method, so I just recreated a new list for the test case




----------------------------------------------------------------
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.

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] pvary commented on pull request #2121: Hive: New test case for writing to the table with projection and column specification

Posted by GitBox <gi...@apache.org>.
pvary commented on pull request #2121:
URL: https://github.com/apache/iceberg/pull/2121#issuecomment-767004823


   @marton-bod, @lcspinter: Could you please review?
   
   Thanks,
   Peter 


----------------------------------------------------------------
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.

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] pvary commented on a change in pull request #2121: Hive: New test case for writing to the table with projection and column specification

Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #2121:
URL: https://github.com/apache/iceberg/pull/2121#discussion_r568173699



##########
File path: mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveShell.java
##########
@@ -186,6 +186,9 @@ private HiveConf initializeConf() {
     // Tez configuration
     hiveConf.setBoolean("tez.local.mode", true);
 
+    // Disable vectorization for HiveIcebergInputFormat
+    hiveConf.setBoolVar(HiveConf.ConfVars.HIVE_VECTORIZATION_ENABLED, false);

Review comment:
       I think it is important, but no hard data on yet. Will work on read/write performance in the near future, so  would guess that vectorization will be something we check too.




----------------------------------------------------------------
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.

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] pvary commented on a change in pull request #2121: Hive: New test case for writing to the table with projection and column specification

Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #2121:
URL: https://github.com/apache/iceberg/pull/2121#discussion_r568486900



##########
File path: mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerWithEngine.java
##########
@@ -335,6 +335,25 @@ public void testInsertFromSelectWithOrderBy() throws IOException {
     HiveIcebergTestUtils.validateData(table, records, 0);
   }
 
+  @Test
+  public void testInsertFromSelectWithProjection() throws IOException {
+    Assume.assumeTrue("Tez write is not implemented yet", executionEngine.equals("mr"));
+
+    Table table = testTables.createTable(shell, "customers", HiveIcebergStorageHandlerTestUtils.CUSTOMER_SCHEMA,
+        fileFormat, ImmutableList.of());
+    testTables.createTable(shell, "orders", ORDER_SCHEMA, fileFormat, ORDER_RECORDS);
+
+    shell.executeStatement(
+        "INSERT INTO customers (customer_id, last_name) SELECT distinct(customer_id), 'test' FROM orders");
+
+    List<Record> expected = TestHelper.RecordsBuilder.newInstance(HiveIcebergStorageHandlerTestUtils.CUSTOMER_SCHEMA)
+        .add(0L, null, "test")
+        .add(1L, null, "test")
+        .build();
+
+    HiveIcebergTestUtils.validateData(table, new ArrayList<>(expected), 0);

Review comment:
       Good point. Fixed here, and all the relevant other 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.

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 change in pull request #2121: Hive: New test case for writing to the table with projection and column specification

Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #2121:
URL: https://github.com/apache/iceberg/pull/2121#discussion_r568171424



##########
File path: mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerWithEngine.java
##########
@@ -335,6 +335,25 @@ public void testInsertFromSelectWithOrderBy() throws IOException {
     HiveIcebergTestUtils.validateData(table, records, 0);
   }
 
+  @Test
+  public void testInsertFromSelectWithProjection() throws IOException {
+    Assume.assumeTrue("Tez write is not implemented yet", executionEngine.equals("mr"));
+
+    Table table = testTables.createTable(shell, "customers", HiveIcebergStorageHandlerTestUtils.CUSTOMER_SCHEMA,
+        fileFormat, ImmutableList.of());
+    testTables.createTable(shell, "orders", ORDER_SCHEMA, fileFormat, ORDER_RECORDS);
+
+    shell.executeStatement(
+        "INSERT INTO customers (customer_id, last_name) SELECT distinct(customer_id), 'test' FROM orders");
+
+    List<Record> expected = TestHelper.RecordsBuilder.newInstance(HiveIcebergStorageHandlerTestUtils.CUSTOMER_SCHEMA)
+        .add(0L, null, "test")
+        .add(1L, null, "test")
+        .build();
+
+    HiveIcebergTestUtils.validateData(table, new ArrayList<>(expected), 0);

Review comment:
       Why does this create a new `ArrayList` from the expected records?




----------------------------------------------------------------
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.

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 pull request #2121: Hive: New test case for writing to the table with projection and column specification

Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #2121:
URL: https://github.com/apache/iceberg/pull/2121#issuecomment-772103041


   Thanks, @pvary!


----------------------------------------------------------------
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.

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] lcspinter commented on a change in pull request #2121: Hive: New test case for writing to the table with projection and column specification

Posted by GitBox <gi...@apache.org>.
lcspinter commented on a change in pull request #2121:
URL: https://github.com/apache/iceberg/pull/2121#discussion_r563961054



##########
File path: mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerWithEngine.java
##########
@@ -335,6 +335,26 @@ public void testInsertFromSelectWithOrderBy() throws IOException {
     HiveIcebergTestUtils.validateData(table, records, 0);
   }
 
+  @Test
+  public void testInsertFromSelectWithProjection() throws IOException {
+    Assume.assumeTrue("Tez write is not implemented yet", executionEngine.equals("mr"));
+
+    Table table = testTables.createTable(shell, "customers", HiveIcebergStorageHandlerTestUtils.CUSTOMER_SCHEMA,
+        fileFormat, ImmutableList.of());
+    testTables.createTable(shell, "orders", ORDER_SCHEMA, fileFormat, ORDER_RECORDS);
+
+    shell.setHiveSessionValue("hive.vectorized.execution.enabled", false);

Review comment:
       Vectorization was already disabled in TestHiveShell.initializeConf? Do we need to disable it again?




----------------------------------------------------------------
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.

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] pvary commented on a change in pull request #2121: Hive: New test case for writing to the table with projection and column specification

Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #2121:
URL: https://github.com/apache/iceberg/pull/2121#discussion_r568486900



##########
File path: mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerWithEngine.java
##########
@@ -335,6 +335,25 @@ public void testInsertFromSelectWithOrderBy() throws IOException {
     HiveIcebergTestUtils.validateData(table, records, 0);
   }
 
+  @Test
+  public void testInsertFromSelectWithProjection() throws IOException {
+    Assume.assumeTrue("Tez write is not implemented yet", executionEngine.equals("mr"));
+
+    Table table = testTables.createTable(shell, "customers", HiveIcebergStorageHandlerTestUtils.CUSTOMER_SCHEMA,
+        fileFormat, ImmutableList.of());
+    testTables.createTable(shell, "orders", ORDER_SCHEMA, fileFormat, ORDER_RECORDS);
+
+    shell.executeStatement(
+        "INSERT INTO customers (customer_id, last_name) SELECT distinct(customer_id), 'test' FROM orders");
+
+    List<Record> expected = TestHelper.RecordsBuilder.newInstance(HiveIcebergStorageHandlerTestUtils.CUSTOMER_SCHEMA)
+        .add(0L, null, "test")
+        .add(1L, null, "test")
+        .build();
+
+    HiveIcebergTestUtils.validateData(table, new ArrayList<>(expected), 0);

Review comment:
       Good point. Fixed here, and all the relevant other 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.

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] pvary commented on a change in pull request #2121: Hive: New test case for writing to the table with projection and column specification

Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #2121:
URL: https://github.com/apache/iceberg/pull/2121#discussion_r564313461



##########
File path: mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerWithEngine.java
##########
@@ -335,6 +335,26 @@ public void testInsertFromSelectWithOrderBy() throws IOException {
     HiveIcebergTestUtils.validateData(table, records, 0);
   }
 
+  @Test
+  public void testInsertFromSelectWithProjection() throws IOException {
+    Assume.assumeTrue("Tez write is not implemented yet", executionEngine.equals("mr"));
+
+    Table table = testTables.createTable(shell, "customers", HiveIcebergStorageHandlerTestUtils.CUSTOMER_SCHEMA,
+        fileFormat, ImmutableList.of());
+    testTables.createTable(shell, "orders", ORDER_SCHEMA, fileFormat, ORDER_RECORDS);
+
+    shell.setHiveSessionValue("hive.vectorized.execution.enabled", false);

Review comment:
       Yeah, I have missed this.
   Thanks for catching!




----------------------------------------------------------------
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.

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] pvary commented on pull request #2121: Hive: New test case for writing to the table with projection and column specification

Posted by GitBox <gi...@apache.org>.
pvary commented on pull request #2121:
URL: https://github.com/apache/iceberg/pull/2121#issuecomment-770873937


   @rdblue: Could you please take a look at this PR? Only adds a single test case which failed for tez writes. Nothing fancy, but good to have a test case for MR as well.
   
   Thanks,
   Peter


----------------------------------------------------------------
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.

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 merged pull request #2121: Hive: New test case for writing to the table with projection and column specification

Posted by GitBox <gi...@apache.org>.
rdblue merged pull request #2121:
URL: https://github.com/apache/iceberg/pull/2121


   


----------------------------------------------------------------
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.

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] lcspinter commented on a change in pull request #2121: Hive: New test case for writing to the table with projection and column specification

Posted by GitBox <gi...@apache.org>.
lcspinter commented on a change in pull request #2121:
URL: https://github.com/apache/iceberg/pull/2121#discussion_r563961054



##########
File path: mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerWithEngine.java
##########
@@ -335,6 +335,26 @@ public void testInsertFromSelectWithOrderBy() throws IOException {
     HiveIcebergTestUtils.validateData(table, records, 0);
   }
 
+  @Test
+  public void testInsertFromSelectWithProjection() throws IOException {
+    Assume.assumeTrue("Tez write is not implemented yet", executionEngine.equals("mr"));
+
+    Table table = testTables.createTable(shell, "customers", HiveIcebergStorageHandlerTestUtils.CUSTOMER_SCHEMA,
+        fileFormat, ImmutableList.of());
+    testTables.createTable(shell, "orders", ORDER_SCHEMA, fileFormat, ORDER_RECORDS);
+
+    shell.setHiveSessionValue("hive.vectorized.execution.enabled", false);

Review comment:
       Vectorization was already disabled in TestHiveShell.initializeConf? Do we need to disable it again?




----------------------------------------------------------------
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.

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 change in pull request #2121: Hive: New test case for writing to the table with projection and column specification

Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #2121:
URL: https://github.com/apache/iceberg/pull/2121#discussion_r568170358



##########
File path: mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveShell.java
##########
@@ -186,6 +186,9 @@ private HiveConf initializeConf() {
     // Tez configuration
     hiveConf.setBoolean("tez.local.mode", true);
 
+    // Disable vectorization for HiveIcebergInputFormat
+    hiveConf.setBoolVar(HiveConf.ConfVars.HIVE_VECTORIZATION_ENABLED, false);

Review comment:
       I think this is fine, but I'm curious: do we plan to add vectorized writes? Or is this not something that we think is important?




----------------------------------------------------------------
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.

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 pull request #2121: Hive: New test case for writing to the table with projection and column specification

Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #2121:
URL: https://github.com/apache/iceberg/pull/2121#issuecomment-772103041


   Thanks, @pvary!


----------------------------------------------------------------
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.

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 change in pull request #2121: Hive: New test case for writing to the table with projection and column specification

Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #2121:
URL: https://github.com/apache/iceberg/pull/2121#discussion_r568184958



##########
File path: mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerWithEngine.java
##########
@@ -335,6 +335,25 @@ public void testInsertFromSelectWithOrderBy() throws IOException {
     HiveIcebergTestUtils.validateData(table, records, 0);
   }
 
+  @Test
+  public void testInsertFromSelectWithProjection() throws IOException {
+    Assume.assumeTrue("Tez write is not implemented yet", executionEngine.equals("mr"));
+
+    Table table = testTables.createTable(shell, "customers", HiveIcebergStorageHandlerTestUtils.CUSTOMER_SCHEMA,
+        fileFormat, ImmutableList.of());
+    testTables.createTable(shell, "orders", ORDER_SCHEMA, fileFormat, ORDER_RECORDS);
+
+    shell.executeStatement(
+        "INSERT INTO customers (customer_id, last_name) SELECT distinct(customer_id), 'test' FROM orders");
+
+    List<Record> expected = TestHelper.RecordsBuilder.newInstance(HiveIcebergStorageHandlerTestUtils.CUSTOMER_SCHEMA)
+        .add(0L, null, "test")
+        .add(1L, null, "test")
+        .build();
+
+    HiveIcebergTestUtils.validateData(table, new ArrayList<>(expected), 0);

Review comment:
       Doesn't this leak the concern about sorting outside of the method? I would rather have the array copy happen defensively inside the method.




----------------------------------------------------------------
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.

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 merged pull request #2121: Hive: New test case for writing to the table with projection and column specification

Posted by GitBox <gi...@apache.org>.
rdblue merged pull request #2121:
URL: https://github.com/apache/iceberg/pull/2121


   


----------------------------------------------------------------
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.

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 pull request #2121: Hive: New test case for writing to the table with projection and column specification

Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #2121:
URL: https://github.com/apache/iceberg/pull/2121#issuecomment-771190634


   Overall, this looks fine. One minor question about why it creates an extra list. Otherwise +1.


----------------------------------------------------------------
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.

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