Fokko commented on code in PR #13445:
URL: https://github.com/apache/iceberg/pull/13445#discussion_r2281035160


##########
spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/data/SparkParquetWriters.java:
##########
@@ -79,6 +84,123 @@ public static <T> ParquetValueWriter<T> 
buildWriter(StructType dfSchema, Message
         ParquetWithSparkSchemaVisitor.visit(dfSchema, type, new 
WriteBuilder(type));
   }
 
+  @SuppressWarnings("unchecked")
+  public static <T> ParquetValueWriter<T> buildWriter(Schema iSchema, 
MessageType type) {

Review Comment:
   @rdblue I went a couple of times back and forth, but I think this is the 
cleanest way.
   
   - We need the Iceberg StructType anyway, to correctly map the fields:
   
https://github.com/apache/iceberg/blob/0b599bc37e39b3893e443393136c7fddc8391d6d/parquet/src/main/java/org/apache/iceberg/parquet/ParquetValueWriters.java#L712-L731
   - The UnknownType is not in the Parquet file, therefore we would need to 
skip over that field, which would alter the logic of the original 
`ParquetWithSparkSchemaVisitor` visitor. The Spark types don't carry FieldIDs, 
so we would need to do this by name 🙀 
   
   The big downside is that we now also compute the Spark types in this 
visitor, which might be expensive.
   
   Let me know what you think!



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to