Jefffrey commented on code in PR #10647:
URL: https://github.com/apache/arrow-rs/pull/10647#discussion_r3771888946
##########
arrow-ipc/src/convert.rs:
##########
@@ -158,52 +158,76 @@ pub fn schema_to_fb_offset<'a>(
}
/// Convert an IPC Field to Arrow Field
+///
+/// This panics on malformed input; every reader path uses the fallible
+/// conversion instead. kept for backwards compatibility only.
impl From<crate::Field<'_>> for Field {
fn from(field: crate::Field) -> Field {
- let arrow_field = if let Some(dictionary) = field.dictionary() {
- #[allow(deprecated)]
- Field::new_dict(
- field.name().unwrap_or_default(),
- get_data_type(field, true),
- field.nullable(),
- dictionary.id(),
- dictionary.isOrdered(),
- )
- } else {
- Field::new(
- field.name().unwrap_or_default(),
- get_data_type(field, true),
- field.nullable(),
- )
- };
+ try_field_from(field).expect("invalid IPC field")
+ }
+}
- let mut metadata_map = HashMap::default();
- if let Some(list) = field.custom_metadata() {
- for kv in list {
- if let (Some(k), Some(v)) = (kv.key(), kv.value()) {
- metadata_map.insert(k.to_string(), v.to_string());
- }
+/// Fallible conversion of an IPC Field to an Arrow Field.
+fn try_field_from(field: crate::Field) -> Result<Field, ArrowError> {
+ let arrow_field = if let Some(dictionary) = field.dictionary() {
+ #[allow(deprecated)]
+ Field::new_dict(
+ field.name().unwrap_or_default(),
+ get_data_type(field, true)?,
+ field.nullable(),
+ dictionary.id(),
+ dictionary.isOrdered(),
+ )
+ } else {
+ Field::new(
+ field.name().unwrap_or_default(),
+ get_data_type(field, true)?,
+ field.nullable(),
+ )
+ };
+
+ let mut metadata_map = HashMap::default();
+ if let Some(list) = field.custom_metadata() {
+ for kv in list {
+ if let (Some(k), Some(v)) = (kv.key(), kv.value()) {
+ metadata_map.insert(k.to_string(), v.to_string());
}
}
-
- arrow_field.with_metadata(metadata_map)
}
+
+ Ok(arrow_field.with_metadata(metadata_map))
}
/// Deserialize an ipc [`crate::Schema`] from flat buffers to an arrow
[Schema].
+///
+/// Deprecated: this panics on malformed input. Use the fallible
+/// [`try_fb_to_schema`] instead.
+#[deprecated(since = "59.2.0", note = "Use `try_fb_to_schema` instead")]
pub fn fb_to_schema(fb: crate::Schema) -> Schema {
Review Comment:
```suggestion
#[deprecated(since = "60.0.0", note = "Use `try_fb_to_schema` instead")]
pub fn fb_to_schema(fb: crate::Schema) -> Schema {
```
dont need to restate deprecation in the doc, attribute is sufficient
##########
arrow-ipc/src/convert.rs:
##########
@@ -158,52 +158,76 @@ pub fn schema_to_fb_offset<'a>(
}
/// Convert an IPC Field to Arrow Field
+///
+/// This panics on malformed input; every reader path uses the fallible
+/// conversion instead. kept for backwards compatibility only.
impl From<crate::Field<'_>> for Field {
fn from(field: crate::Field) -> Field {
- let arrow_field = if let Some(dictionary) = field.dictionary() {
- #[allow(deprecated)]
- Field::new_dict(
- field.name().unwrap_or_default(),
- get_data_type(field, true),
- field.nullable(),
- dictionary.id(),
- dictionary.isOrdered(),
- )
- } else {
- Field::new(
- field.name().unwrap_or_default(),
- get_data_type(field, true),
- field.nullable(),
- )
- };
+ try_field_from(field).expect("invalid IPC field")
+ }
+}
- let mut metadata_map = HashMap::default();
- if let Some(list) = field.custom_metadata() {
- for kv in list {
- if let (Some(k), Some(v)) = (kv.key(), kv.value()) {
- metadata_map.insert(k.to_string(), v.to_string());
- }
+/// Fallible conversion of an IPC Field to an Arrow Field.
Review Comment:
```suggestion
/// Convert an IPC Field to Arrow Field
```
it being fallible is already evident from the signature (it returns a
result).
--
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]
