szehon-ho commented on code in PR #12771:
URL: https://github.com/apache/iceberg/pull/12771#discussion_r2055328971


##########
parquet/src/main/java/org/apache/iceberg/parquet/Parquet.java:
##########
@@ -306,33 +309,35 @@ private WriteBuilder createContextFunc(
       return this;
     }
 
+    // Utility method to get the column path
+    private String getParquetColumnPath(

Review Comment:
   I was wondering, the field Id seems not used, couldnt we just make a map of 
column name to column path?
   
   
   ```    
    Map<String, String> colNameToPaths =
             parquetSchema.getColumns().stream()
                 .filter(col -> col.getPrimitiveType().getId() != null)
                 .collect(
                     Collectors.toMap(
                         col ->  {
                                int fieldId = 
col.getPrimitiveType().getId().intValue();
                                return schema.findColumnName(fieldId);
                           },
                         col -> String.join(".", col.getPath())));
   ```
   
   Worth to double check the logic, i think this would make the subsequent 
logic simpler, if it is correct?



##########
parquet/src/main/java/org/apache/iceberg/parquet/Parquet.java:
##########
@@ -306,33 +309,35 @@ private WriteBuilder createContextFunc(
       return this;
     }
 
+    // Utility method to get the column path
+    private String getParquetColumnPath(

Review Comment:
   would probably need to tweak the code to strengthen null handling for the 
two null cases



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