From c2469c99383dd70761668cbff26a4729b0a88d12 Mon Sep 17 00:00:00 2001 From: Raminder Singh Date: Wed, 29 May 2024 06:54:47 +0530 Subject: [PATCH] fix: clippy warnings --- crates/core/src/kernel/snapshot/mod.rs | 6 ++--- crates/core/src/kernel/snapshot/visitors.rs | 2 +- crates/core/src/operations/cast.rs | 4 ++-- crates/core/src/operations/optimize.rs | 2 +- .../src/operations/transaction/application.rs | 6 ++--- crates/core/src/operations/writer.rs | 4 ++-- crates/core/src/protocol/checkpoints.rs | 2 +- crates/core/src/table/state_arrow.rs | 2 +- crates/core/src/writer/json.rs | 7 ++---- crates/core/src/writer/record_batch.rs | 6 ++--- crates/core/tests/command_merge.rs | 24 +++++++++---------- crates/core/tests/integration_datafusion.rs | 2 +- 12 files changed, 32 insertions(+), 35 deletions(-) diff --git a/crates/core/src/kernel/snapshot/mod.rs b/crates/core/src/kernel/snapshot/mod.rs index 254a15ea5a..cd6cf8bb5f 100644 --- a/crates/core/src/kernel/snapshot/mod.rs +++ b/crates/core/src/kernel/snapshot/mod.rs @@ -393,7 +393,7 @@ impl EagerSnapshot { ) -> DeltaResult { let mut visitors = tracked_actions .iter() - .flat_map(|a| get_visitor(a)) + .flat_map(get_visitor) .collect::>(); let snapshot = Snapshot::try_new(table_root, store.clone(), config, version).await?; let files = snapshot.files(store, &mut visitors)?.try_collect().await?; @@ -469,7 +469,7 @@ impl EagerSnapshot { let mut visitors = self .tracked_actions .iter() - .flat_map(|a| get_visitor(a)) + .flat_map(get_visitor) .collect::>(); let mut schema_actions: HashSet<_> = @@ -629,7 +629,7 @@ impl EagerSnapshot { let mut visitors = self .tracked_actions .iter() - .flat_map(|a| get_visitor(a)) + .flat_map(get_visitor) .collect::>(); let mut schema_actions: HashSet<_> = visitors.iter().flat_map(|v| v.required_actions()).collect(); diff --git a/crates/core/src/kernel/snapshot/visitors.rs b/crates/core/src/kernel/snapshot/visitors.rs index 2f9a010453..1b68026a5b 100644 --- a/crates/core/src/kernel/snapshot/visitors.rs +++ b/crates/core/src/kernel/snapshot/visitors.rs @@ -50,7 +50,7 @@ impl AppTransactionVisitor { for (key, value) in &self.app_transaction_version { clone.insert(key.clone(), value.clone()); } - return clone; + clone } } diff --git a/crates/core/src/operations/cast.rs b/crates/core/src/operations/cast.rs index f92b3c646e..b231346266 100644 --- a/crates/core/src/operations/cast.rs +++ b/crates/core/src/operations/cast.rs @@ -230,7 +230,7 @@ mod tests { assert_eq!(result.fields().len(), 1); let delta_type: DeltaDataType = result.fields()[0].data_type().try_into().unwrap(); assert_eq!(delta_type, DeltaDataType::STRING); - assert_eq!(result.fields()[0].is_nullable(), true); + assert!(result.fields()[0].is_nullable()); } #[test] @@ -282,7 +282,7 @@ mod tests { delta_type, DeltaDataType::Array(Box::new(DeltaArrayType::new(DeltaDataType::STRING, false))) ); - assert_eq!(result.fields()[0].is_nullable(), true); + assert!(result.fields()[0].is_nullable()); } #[test] diff --git a/crates/core/src/operations/optimize.rs b/crates/core/src/operations/optimize.rs index 3a15c3f130..10cbb6a22a 100644 --- a/crates/core/src/operations/optimize.rs +++ b/crates/core/src/operations/optimize.rs @@ -1345,7 +1345,7 @@ pub(super) mod zorder { "+-----+-----+-----------+", ]; - let expected = vec![expected_1, expected_2, expected_3]; + let expected = [expected_1, expected_2, expected_3]; let indices = Int32Array::from(shuffled_indices().to_vec()); let shuffled_columns = batch diff --git a/crates/core/src/operations/transaction/application.rs b/crates/core/src/operations/transaction/application.rs index b13128c9b3..5a636bcecf 100644 --- a/crates/core/src/operations/transaction/application.rs +++ b/crates/core/src/operations/transaction/application.rs @@ -97,7 +97,7 @@ mod tests { .with_partition_columns(["modified"]) .with_commit_properties( CommitProperties::default() - .with_application_transaction(Transaction::new(&"my-app", 1)), + .with_application_transaction(Transaction::new("my-app", 1)), ) .await .unwrap(); @@ -113,7 +113,7 @@ mod tests { .write(vec![get_record_batch(None, false)]) .with_commit_properties( CommitProperties::default() - .with_application_transaction(Transaction::new(&"my-app", 2)), + .with_application_transaction(Transaction::new("my-app", 2)), ) .await .unwrap(); @@ -123,7 +123,7 @@ mod tests { .write(vec![get_record_batch(None, false)]) .with_commit_properties( CommitProperties::default() - .with_application_transaction(Transaction::new(&"my-app", 3)), + .with_application_transaction(Transaction::new("my-app", 3)), ) .await; diff --git a/crates/core/src/operations/writer.rs b/crates/core/src/operations/writer.rs index e42adf0288..f04d68e412 100644 --- a/crates/core/src/operations/writer.rs +++ b/crates/core/src/operations/writer.rs @@ -613,7 +613,7 @@ mod tests { match result { Ok(_) => { - assert!(false, "Should not have successfully written"); + panic!("Should not have successfully written"); } Err(e) => { match e { @@ -621,7 +621,7 @@ mod tests { // this is expected } others => { - assert!(false, "Got the wrong error: {others:?}"); + panic!("Got the wrong error: {others:?}"); } } } diff --git a/crates/core/src/protocol/checkpoints.rs b/crates/core/src/protocol/checkpoints.rs index 3b843a11d5..67994c5e49 100644 --- a/crates/core/src/protocol/checkpoints.rs +++ b/crates/core/src/protocol/checkpoints.rs @@ -302,7 +302,7 @@ fn parquet_bytes_from_state( state .app_transaction_version() .map_err(|_| CheckpointError::MissingActionType("txn".to_string()))? - .map(|txn| Action::Txn(txn)), + .map(Action::Txn), ) // removes .chain(tombstones.iter().map(|r| { diff --git a/crates/core/src/table/state_arrow.rs b/crates/core/src/table/state_arrow.rs index 9d23f3169f..fe35787cb4 100644 --- a/crates/core/src/table/state_arrow.rs +++ b/crates/core/src/table/state_arrow.rs @@ -572,7 +572,7 @@ impl DeltaTableState { // into StructArrays, until it is consolidated into a single array. columnar_stats = columnar_stats .into_iter() - .group_by(|col_stat| { + .chunk_by(|col_stat| { if col_stat.path.len() < level { col_stat.path.clone() } else { diff --git a/crates/core/src/writer/json.rs b/crates/core/src/writer/json.rs index 72d6ffff42..d97d3ef16c 100644 --- a/crates/core/src/writer/json.rs +++ b/crates/core/src/writer/json.rs @@ -600,11 +600,8 @@ mod tests { } ); - match writer.write(vec![second_data]).await { - Ok(_) => { - assert!(false, "Should not have successfully written"); - } - _ => {} + if writer.write(vec![second_data]).await.is_ok() { + panic!("Should not have successfully written"); } } diff --git a/crates/core/src/writer/record_batch.rs b/crates/core/src/writer/record_batch.rs index 44e2e676d7..c21435dd14 100644 --- a/crates/core/src/writer/record_batch.rs +++ b/crates/core/src/writer/record_batch.rs @@ -566,7 +566,7 @@ mod tests { let mut writer = RecordBatchWriter::for_table(&table).unwrap(); let partitions = writer.divide_by_partition_values(&batch).unwrap(); - let expected_keys = vec![ + let expected_keys = [ String::from("modified=2021-02-01"), String::from("modified=2021-02-02"), ]; @@ -710,7 +710,7 @@ mod tests { match result { Ok(_) => { - assert!(false, "Should not have successfully written"); + panic!("Should not have successfully written"); } Err(e) => { match e { @@ -718,7 +718,7 @@ mod tests { // this is expected } others => { - assert!(false, "Got the wrong error: {others:?}"); + panic!("Got the wrong error: {others:?}"); } } } diff --git a/crates/core/tests/command_merge.rs b/crates/core/tests/command_merge.rs index 988891f332..59a941a24f 100644 --- a/crates/core/tests/command_merge.rs +++ b/crates/core/tests/command_merge.rs @@ -14,9 +14,9 @@ use deltalake_core::protocol::SaveMode; use deltalake_core::{open_table, DeltaOps, DeltaResult, DeltaTable, DeltaTableError}; use std::sync::Arc; -async fn create_table(table_uri: &String, partition: Option>) -> DeltaTable { +async fn create_table(table_uri: &str, partition: Option>) -> DeltaTable { let table_schema = get_delta_schema(); - let ops = DeltaOps::try_from_uri(table_uri.clone()).await.unwrap(); + let ops = DeltaOps::try_from_uri(table_uri).await.unwrap(); let table = ops .create() .with_columns(table_schema.fields().clone()) @@ -25,7 +25,7 @@ async fn create_table(table_uri: &String, partition: Option>) -> Delta .expect("Failed to create table"); let schema = get_arrow_schema(); - return write_data(table, &schema).await; + write_data(table, &schema).await } fn get_delta_schema() -> StructType { @@ -49,11 +49,11 @@ fn get_delta_schema() -> StructType { } fn get_arrow_schema() -> Arc { - return Arc::new(ArrowSchema::new(vec![ + Arc::new(ArrowSchema::new(vec![ Field::new("id", DataType::Utf8, true), Field::new("value", DataType::Int32, true), Field::new("event_date", DataType::Utf8, true), - ])); + ])) } async fn write_data(table: DeltaTable, schema: &Arc) -> DeltaTable { @@ -108,7 +108,7 @@ fn create_test_data() -> (DataFrame, DataFrame) { ) .unwrap(); let df2 = ctx.read_batch(batch).unwrap(); - return (df1, df2); + (df1, df2) } async fn merge( @@ -116,7 +116,7 @@ async fn merge( df: DataFrame, predicate: Expr, ) -> DeltaResult<(DeltaTable, MergeMetrics)> { - return DeltaOps(table) + DeltaOps(table) .merge(df, predicate) .with_source_alias("source") .with_target_alias("target") @@ -133,7 +133,7 @@ async fn merge( .set("event_date", col("source.event_date")) }) .unwrap() - .await; + .await } #[tokio::test] @@ -142,7 +142,7 @@ async fn test_merge_concurrent_conflict() { let tmp_dir = tempfile::tempdir().unwrap(); let table_uri = tmp_dir.path().to_str().to_owned().unwrap(); - let table_ref1 = create_table(&table_uri.to_string(), Some(vec!["event_date"])).await; + let table_ref1 = create_table(table_uri, Some(vec!["event_date"])).await; let table_ref2 = open_table(table_uri).await.unwrap(); let (df1, df2) = create_test_data(); @@ -165,7 +165,7 @@ async fn test_merge_concurrent_different_partition() { let tmp_dir = tempfile::tempdir().unwrap(); let table_uri = tmp_dir.path().to_str().to_owned().unwrap(); - let table_ref1 = create_table(&table_uri.to_string(), Some(vec!["event_date"])).await; + let table_ref1 = create_table(table_uri, Some(vec!["event_date"])).await; let table_ref2 = open_table(table_uri).await.unwrap(); let (df1, df2) = create_test_data(); @@ -177,7 +177,7 @@ async fn test_merge_concurrent_different_partition() { // TODO: Currently it throws a Version mismatch error, but the merge commit was successfully // This bug needs to be fixed, see pull request #2280 - assert!(matches!(result.as_ref().is_ok(), true)); + assert!(result.as_ref().is_ok()); } #[tokio::test] @@ -186,7 +186,7 @@ async fn test_merge_concurrent_with_overlapping_files() { let tmp_dir = tempfile::tempdir().unwrap(); let table_uri = tmp_dir.path().to_str().to_owned().unwrap(); - let table_ref1 = create_table(&table_uri.to_string(), None).await; + let table_ref1 = create_table(table_uri, None).await; let table_ref2 = open_table(table_uri).await.unwrap(); let (df1, _df2) = create_test_data(); diff --git a/crates/core/tests/integration_datafusion.rs b/crates/core/tests/integration_datafusion.rs index 54a70faf53..64d80e3bce 100644 --- a/crates/core/tests/integration_datafusion.rs +++ b/crates/core/tests/integration_datafusion.rs @@ -114,7 +114,7 @@ mod local { for batch in batches { table = DeltaOps(table) .write(vec![batch]) - .with_save_mode(save_mode.clone()) + .with_save_mode(save_mode) .await .unwrap(); }