CTTY commented on code in PR #2840:
URL: https://github.com/apache/iceberg-rust/pull/2840#discussion_r3607053882
##########
crates/iceberg/src/arrow/schema.rs:
##########
@@ -164,6 +164,21 @@ pub trait ArrowSchemaVisitor {
/// Called when see a primitive type.
fn primitive(&mut self, p: &DataType) -> Result<Self::T>;
+
+ /// Called when a field carries the `arrow.parquet.variant` extension type.
+ ///
+ /// The default treats the variant as its underlying Arrow struct storage:
it
+ /// re-enters normal traversal, preserving the behavior of visitors that
don't
+ /// special-case variants. A visitor that produces Iceberg types (or
otherwise
+ /// needs variant identity) should override this to fold the struct into a
+ /// single logical variant.
+ ///
+ /// Takes the `&FieldRef` rather than a `&DataType` because the variant
signal
+ /// lives on the field's metadata, not on its data type.
Review Comment:
Makes sense
##########
crates/iceberg/src/arrow/schema.rs:
##########
@@ -544,6 +569,21 @@ impl ArrowSchemaVisitor for ArrowSchemaConverter {
)),
}
}
+
+ fn variant(&mut self, field: &FieldRef) -> Result<Self::T> {
+ // The extension may only sit on struct storage (mirrors
+ // `VariantExtensionType::supports_data_type`).
+ if !matches!(field.data_type(), DataType::Struct(_)) {
Review Comment:
Should we match the nested types here as well (metadata and value)?
--
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]
