danielcweeks commented on code in PR #13699:
URL: https://github.com/apache/iceberg/pull/13699#discussion_r2283640724
##########
arrow/src/main/java/org/apache/iceberg/arrow/ArrowSchemaUtil.java:
##########
@@ -51,94 +54,134 @@ private ArrowSchemaUtil() {}
public static Schema convert(final org.apache.iceberg.Schema schema) {
ImmutableList.Builder<Field> fields = ImmutableList.builder();
- for (NestedField f : schema.columns()) {
- fields.add(convert(f));
+ for (NestedField field : schema.columns()) {
Review Comment:
@stevenzwu I had the [same comment
here](https://github.com/apache/iceberg/pull/13699#discussion_r2248978982), but
the issue is around Arrow types and how they handle the enclosing schema (it's
not a struct, just a list of fields). It becomes awkward to convert at the
schema level because it would require the generic type be the arrow `Schema`
which is only constructed after visiting the full schema. Being able to return
`Field` is more consistent with the behavior we want out of visiting and
translating the type.
This is also consistent with patterns we have with converting schema's to
parquet ([example
here](https://github.com/apache/iceberg/blob/018710edd56652e915fe858cba512b3064efcfa9/parquet/src/main/java/org/apache/iceberg/parquet/TypeToMessageType.java#L81))
--
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
