refactor: remove unwraps

crates/iceberg/src/avro/schema.rs

@@ -48,7 +48,10 @@ impl SchemaVisitor for SchemaToAvroSchema {
     type T = AvroSchemaOrField;
 
     fn schema(&mut self, _schema: &Schema, value: AvroSchemaOrField) -> Result<AvroSchemaOrField> {
-        let mut avro_schema = value.unwrap_left();
+        let mut avro_schema = value.left().ok_or(Error::new(
+            ErrorKind::Unexpected,
+            "value expected to be schema",
+        ))?;
 
         if let AvroSchema::Record(record) = &mut avro_schema {
             record.name = Name::from(self.schema.as_str());
@@ -67,7 +70,10 @@ impl SchemaVisitor for SchemaToAvroSchema {
         field: &NestedFieldRef,
         avro_schema: AvroSchemaOrField,
     ) -> Result<AvroSchemaOrField> {
-        let mut field_schema = avro_schema.unwrap_left();
+        let mut field_schema = avro_schema.left().ok_or(Error::new(
+            ErrorKind::Unexpected,
+            "value expected to be schema",
+        ))?;
         if let AvroSchema::Record(record) = &mut field_schema {
             record.name = Name::from(format!("r{}", field.id).as_str());
         }
@@ -103,7 +109,15 @@ impl SchemaVisitor for SchemaToAvroSchema {
         _struct: &StructType,
         results: Vec<AvroSchemaOrField>,
     ) -> Result<AvroSchemaOrField> {
-        let avro_fields = results.into_iter().map(|r| r.unwrap_right()).collect_vec();
+        let avro_fields = results
+            .into_iter()
+            .map(|r| {
+                r.right().ok_or(Error::new(
+                    ErrorKind::Unexpected,
+                    "result should be avro record field",
+                ))
+            })
+            .collect::<Result<Vec<_>>>()?;
 
         Ok(Either::Left(
             // The name of this record schema should be determined later, by schema name or field
@@ -113,7 +127,10 @@ impl SchemaVisitor for SchemaToAvroSchema {
     }
 
     fn list(&mut self, list: &ListType, value: AvroSchemaOrField) -> Result<AvroSchemaOrField> {
-        let mut field_schema = value.unwrap_left();
+        let mut field_schema = value.left().ok_or(Error::new(
+            ErrorKind::Unexpected,
+            "value expected to be schema",
+        ))?;
 
         if let AvroSchema::Record(record) = &mut field_schema {
             record.name = Name::from(format!("r{}", list.element_field.id).as_str());
@@ -133,8 +150,14 @@ impl SchemaVisitor for SchemaToAvroSchema {
         key_value: AvroSchemaOrField,
         value: AvroSchemaOrField,
     ) -> Result<AvroSchemaOrField> {
-        let key_field_schema = key_value.unwrap_left();
-        let mut value_field_schema = value.unwrap_left();
+        let key_field_schema = key_value.left().ok_or(Error::new(
+            ErrorKind::Unexpected,
+            "value expected to be schema",
+        ))?;
+        let mut value_field_schema = value.left().ok_or(Error::new(
+            ErrorKind::Unexpected,
+            "value expected to be schema",
+        ))?;
         if !map.value_field.required {
             value_field_schema = avro_optional(value_field_schema)?;
         }
@@ -219,7 +242,13 @@ pub(crate) fn schema_to_avro_schema(name: impl ToString, schema: &Schema) -> Res
         schema: name.to_string(),
     };
 
-    visit_schema(schema, &mut converter).map(Either::unwrap_left)
+    match visit_schema(schema, &mut converter) {
+        Ok(s) => Ok(s.left().ok_or(Error::new(
+            ErrorKind::Unexpected,
+            "value expected to be schema",
+        ))?),
+        Err(e) => Err(e),
+    }
 }
 
 fn avro_record_schema(name: &str, fields: Vec<AvroRecordField>) -> Result<AvroSchema> {
@@ -361,10 +390,12 @@ impl AvroSchemaVisitor for AvroSchemaToSchema {
 
             let optional = is_avro_optional(&avro_field.schema);
 
+            let typ = typ.ok_or(Error::new(ErrorKind::Unexpected, "field type None"))?;
+
             let mut field = if optional {
-                NestedField::optional(field_id as i32, &avro_field.name, typ.unwrap())
+                NestedField::optional(field_id as i32, &avro_field.name, typ)
             } else {
-                NestedField::required(field_id as i32, &avro_field.name, typ.unwrap())
+                NestedField::required(field_id as i32, &avro_field.name, typ)
             };
 
             if let Some(doc) = &avro_field.doc {
@@ -397,17 +428,23 @@ impl AvroSchemaVisitor for AvroSchemaToSchema {
         }
 
         if options.len() == 1 {
-            Ok(Some(options.remove(0).unwrap()))
+            Ok(Some(options.remove(0).ok_or(Error::new(
+                ErrorKind::Unexpected,
+                "type removed is None",
+            ))?))
         } else {
-            Ok(Some(options.remove(1).unwrap()))
+            Ok(Some(options.remove(1).ok_or(Error::new(
+                ErrorKind::Unexpected,
+                "type removed is None",
+            ))?))
         }
     }
 
     fn array(&mut self, array: &AvroSchema, item: Option<Type>) -> Result<Self::T> {
         if let AvroSchema::Array(item_schema) = array {
             let element_field = NestedField::list_element(
                 self.next_field_id(),
-                item.unwrap(),
+                item.ok_or(Error::new(ErrorKind::Unexpected, "item should not be None"))?,
                 !is_avro_optional(item_schema),
             )
             .into();
@@ -430,7 +467,10 @@ impl AvroSchemaVisitor for AvroSchemaToSchema {
             );
             let value_field = NestedField::map_value_element(
                 self.next_field_id(),
-                value.unwrap(),
+                value.ok_or(Error::new(
+                    ErrorKind::Unexpected,
+                    "valud should not be None",
+                ))?,
                 !is_avro_optional(value_schema),
             );
             Ok(Some(Type::Map(MapType {
@@ -500,7 +540,10 @@ impl AvroSchemaVisitor for AvroSchemaToSchema {
 pub(crate) fn avro_schema_to_schema(avro_schema: &AvroSchema) -> Result<Schema> {
     if let AvroSchema::Record(_) = avro_schema {
         let mut converter = AvroSchemaToSchema { next_id: 0 };
-        let typ = visit(avro_schema, &mut converter)?.expect("Iceberg schema should not be none.");
+        let typ = visit(avro_schema, &mut converter)?.ok_or(Error::new(
+            ErrorKind::Unexpected,
+            "Iceberg schema should not be none.",
+        ))?;
         if let Type::Struct(s) = typ {
             Schema::builder()
                 .with_fields(s.fields().iter().cloned())
@@ -705,7 +748,7 @@ mod tests {
             "field-id": 100
         }
     ]
-}            
+}
 "#,
             )
             .unwrap()
@@ -768,7 +811,7 @@ mod tests {
             "field-id": 100
         }
     ]
-}            
+}
 "#,
             )
             .unwrap()
@@ -915,7 +958,7 @@ mod tests {
             "type": "string"
         }
     ]
-}               
+}
 "#,
             )
             .unwrap()

crates/iceberg/src/spec/manifest.rs

@@ -695,7 +695,7 @@ mod _const_schema {
                 ])),
             )),
         ];
-        let schema = Schema::builder().with_fields(fields).build().unwrap();
+        let schema = Schema::builder().with_fields(fields).build()?;
         schema_to_avro_schema("manifest_entry", &schema)
     }
 }

crates/iceberg/src/spec/schema.rs

@@ -136,7 +136,7 @@ impl SchemaBuilder {
         id_to_field: &HashMap<i32, NestedFieldRef>,
         identifier_field_ids: impl Iterator<Item = i32>,
     ) -> Result<()> {
-        let id_to_parent = index_parents(r#struct);
+        let id_to_parent = index_parents(r#struct)?;
         for identifier_field_id in identifier_field_ids {
             let field = id_to_field.get(&identifier_field_id).ok_or_else(|| {
                 Error::new(
@@ -169,9 +169,10 @@ impl SchemaBuilder {
 
             let mut cur_field_id = identifier_field_id;
             while let Some(parent) = id_to_parent.get(&cur_field_id) {
-                let parent_field = id_to_field
-                    .get(parent)
-                    .expect("Field id should not disappear.");
+                let parent_field = id_to_field.get(parent).ok_or(Error::new(
+                    ErrorKind::FeatureUnsupported,
+                    "Field id should not disappear.",
+                ))?;
                 ensure_data_valid!(
                     parent_field.field_type.is_struct(),
                     "Cannot add field {} as an identifier field: must not be nested in {:?}",
@@ -406,7 +407,7 @@ pub fn index_by_id(r#struct: &StructType) -> Result<HashMap<i32, NestedFieldRef>
 }
 
 /// Creates a field id to parent field id map.
-pub fn index_parents(r#struct: &StructType) -> HashMap<i32, i32> {
+pub fn index_parents(r#struct: &StructType) -> Result<HashMap<i32, i32>> {
     struct IndexByParent {
         parents: Vec<i32>,
         result: HashMap<i32, i32>,
@@ -487,8 +488,8 @@ pub fn index_parents(r#struct: &StructType) -> HashMap<i32, i32> {
         parents: vec![],
         result: HashMap::new(),
     };
-    visit_struct(r#struct, &mut index).unwrap();
-    index.result
+    visit_struct(r#struct, &mut index)?;
+    Ok(index.result)
 }
 
 #[derive(Default)]
@@ -971,13 +972,13 @@ mod tests {
 
     #[test]
     fn test_schema_display() {
-        let expected_str = r#"
+        let expected_str = "
 table {
-  1: foo: optional string 
-  2: bar: required int 
-  3: baz: optional boolean 
+  1: foo: optional string\x20
+  2: bar: required int\x20
+  3: baz: optional boolean\x20
 }
-"#;
+";
 
         assert_eq!(expected_str, format!("\n{}", table_schema_simple().0));
     }

crates/iceberg/src/spec/table_metadata.rs

@@ -210,10 +210,8 @@ impl TableMetadata {
     /// Get current snapshot
     #[inline]
     pub fn current_snapshot(&self) -> Option<&SnapshotRef> {
-        self.current_snapshot_id.map(|s| {
-            self.snapshot_by_id(s)
-                .expect("Current snapshot id has been set, but doesn't exist in metadata")
-        })
+        self.current_snapshot_id
+            .and_then(|s| self.snapshot_by_id(s))
     }
 
     /// Return all sort orders.

crates/iceberg/src/transaction.rs

@@ -160,7 +160,10 @@ impl<'a> ReplaceSortOrderAction<'a> {
                     .table
                     .metadata()
                     .default_sort_order()
-                    .expect("default sort order impossible to be None")
+                    .ok_or(Error::new(
+                        ErrorKind::Unexpected,
+                        "default sort order impossible to be none",
+                    ))?
                     .order_id,
             },
         ];