refactor: remove unwraps

crates/iceberg/src/avro/schema.rs

@@ -48,6 +48,7 @@ impl SchemaVisitor for SchemaToAvroSchema {
     type T = AvroSchemaOrField;
 
     fn schema(&mut self, _schema: &Schema, value: AvroSchemaOrField) -> Result<AvroSchemaOrField> {
+        // value should never be None
         let mut avro_schema = value.unwrap_left();
 
         if let AvroSchema::Record(record) = &mut avro_schema {
@@ -67,7 +68,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 +107,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 +125,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 +148,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 +240,12 @@ 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)
+    visit_schema(schema, &mut converter).and_then(|s| {
+        s.left().ok_or(Error::new(
+            ErrorKind::Unexpected,
+            "value expected to be schema",
+        ))
+    })
 }
 
 fn avro_record_schema(name: &str, fields: Vec<AvroRecordField>) -> Result<AvroSchema> {
@@ -361,10 +387,15 @@ impl AvroSchemaVisitor for AvroSchemaToSchema {
 
             let optional = is_avro_optional(&avro_field.schema);
 
+            let typ = typ.ok_or(Error::new(
+                ErrorKind::Unexpected,
+                "Field type should not be 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 {
@@ -705,7 +736,7 @@ mod tests {
             "field-id": 100
         }
     ]
-}            
+}
 "#,
             )
             .unwrap()
@@ -768,7 +799,7 @@ mod tests {
             "field-id": 100
         }
     ]
-}            
+}
 "#,
             )
             .unwrap()
@@ -915,7 +946,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(
@@ -406,7 +406,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 +487,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 +971,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/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,
             },
         ];