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