chore: bump MSRV to 1.88, fix warnings and clippy errors (#1902)

#1899 requires a bump to MSRV
1.88. This version is within the policy of this project, and since the
README mentions `MSRV is updated when we release iceberg-rust` and we're
preparing 0.8, here's a PR for just MSRV 1.88.

## Which issue does this PR close?

<!--
We generally require a GitHub issue to be filed for all bug fixes and
enhancements and this helps us generate change logs for our releases.
You can link an issue to this PR using the GitHub syntax. For example
`Closes #123` indicates that this PR will close issue #123.
-->

N/A.

## What changes are included in this PR?

<!--
Provide a summary of the modifications in this PR. List the main changes
such as new features, bug fixes, refactoring, or any other updates.
-->

- Bump MSRV to 1.88
- Fix warnings
- Fix errors found by `make check-clippy`
- Format

## Are these changes tested?

<!--
Specify what test covers (unit test, integration test, etc.).

If tests are not included in your PR, please explain why (for example,
are they covered by existing tests)?
-->

Existing tests

Author: mbutrovich

Cargo.toml

@@ -36,7 +36,7 @@ version = "0.7.0"
 license = "Apache-2.0"
 repository = "https://github.com/apache/iceberg-rust"
 # Check the MSRV policy in README.md before changing this
-rust-version = "1.87"
+rust-version = "1.88"
 
 [workspace.dependencies]
 anyhow = "1.0.72"

crates/catalog/glue/src/catalog.rs

@@ -151,33 +151,33 @@ impl GlueCatalog {
     async fn new(config: GlueCatalogConfig) -> Result<Self> {
         let sdk_config = create_sdk_config(&config.props, config.uri.as_ref()).await;
         let mut file_io_props = config.props.clone();
-        if !file_io_props.contains_key(S3_ACCESS_KEY_ID) {
-            if let Some(access_key_id) = file_io_props.get(AWS_ACCESS_KEY_ID) {
-                file_io_props.insert(S3_ACCESS_KEY_ID.to_string(), access_key_id.to_string());
-            }
+        if !file_io_props.contains_key(S3_ACCESS_KEY_ID)
+            && let Some(access_key_id) = file_io_props.get(AWS_ACCESS_KEY_ID)
+        {
+            file_io_props.insert(S3_ACCESS_KEY_ID.to_string(), access_key_id.to_string());
         }
-        if !file_io_props.contains_key(S3_SECRET_ACCESS_KEY) {
-            if let Some(secret_access_key) = file_io_props.get(AWS_SECRET_ACCESS_KEY) {
-                file_io_props.insert(
-                    S3_SECRET_ACCESS_KEY.to_string(),
-                    secret_access_key.to_string(),
-                );
-            }
+        if !file_io_props.contains_key(S3_SECRET_ACCESS_KEY)
+            && let Some(secret_access_key) = file_io_props.get(AWS_SECRET_ACCESS_KEY)
+        {
+            file_io_props.insert(
+                S3_SECRET_ACCESS_KEY.to_string(),
+                secret_access_key.to_string(),
+            );
         }
-        if !file_io_props.contains_key(S3_REGION) {
-            if let Some(region) = file_io_props.get(AWS_REGION_NAME) {
-                file_io_props.insert(S3_REGION.to_string(), region.to_string());
-            }
+        if !file_io_props.contains_key(S3_REGION)
+            && let Some(region) = file_io_props.get(AWS_REGION_NAME)
+        {
+            file_io_props.insert(S3_REGION.to_string(), region.to_string());
         }
-        if !file_io_props.contains_key(S3_SESSION_TOKEN) {
-            if let Some(session_token) = file_io_props.get(AWS_SESSION_TOKEN) {
-                file_io_props.insert(S3_SESSION_TOKEN.to_string(), session_token.to_string());
-            }
+        if !file_io_props.contains_key(S3_SESSION_TOKEN)
+            && let Some(session_token) = file_io_props.get(AWS_SESSION_TOKEN)
+        {
+            file_io_props.insert(S3_SESSION_TOKEN.to_string(), session_token.to_string());
         }
-        if !file_io_props.contains_key(S3_ENDPOINT) {
-            if let Some(aws_endpoint) = config.uri.as_ref() {
-                file_io_props.insert(S3_ENDPOINT.to_string(), aws_endpoint.to_string());
-            }
+        if !file_io_props.contains_key(S3_ENDPOINT)
+            && let Some(aws_endpoint) = config.uri.as_ref()
+        {
+            file_io_props.insert(S3_ENDPOINT.to_string(), aws_endpoint.to_string());
         }
 
         let client = aws_sdk_glue::Client::new(&sdk_config);

crates/iceberg/src/arrow/reader.rs

@@ -504,10 +504,10 @@ impl ArrowReader {
                     // we need to call next() to update the cache with the newly positioned value.
                     delete_vector_iter.advance_to(next_row_group_base_idx);
                     // Only update the cache if the cached value is stale (in the skipped range)
-                    if let Some(cached_idx) = next_deleted_row_idx_opt {
-                        if cached_idx < next_row_group_base_idx {
-                            next_deleted_row_idx_opt = delete_vector_iter.next();
-                        }
+                    if let Some(cached_idx) = next_deleted_row_idx_opt
+                        && cached_idx < next_row_group_base_idx
+                    {
+                        next_deleted_row_idx_opt = delete_vector_iter.next();
                     }
 
                     // still increment the current page base index but then skip to the next row group
@@ -861,10 +861,10 @@ impl ArrowReader {
         };
 
         // If all row groups were filtered out, return an empty RowSelection (select no rows)
-        if let Some(selected_row_groups) = selected_row_groups {
-            if selected_row_groups.is_empty() {
-                return Ok(RowSelection::from(Vec::new()));
-            }
+        if let Some(selected_row_groups) = selected_row_groups
+            && selected_row_groups.is_empty()
+        {
+            return Ok(RowSelection::from(Vec::new()));
         }
 
         let mut selected_row_groups_idx = 0;
@@ -897,10 +897,10 @@ impl ArrowReader {
 
             results.push(selections_for_page);
 
-            if let Some(selected_row_groups) = selected_row_groups {
-                if selected_row_groups_idx == selected_row_groups.len() {
-                    break;
-                }
+            if let Some(selected_row_groups) = selected_row_groups
+                && selected_row_groups_idx == selected_row_groups.len()
+            {
+                break;
             }
         }
 
@@ -1031,13 +1031,13 @@ fn apply_name_mapping_to_arrow_schema(
 
             let mut metadata = field.metadata().clone();
 
-            if let Some(mapped_field) = mapped_field_opt {
-                if let Some(field_id) = mapped_field.field_id() {
-                    // Field found in mapping with a field_id → assign it
-                    metadata.insert(PARQUET_FIELD_ID_META_KEY.to_string(), field_id.to_string());
-                }
-                // If field_id is None, leave the field without an ID (will be filtered by projection)
+            if let Some(mapped_field) = mapped_field_opt
+                && let Some(field_id) = mapped_field.field_id()
+            {
+                // Field found in mapping with a field_id → assign it
+                metadata.insert(PARQUET_FIELD_ID_META_KEY.to_string(), field_id.to_string());
             }
+            // If field_id is None, leave the field without an ID (will be filtered by projection)
             // If field not found in mapping, leave it without an ID (will be filtered by projection)
 
             Field::new(field.name(), field.data_type().clone(), field.is_nullable())
@@ -2731,15 +2731,14 @@ message schema {
         // Step 4: Verify we got 199 rows (not 200)
         let total_rows: usize = result.iter().map(|b| b.num_rows()).sum();
 
-        println!("Total rows read: {}", total_rows);
+        println!("Total rows read: {total_rows}");
         println!("Expected: 199 rows (deleted row 199 which had id=200)");
 
         // This assertion will FAIL before the fix and PASS after the fix
         assert_eq!(
             total_rows, 199,
-            "Expected 199 rows after deleting row 199, but got {} rows. \
-             The bug causes position deletes in later row groups to be ignored.",
-            total_rows
+            "Expected 199 rows after deleting row 199, but got {total_rows} rows. \
+             The bug causes position deletes in later row groups to be ignored."
         );
 
         // Verify the deleted row (id=200) is not present
@@ -2950,16 +2949,15 @@ message schema {
         // Row group 1 has 100 rows (ids 101-200), minus 1 delete (id=200) = 99 rows
         let total_rows: usize = result.iter().map(|b| b.num_rows()).sum();
 
-        println!("Total rows read from row group 1: {}", total_rows);
+        println!("Total rows read from row group 1: {total_rows}");
         println!("Expected: 99 rows (row group 1 has 100 rows, 1 delete at position 199)");
 
         // This assertion will FAIL before the fix and PASS after the fix
         assert_eq!(
             total_rows, 99,
-            "Expected 99 rows from row group 1 after deleting position 199, but got {} rows. \
+            "Expected 99 rows from row group 1 after deleting position 199, but got {total_rows} rows. \
              The bug causes position deletes to be lost when advance_to() is followed by next() \
-             when skipping unselected row groups.",
-            total_rows
+             when skipping unselected row groups."
         );
 
         // Verify the deleted row (id=200) is not present
@@ -3241,7 +3239,7 @@ message schema {
                 start: 0,
                 length: 0,
                 record_count: None,
-                data_file_path: format!("{}/1.parquet", table_location),
+                data_file_path: format!("{table_location}/1.parquet"),
                 data_file_format: DataFileFormat::Parquet,
                 schema: schema.clone(),
                 project_field_ids: vec![1, 2],
@@ -3338,7 +3336,7 @@ message schema {
                 start: 0,
                 length: 0,
                 record_count: None,
-                data_file_path: format!("{}/1.parquet", table_location),
+                data_file_path: format!("{table_location}/1.parquet"),
                 data_file_format: DataFileFormat::Parquet,
                 schema: schema.clone(),
                 project_field_ids: vec![1, 3],
@@ -3424,7 +3422,7 @@ message schema {
                 start: 0,
                 length: 0,
                 record_count: None,
-                data_file_path: format!("{}/1.parquet", table_location),
+                data_file_path: format!("{table_location}/1.parquet"),
                 data_file_format: DataFileFormat::Parquet,
                 schema: schema.clone(),
                 project_field_ids: vec![1, 2, 3],
@@ -3524,7 +3522,7 @@ message schema {
                 start: 0,
                 length: 0,
                 record_count: None,
-                data_file_path: format!("{}/1.parquet", table_location),
+                data_file_path: format!("{table_location}/1.parquet"),
                 data_file_format: DataFileFormat::Parquet,
                 schema: schema.clone(),
                 project_field_ids: vec![1, 2],
@@ -3565,7 +3563,7 @@ message schema {
         assert_eq!(all_values.len(), 6);
 
         for i in 0..6 {
-            assert_eq!(all_names[i], format!("name_{}", i));
+            assert_eq!(all_names[i], format!("name_{i}"));
             assert_eq!(all_values[i], i as i32);
         }
     }
@@ -3653,7 +3651,7 @@ message schema {
                 start: 0,
                 length: 0,
                 record_count: None,
-                data_file_path: format!("{}/1.parquet", table_location),
+                data_file_path: format!("{table_location}/1.parquet"),
                 data_file_format: DataFileFormat::Parquet,
                 schema: schema.clone(),
                 project_field_ids: vec![1, 2],
@@ -3749,7 +3747,7 @@ message schema {
                 start: 0,
                 length: 0,
                 record_count: None,
-                data_file_path: format!("{}/1.parquet", table_location),
+                data_file_path: format!("{table_location}/1.parquet"),
                 data_file_format: DataFileFormat::Parquet,
                 schema: schema.clone(),
                 project_field_ids: vec![1, 5, 2],
@@ -3858,7 +3856,7 @@ message schema {
                 start: 0,
                 length: 0,
                 record_count: None,
-                data_file_path: format!("{}/1.parquet", table_location),
+                data_file_path: format!("{table_location}/1.parquet"),
                 data_file_format: DataFileFormat::Parquet,
                 schema: schema.clone(),
                 project_field_ids: vec![1, 2, 3],
@@ -3997,7 +3995,7 @@ message schema {
                 start: 0,
                 length: 0,
                 record_count: None,
-                data_file_path: format!("{}/data.parquet", table_location),
+                data_file_path: format!("{table_location}/data.parquet"),
                 data_file_format: DataFileFormat::Parquet,
                 schema: schema.clone(),
                 project_field_ids: vec![1, 2],

crates/iceberg/src/arrow/record_batch_projector.rs

@@ -133,25 +133,24 @@ impl RecordBatchProjector {
     {
         for (pos, field) in fields.iter().enumerate() {
             let id = field_id_fetch_func(field)?;
-            if let Some(id) = id {
-                if target_field_id == id {
-                    index_vec.push(pos);
-                    return Ok(Some(field.clone()));
-                }
+            if let Some(id) = id
+                && target_field_id == id
+            {
+                index_vec.push(pos);
+                return Ok(Some(field.clone()));
             }
-            if let DataType::Struct(inner) = field.data_type() {
-                if searchable_field_func(field) {
-                    if let Some(res) = Self::fetch_field_index(
-                        inner,
-                        index_vec,
-                        target_field_id,
-                        field_id_fetch_func,
-                        searchable_field_func,
-                    )? {
-                        index_vec.push(pos);
-                        return Ok(Some(res));
-                    }
-                }
+            if let DataType::Struct(inner) = field.data_type()
+                && searchable_field_func(field)
+                && let Some(res) = Self::fetch_field_index(
+                    inner,
+                    index_vec,
+                    target_field_id,
+                    field_id_fetch_func,
+                    searchable_field_func,
+                )?
+            {
+                index_vec.push(pos);
+                return Ok(Some(res));
             }
         }
         Ok(None)

crates/iceberg/src/arrow/record_batch_transformer.rs

@@ -582,7 +582,7 @@ impl RecordBatchTransformer {
                 let this_field_id = field_id_str.parse().map_err(|e| {
                     Error::new(
                         ErrorKind::DataInvalid,
-                        format!("field id not parseable as an i32: {}", e),
+                        format!("field id not parseable as an i32: {e}"),
                     )
                 })?;
 

crates/iceberg/src/arrow/value.rs

@@ -261,15 +261,15 @@ impl SchemaWithPartnerVisitor<ArrayRef> for ArrowArrayToIcebergStructConverter {
                             "The partner is not a decimal128 array",
                         )
                     })?;
-                if let DataType::Decimal128(arrow_precision, arrow_scale) = array.data_type() {
-                    if *arrow_precision as u32 != *precision || *arrow_scale as u32 != *scale {
-                        return Err(Error::new(
-                            ErrorKind::DataInvalid,
-                            format!(
-                                "The precision or scale ({arrow_precision},{arrow_scale}) of arrow decimal128 array is not compatible with iceberg decimal type ({precision},{scale})"
-                            ),
-                        ));
-                    }
+                if let DataType::Decimal128(arrow_precision, arrow_scale) = array.data_type()
+                    && (*arrow_precision as u32 != *precision || *arrow_scale as u32 != *scale)
+                {
+                    return Err(Error::new(
+                        ErrorKind::DataInvalid,
+                        format!(
+                            "The precision or scale ({arrow_precision},{arrow_scale}) of arrow decimal128 array is not compatible with iceberg decimal type ({precision},{scale})"
+                        ),
+                    ));
                 }
                 Ok(array.iter().map(|v| v.map(Literal::decimal)).collect())
             }
@@ -351,10 +351,10 @@ impl SchemaWithPartnerVisitor<ArrayRef> for ArrowArrayToIcebergStructConverter {
                 } else if let Some(array) = partner.as_any().downcast_ref::<StringArray>() {
                     Ok(array.iter().map(|v| v.map(Literal::string)).collect())
                 } else {
-                    return Err(Error::new(
+                    Err(Error::new(
                         ErrorKind::DataInvalid,
                         "The partner is not a string array",
-                    ));
+                    ))
                 }
             }
             PrimitiveType::Uuid => {
@@ -418,10 +418,10 @@ impl SchemaWithPartnerVisitor<ArrayRef> for ArrowArrayToIcebergStructConverter {
                         .map(|v| v.map(|v| Literal::binary(v.to_vec())))
                         .collect())
                 } else {
-                    return Err(Error::new(
+                    Err(Error::new(
                         ErrorKind::DataInvalid,
                         "The partner is not a binary array",
-                    ));
+                    ))
                 }
             }
         }
@@ -724,10 +724,7 @@ pub(crate) fn create_primitive_array_single_element(
         }
         _ => Err(Error::new(
             ErrorKind::Unexpected,
-            format!(
-                "Unsupported constant type combination: {:?} with {:?}",
-                data_type, prim_lit
-            ),
+            format!("Unsupported constant type combination: {data_type:?} with {prim_lit:?}"),
         )),
     }
 }
@@ -825,7 +822,7 @@ pub(crate) fn create_primitive_array_repeated(
         (dt, _) => {
             return Err(Error::new(
                 ErrorKind::Unexpected,
-                format!("unexpected target column type {}", dt),
+                format!("unexpected target column type {dt}"),
             ));
         }
     })

crates/iceberg/src/catalog/mod.rs

@@ -1000,13 +1000,13 @@ mod _serde_set_statistics {
             snapshot_id,
             statistics,
         } = SetStatistics::deserialize(deserializer)?;
-        if let Some(snapshot_id) = snapshot_id {
-            if snapshot_id != statistics.snapshot_id {
-                return Err(serde::de::Error::custom(format!(
-                    "Snapshot id to set {snapshot_id} does not match the statistics file snapshot id {}",
-                    statistics.snapshot_id
-                )));
-            }
+        if let Some(snapshot_id) = snapshot_id
+            && snapshot_id != statistics.snapshot_id
+        {
+            return Err(serde::de::Error::custom(format!(
+                "Snapshot id to set {snapshot_id} does not match the statistics file snapshot id {}",
+                statistics.snapshot_id
+            )));
         }
 
         Ok(statistics)