liurenjie1024 commented on code in PR #277:
URL: https://github.com/apache/iceberg-rust/pull/277#discussion_r1531693970


##########
crates/iceberg/src/arrow.rs:
##########
@@ -106,3 +114,224 @@ impl ArrowReader {
         ProjectionMask::all()
     }
 }
+
+/// The key of column id in the metadata of arrow field.
+pub const COLUMN_ID_META_KEY: &str = "column_id";
+/// The key of doc in the metadata of arrow field.
+pub const DOC: &str = "doc";
+
+struct ToArrowSchemaConverter;
+
+enum ArrowSchemaOrFieldOrType {
+    Schema(ArrowSchema),
+    Field(ArrowFieldRef),
+    Type(ArrowType),
+}
+
+impl SchemaVisitor for ToArrowSchemaConverter {
+    type T = ArrowSchemaOrFieldOrType;
+
+    fn schema(&mut self, _schema: &crate::spec::Schema, value: Self::T) -> 
crate::Result<Self::T> {
+        let struct_type = match value {
+            ArrowSchemaOrFieldOrType::Type(ArrowType::Struct(fields)) => 
fields,
+            _ => unreachable!(),
+        };
+        Ok(ArrowSchemaOrFieldOrType::Schema(ArrowSchema::new(
+            struct_type,
+        )))
+    }
+
+    fn field(
+        &mut self,
+        field: &crate::spec::NestedFieldRef,
+        value: Self::T,
+    ) -> crate::Result<Self::T> {
+        let ty = match value {
+            ArrowSchemaOrFieldOrType::Type(ty) => ty,
+            _ => unreachable!(),
+        };
+        let mut metadata = HashMap::new();
+        metadata.insert(COLUMN_ID_META_KEY.to_string(), field.id.to_string());

Review Comment:
   I think we don't  need this? See python version: 
https://github.com/apache/iceberg-python/blob/afdfa351119090f09d38ef72857d6303e691f5ad/pyiceberg/io/pyarrow.py#L475



##########
crates/iceberg/src/arrow.rs:
##########
@@ -106,3 +114,224 @@ impl ArrowReader {
         ProjectionMask::all()
     }
 }
+
+/// The key of column id in the metadata of arrow field.
+pub const COLUMN_ID_META_KEY: &str = "column_id";
+/// The key of doc in the metadata of arrow field.
+pub const DOC: &str = "doc";
+
+struct ToArrowSchemaConverter;
+
+enum ArrowSchemaOrFieldOrType {
+    Schema(ArrowSchema),
+    Field(ArrowFieldRef),
+    Type(ArrowType),
+}
+
+impl SchemaVisitor for ToArrowSchemaConverter {
+    type T = ArrowSchemaOrFieldOrType;
+
+    fn schema(&mut self, _schema: &crate::spec::Schema, value: Self::T) -> 
crate::Result<Self::T> {

Review Comment:
   Though it's correct, it's better to use concrete type to improve readability



##########
crates/iceberg/src/arrow.rs:
##########
@@ -106,3 +114,224 @@ impl ArrowReader {
         ProjectionMask::all()
     }
 }
+
+/// The key of column id in the metadata of arrow field.
+pub const COLUMN_ID_META_KEY: &str = "column_id";
+/// The key of doc in the metadata of arrow field.
+pub const DOC: &str = "doc";
+
+struct ToArrowSchemaConverter;
+
+enum ArrowSchemaOrFieldOrType {
+    Schema(ArrowSchema),
+    Field(ArrowFieldRef),
+    Type(ArrowType),
+}
+
+impl SchemaVisitor for ToArrowSchemaConverter {
+    type T = ArrowSchemaOrFieldOrType;
+
+    fn schema(&mut self, _schema: &crate::spec::Schema, value: Self::T) -> 
crate::Result<Self::T> {
+        let struct_type = match value {
+            ArrowSchemaOrFieldOrType::Type(ArrowType::Struct(fields)) => 
fields,
+            _ => unreachable!(),
+        };
+        Ok(ArrowSchemaOrFieldOrType::Schema(ArrowSchema::new(
+            struct_type,
+        )))
+    }
+
+    fn field(
+        &mut self,
+        field: &crate::spec::NestedFieldRef,
+        value: Self::T,
+    ) -> crate::Result<Self::T> {
+        let ty = match value {
+            ArrowSchemaOrFieldOrType::Type(ty) => ty,
+            _ => unreachable!(),
+        };
+        let mut metadata = HashMap::new();
+        metadata.insert(COLUMN_ID_META_KEY.to_string(), field.id.to_string());
+        metadata.insert(PARQUET_FIELD_ID_META_KEY.to_string(), 
field.id.to_string());
+        if let Some(doc) = &field.doc {
+            metadata.insert(DOC.to_string(), doc.clone());
+        }
+        Ok(ArrowSchemaOrFieldOrType::Field(
+            ArrowField::new(field.name.clone(), ty, !field.required)
+                .with_metadata(metadata)
+                .into(),
+        ))
+    }
+
+    fn r#struct(
+        &mut self,
+        _: &crate::spec::StructType,
+        results: Vec<Self::T>,
+    ) -> crate::Result<Self::T> {
+        let fields = results
+            .into_iter()
+            .map(|result| match result {
+                ArrowSchemaOrFieldOrType::Field(field) => field,
+                _ => unreachable!(),
+            })
+            .collect();
+        Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::Struct(fields)))
+    }
+
+    fn list(&mut self, list: &crate::spec::ListType, value: Self::T) -> 
crate::Result<Self::T> {
+        let field = match self.field(&list.element_field, value)? {
+            ArrowSchemaOrFieldOrType::Field(field) => field,
+            _ => unreachable!(),
+        };
+        Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::List(field)))
+    }
+
+    fn map(
+        &mut self,
+        map: &crate::spec::MapType,
+        key_value: Self::T,
+        value: Self::T,
+    ) -> crate::Result<Self::T> {
+        let key_field = match self.field(&map.key_field, key_value)? {

Review Comment:
   This is weird, why not just use `key_value`, `value` ?



##########
crates/iceberg/src/arrow.rs:
##########
@@ -106,3 +114,224 @@ impl ArrowReader {
         ProjectionMask::all()
     }
 }
+
+/// The key of column id in the metadata of arrow field.
+pub const COLUMN_ID_META_KEY: &str = "column_id";
+/// The key of doc in the metadata of arrow field.
+pub const DOC: &str = "doc";
+
+struct ToArrowSchemaConverter;
+
+enum ArrowSchemaOrFieldOrType {
+    Schema(ArrowSchema),
+    Field(ArrowFieldRef),
+    Type(ArrowType),
+}
+
+impl SchemaVisitor for ToArrowSchemaConverter {
+    type T = ArrowSchemaOrFieldOrType;
+
+    fn schema(&mut self, _schema: &crate::spec::Schema, value: Self::T) -> 
crate::Result<Self::T> {
+        let struct_type = match value {
+            ArrowSchemaOrFieldOrType::Type(ArrowType::Struct(fields)) => 
fields,
+            _ => unreachable!(),
+        };
+        Ok(ArrowSchemaOrFieldOrType::Schema(ArrowSchema::new(
+            struct_type,
+        )))
+    }
+
+    fn field(
+        &mut self,
+        field: &crate::spec::NestedFieldRef,
+        value: Self::T,
+    ) -> crate::Result<Self::T> {
+        let ty = match value {
+            ArrowSchemaOrFieldOrType::Type(ty) => ty,
+            _ => unreachable!(),
+        };
+        let mut metadata = HashMap::new();
+        metadata.insert(COLUMN_ID_META_KEY.to_string(), field.id.to_string());
+        metadata.insert(PARQUET_FIELD_ID_META_KEY.to_string(), 
field.id.to_string());
+        if let Some(doc) = &field.doc {
+            metadata.insert(DOC.to_string(), doc.clone());
+        }
+        Ok(ArrowSchemaOrFieldOrType::Field(
+            ArrowField::new(field.name.clone(), ty, !field.required)
+                .with_metadata(metadata)
+                .into(),
+        ))
+    }
+
+    fn r#struct(
+        &mut self,
+        _: &crate::spec::StructType,
+        results: Vec<Self::T>,
+    ) -> crate::Result<Self::T> {
+        let fields = results
+            .into_iter()
+            .map(|result| match result {
+                ArrowSchemaOrFieldOrType::Field(field) => field,
+                _ => unreachable!(),
+            })
+            .collect();
+        Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::Struct(fields)))
+    }
+
+    fn list(&mut self, list: &crate::spec::ListType, value: Self::T) -> 
crate::Result<Self::T> {
+        let field = match self.field(&list.element_field, value)? {
+            ArrowSchemaOrFieldOrType::Field(field) => field,
+            _ => unreachable!(),
+        };
+        Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::List(field)))
+    }
+
+    fn map(
+        &mut self,
+        map: &crate::spec::MapType,
+        key_value: Self::T,
+        value: Self::T,
+    ) -> crate::Result<Self::T> {
+        let key_field = match self.field(&map.key_field, key_value)? {
+            ArrowSchemaOrFieldOrType::Field(field) => field,
+            _ => unreachable!(),
+        };
+        let value_field = match self.field(&map.value_field, value)? {
+            ArrowSchemaOrFieldOrType::Field(field) => field,
+            _ => unreachable!(),
+        };
+        let field = ArrowField::new(
+            "entries",
+            ArrowType::Struct(vec![key_field, value_field].into()),
+            map.value_field.required,
+        );
+
+        Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::Map(
+            field.into(),
+            false,
+        )))
+    }
+
+    fn primitive(&mut self, p: &crate::spec::PrimitiveType) -> 
crate::Result<Self::T> {
+        match p {
+            crate::spec::PrimitiveType::Boolean => {
+                Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::Boolean))
+            }
+            crate::spec::PrimitiveType::Int => 
Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::Int32)),
+            crate::spec::PrimitiveType::Long => {
+                Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::Int64))
+            }
+            crate::spec::PrimitiveType::Float => {
+                Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::Float32))
+            }
+            crate::spec::PrimitiveType::Double => {
+                Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::Float64))
+            }
+            crate::spec::PrimitiveType::Decimal { precision, scale } => {
+                Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::Decimal128(
+                    TryInto::try_into(*precision).map_err(|err| {

Review Comment:
   How about using [this 
method](https://docs.rs/arrow/latest/arrow/datatypes/fn.validate_decimal_precision_and_scale.html)
 to do the check?



##########
crates/iceberg/src/arrow.rs:
##########
@@ -106,3 +114,224 @@ impl ArrowReader {
         ProjectionMask::all()
     }
 }
+
+/// The key of column id in the metadata of arrow field.
+pub const COLUMN_ID_META_KEY: &str = "column_id";
+/// The key of doc in the metadata of arrow field.
+pub const DOC: &str = "doc";
+
+struct ToArrowSchemaConverter;
+
+enum ArrowSchemaOrFieldOrType {
+    Schema(ArrowSchema),
+    Field(ArrowFieldRef),
+    Type(ArrowType),
+}
+
+impl SchemaVisitor for ToArrowSchemaConverter {
+    type T = ArrowSchemaOrFieldOrType;
+
+    fn schema(&mut self, _schema: &crate::spec::Schema, value: Self::T) -> 
crate::Result<Self::T> {
+        let struct_type = match value {
+            ArrowSchemaOrFieldOrType::Type(ArrowType::Struct(fields)) => 
fields,
+            _ => unreachable!(),
+        };
+        Ok(ArrowSchemaOrFieldOrType::Schema(ArrowSchema::new(
+            struct_type,
+        )))
+    }
+
+    fn field(
+        &mut self,
+        field: &crate::spec::NestedFieldRef,
+        value: Self::T,
+    ) -> crate::Result<Self::T> {
+        let ty = match value {
+            ArrowSchemaOrFieldOrType::Type(ty) => ty,
+            _ => unreachable!(),
+        };
+        let mut metadata = HashMap::new();
+        metadata.insert(COLUMN_ID_META_KEY.to_string(), field.id.to_string());
+        metadata.insert(PARQUET_FIELD_ID_META_KEY.to_string(), 
field.id.to_string());
+        if let Some(doc) = &field.doc {
+            metadata.insert(DOC.to_string(), doc.clone());
+        }
+        Ok(ArrowSchemaOrFieldOrType::Field(
+            ArrowField::new(field.name.clone(), ty, !field.required)
+                .with_metadata(metadata)
+                .into(),
+        ))
+    }
+
+    fn r#struct(
+        &mut self,
+        _: &crate::spec::StructType,
+        results: Vec<Self::T>,
+    ) -> crate::Result<Self::T> {
+        let fields = results
+            .into_iter()
+            .map(|result| match result {
+                ArrowSchemaOrFieldOrType::Field(field) => field,
+                _ => unreachable!(),
+            })
+            .collect();
+        Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::Struct(fields)))
+    }
+
+    fn list(&mut self, list: &crate::spec::ListType, value: Self::T) -> 
crate::Result<Self::T> {
+        let field = match self.field(&list.element_field, value)? {
+            ArrowSchemaOrFieldOrType::Field(field) => field,
+            _ => unreachable!(),
+        };
+        Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::List(field)))
+    }
+
+    fn map(
+        &mut self,
+        map: &crate::spec::MapType,
+        key_value: Self::T,
+        value: Self::T,
+    ) -> crate::Result<Self::T> {
+        let key_field = match self.field(&map.key_field, key_value)? {
+            ArrowSchemaOrFieldOrType::Field(field) => field,
+            _ => unreachable!(),
+        };
+        let value_field = match self.field(&map.value_field, value)? {
+            ArrowSchemaOrFieldOrType::Field(field) => field,
+            _ => unreachable!(),
+        };
+        let field = ArrowField::new(
+            "entries",
+            ArrowType::Struct(vec![key_field, value_field].into()),
+            map.value_field.required,
+        );
+
+        Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::Map(
+            field.into(),
+            false,
+        )))
+    }
+
+    fn primitive(&mut self, p: &crate::spec::PrimitiveType) -> 
crate::Result<Self::T> {
+        match p {
+            crate::spec::PrimitiveType::Boolean => {
+                Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::Boolean))
+            }
+            crate::spec::PrimitiveType::Int => 
Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::Int32)),
+            crate::spec::PrimitiveType::Long => {
+                Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::Int64))
+            }
+            crate::spec::PrimitiveType::Float => {
+                Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::Float32))
+            }
+            crate::spec::PrimitiveType::Double => {
+                Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::Float64))
+            }
+            crate::spec::PrimitiveType::Decimal { precision, scale } => {
+                Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::Decimal128(
+                    TryInto::try_into(*precision).map_err(|err| {
+                        Error::new(
+                            crate::ErrorKind::DataInvalid,
+                            "incompatible precision for decimal type convert",
+                        )
+                        .with_source(err)
+                    })?,
+                    TryInto::try_into(*scale).map_err(|err| {
+                        Error::new(
+                            crate::ErrorKind::DataInvalid,
+                            "incompatible scale for decimal type convert",
+                        )
+                        .with_source(err)
+                    })?,
+                )))
+            }
+            crate::spec::PrimitiveType::Date => {
+                Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::Date32))
+            }
+            crate::spec::PrimitiveType::Time => 
Ok(ArrowSchemaOrFieldOrType::Type(
+                ArrowType::Time32(TimeUnit::Microsecond),
+            )),
+            crate::spec::PrimitiveType::Timestamp => 
Ok(ArrowSchemaOrFieldOrType::Type(
+                ArrowType::Timestamp(TimeUnit::Microsecond, None),
+            )),
+            crate::spec::PrimitiveType::Timestamptz => 
Ok(ArrowSchemaOrFieldOrType::Type(
+                // Timestampz always stored as UTC
+                ArrowType::Timestamp(TimeUnit::Microsecond, 
Some("+00:00".into())),
+            )),
+            crate::spec::PrimitiveType::String => {
+                Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::Utf8))
+            }
+            crate::spec::PrimitiveType::Uuid => 
Ok(ArrowSchemaOrFieldOrType::Type(
+                ArrowType::FixedSizeBinary(16),
+            )),
+            crate::spec::PrimitiveType::Fixed(len) => 
Ok(ArrowSchemaOrFieldOrType::Type(
+                len.to_i32()
+                    .map(ArrowType::FixedSizeBinary)
+                    .unwrap_or(ArrowType::LargeBinary),
+            )),
+            crate::spec::PrimitiveType::Binary => {
+                Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::LargeBinary))
+            }
+        }
+    }
+}
+
+pub(crate) fn schema_to_arrow_schema(schema: &crate::spec::Schema) -> 
crate::Result<ArrowSchema> {

Review Comment:
   This maybe useful, how about make it `pub`?



##########
crates/iceberg/src/arrow.rs:
##########
@@ -106,3 +114,224 @@ impl ArrowReader {
         ProjectionMask::all()
     }
 }
+
+/// The key of column id in the metadata of arrow field.
+pub const COLUMN_ID_META_KEY: &str = "column_id";

Review Comment:
   This is incorrect, see 
https://github.com/apache/iceberg-python/blob/afdfa351119090f09d38ef72857d6303e691f5ad/pyiceberg/io/pyarrow.py#L168



##########
crates/iceberg/src/arrow.rs:
##########
@@ -106,3 +114,224 @@ impl ArrowReader {
         ProjectionMask::all()
     }
 }
+
+/// The key of column id in the metadata of arrow field.
+pub const COLUMN_ID_META_KEY: &str = "column_id";
+/// The key of doc in the metadata of arrow field.
+pub const DOC: &str = "doc";
+
+struct ToArrowSchemaConverter;
+
+enum ArrowSchemaOrFieldOrType {
+    Schema(ArrowSchema),
+    Field(ArrowFieldRef),
+    Type(ArrowType),
+}
+
+impl SchemaVisitor for ToArrowSchemaConverter {
+    type T = ArrowSchemaOrFieldOrType;
+
+    fn schema(&mut self, _schema: &crate::spec::Schema, value: Self::T) -> 
crate::Result<Self::T> {
+        let struct_type = match value {
+            ArrowSchemaOrFieldOrType::Type(ArrowType::Struct(fields)) => 
fields,
+            _ => unreachable!(),
+        };
+        Ok(ArrowSchemaOrFieldOrType::Schema(ArrowSchema::new(
+            struct_type,
+        )))
+    }
+
+    fn field(
+        &mut self,
+        field: &crate::spec::NestedFieldRef,
+        value: Self::T,
+    ) -> crate::Result<Self::T> {
+        let ty = match value {
+            ArrowSchemaOrFieldOrType::Type(ty) => ty,
+            _ => unreachable!(),
+        };
+        let mut metadata = HashMap::new();
+        metadata.insert(COLUMN_ID_META_KEY.to_string(), field.id.to_string());
+        metadata.insert(PARQUET_FIELD_ID_META_KEY.to_string(), 
field.id.to_string());
+        if let Some(doc) = &field.doc {
+            metadata.insert(DOC.to_string(), doc.clone());
+        }
+        Ok(ArrowSchemaOrFieldOrType::Field(
+            ArrowField::new(field.name.clone(), ty, !field.required)
+                .with_metadata(metadata)
+                .into(),
+        ))
+    }
+
+    fn r#struct(
+        &mut self,
+        _: &crate::spec::StructType,
+        results: Vec<Self::T>,
+    ) -> crate::Result<Self::T> {
+        let fields = results
+            .into_iter()
+            .map(|result| match result {
+                ArrowSchemaOrFieldOrType::Field(field) => field,
+                _ => unreachable!(),
+            })
+            .collect();
+        Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::Struct(fields)))
+    }
+
+    fn list(&mut self, list: &crate::spec::ListType, value: Self::T) -> 
crate::Result<Self::T> {
+        let field = match self.field(&list.element_field, value)? {
+            ArrowSchemaOrFieldOrType::Field(field) => field,
+            _ => unreachable!(),
+        };
+        Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::List(field)))
+    }
+
+    fn map(
+        &mut self,
+        map: &crate::spec::MapType,
+        key_value: Self::T,
+        value: Self::T,
+    ) -> crate::Result<Self::T> {
+        let key_field = match self.field(&map.key_field, key_value)? {
+            ArrowSchemaOrFieldOrType::Field(field) => field,
+            _ => unreachable!(),
+        };
+        let value_field = match self.field(&map.value_field, value)? {
+            ArrowSchemaOrFieldOrType::Field(field) => field,
+            _ => unreachable!(),
+        };
+        let field = ArrowField::new(
+            "entries",
+            ArrowType::Struct(vec![key_field, value_field].into()),
+            map.value_field.required,
+        );
+
+        Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::Map(
+            field.into(),
+            false,
+        )))
+    }
+
+    fn primitive(&mut self, p: &crate::spec::PrimitiveType) -> 
crate::Result<Self::T> {
+        match p {
+            crate::spec::PrimitiveType::Boolean => {
+                Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::Boolean))
+            }
+            crate::spec::PrimitiveType::Int => 
Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::Int32)),
+            crate::spec::PrimitiveType::Long => {
+                Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::Int64))
+            }
+            crate::spec::PrimitiveType::Float => {
+                Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::Float32))
+            }
+            crate::spec::PrimitiveType::Double => {
+                Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::Float64))
+            }
+            crate::spec::PrimitiveType::Decimal { precision, scale } => {
+                Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::Decimal128(
+                    TryInto::try_into(*precision).map_err(|err| {
+                        Error::new(
+                            crate::ErrorKind::DataInvalid,
+                            "incompatible precision for decimal type convert",
+                        )
+                        .with_source(err)
+                    })?,
+                    TryInto::try_into(*scale).map_err(|err| {
+                        Error::new(
+                            crate::ErrorKind::DataInvalid,
+                            "incompatible scale for decimal type convert",
+                        )
+                        .with_source(err)
+                    })?,
+                )))
+            }
+            crate::spec::PrimitiveType::Date => {
+                Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::Date32))
+            }
+            crate::spec::PrimitiveType::Time => 
Ok(ArrowSchemaOrFieldOrType::Type(
+                ArrowType::Time32(TimeUnit::Microsecond),
+            )),
+            crate::spec::PrimitiveType::Timestamp => 
Ok(ArrowSchemaOrFieldOrType::Type(
+                ArrowType::Timestamp(TimeUnit::Microsecond, None),
+            )),
+            crate::spec::PrimitiveType::Timestamptz => 
Ok(ArrowSchemaOrFieldOrType::Type(
+                // Timestampz always stored as UTC
+                ArrowType::Timestamp(TimeUnit::Microsecond, 
Some("+00:00".into())),
+            )),
+            crate::spec::PrimitiveType::String => {
+                Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::Utf8))
+            }
+            crate::spec::PrimitiveType::Uuid => 
Ok(ArrowSchemaOrFieldOrType::Type(
+                ArrowType::FixedSizeBinary(16),
+            )),
+            crate::spec::PrimitiveType::Fixed(len) => 
Ok(ArrowSchemaOrFieldOrType::Type(
+                len.to_i32()
+                    .map(ArrowType::FixedSizeBinary)
+                    .unwrap_or(ArrowType::LargeBinary),
+            )),
+            crate::spec::PrimitiveType::Binary => {
+                Ok(ArrowSchemaOrFieldOrType::Type(ArrowType::LargeBinary))
+            }
+        }
+    }
+}
+
+pub(crate) fn schema_to_arrow_schema(schema: &crate::spec::Schema) -> 
crate::Result<ArrowSchema> {
+    let mut converter = ToArrowSchemaConverter;
+    match visit_schema(schema, &mut converter)? {
+        ArrowSchemaOrFieldOrType::Schema(schema) => Ok(schema),
+        _ => unreachable!(),
+    }
+}
+
+impl TryFrom<&crate::spec::Schema> for ArrowSchema {
+    type Error = Error;
+
+    fn try_from(schema: &crate::spec::Schema) -> crate::Result<Self> {
+        schema_to_arrow_schema(schema)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::spec::{NestedField, PrimitiveType, Schema, Type};
+
+    use super::*;
+
+    #[test]
+    fn test_try_into_arrow_schema() {

Review Comment:
   It's better to have more types covered.



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