From 1a10cce8e666fd69d3440a3c09a2eff00e1516c6 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 1 Sep 2026 13:27:39 +0200 Subject: [PATCH 1/6] Enable clippy::manual_string_new `String::new()` instead of `"".to_string()` / `"".to_owned()` / `"".into()`, which skips the copy-from-empty-slice path. All sites fixed by `cargo clippy --fix`. --- Cargo.toml | 1 - datafusion-cli/src/exec.rs | 4 ++-- datafusion-cli/src/functions.rs | 3 +-- .../examples/data_io/parquet_encrypted_with_kms.rs | 2 +- datafusion/common/src/config.rs | 8 ++++---- datafusion/common/src/error.rs | 11 +++++------ datafusion/common/src/scalar/mod.rs | 14 +++++++------- datafusion/core/src/bin/print_functions_docs.rs | 2 +- .../core/src/datasource/listing_table_factory.rs | 2 +- .../core/src/datasource/physical_plan/csv.rs | 2 +- .../core/src/datasource/physical_plan/json.rs | 2 +- .../fuzz_cases/aggregation_fuzzer/query_builder.rs | 2 +- datafusion/core/tests/fuzz_cases/window_fuzz.rs | 2 +- datafusion/datasource/src/file_compression_type.rs | 2 +- datafusion/expr-common/src/casts.rs | 4 ++-- datafusion/expr/src/expr_schema.rs | 4 ++-- datafusion/expr/src/logical_plan/display.rs | 2 +- datafusion/expr/src/logical_plan/plan.rs | 8 ++++---- datafusion/functions/src/regex/regexpcount.rs | 6 +++--- datafusion/functions/src/string/ascii.rs | 2 +- datafusion/functions/src/string/concat.rs | 4 ++-- datafusion/functions/src/string/octet_length.rs | 8 ++++---- datafusion/functions/src/string/replace.rs | 2 +- datafusion/functions/src/string/split_part.rs | 8 ++++---- .../functions/src/unicode/character_length.rs | 2 +- datafusion/functions/src/unicode/find_in_set.rs | 4 ++-- datafusion/functions/src/unicode/initcap.rs | 2 +- datafusion/functions/src/unicode/left.rs | 2 +- datafusion/functions/src/unicode/lpad.rs | 2 +- datafusion/functions/src/unicode/right.rs | 2 +- datafusion/physical-plan/src/display.rs | 4 ++-- datafusion/physical-plan/src/filter.rs | 4 ++-- .../physical-plan/src/joins/hash_join/exec.rs | 10 +++++----- .../physical-plan/src/joins/nested_loop_join.rs | 10 +++++----- .../src/joins/sort_merge_join/exec.rs | 8 ++++---- .../physical-plan/src/joins/symmetric_hash_join.rs | 8 ++++---- datafusion/physical-plan/src/render_tree.rs | 2 +- .../proto/tests/cases/roundtrip_logical_plan.rs | 2 +- datafusion/spark/src/function/string/char.rs | 6 +++--- .../spark/src/function/string/format_string.rs | 8 ++++---- datafusion/spark/src/function/string/length.rs | 4 ++-- datafusion/spark/src/function/url/parse_url.rs | 10 +++++----- datafusion/sql/src/statement.rs | 4 ++-- test-utils/src/array_gen/string.rs | 2 +- 44 files changed, 99 insertions(+), 102 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 66bd816945908..9bbfbffce268b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -288,7 +288,6 @@ implicit_clone = "allow" # 198 hits implicit_hasher = "allow" # 17 hits; some sites feed arrow APIs that require the default hasher inline_always = "allow" # 45 hits items_after_statements = "allow" # 171 hits -manual_string_new = "allow" # 84 hits many_single_char_names = "allow" # 12 hits; short names are idiomatic in the numeric kernels map_unwrap_or = "allow" # 198 hits match_bool = "allow" # 46 hits diff --git a/datafusion-cli/src/exec.rs b/datafusion-cli/src/exec.rs index 288ce4b7351b6..7a749386dbbd0 100644 --- a/datafusion-cli/src/exec.rs +++ b/datafusion-cli/src/exec.rs @@ -69,7 +69,7 @@ pub async fn exec_from_lines( reader: &mut BufReader, print_options: &PrintOptions, ) -> Result<()> { - let mut query = "".to_owned(); + let mut query = String::new(); for line in reader.lines() { match line { @@ -82,7 +82,7 @@ pub async fn exec_from_lines( Ok(_) => {} Err(err) => eprintln!("{err}"), } - query = "".to_string(); + query = String::new(); } else { query.push('\n'); } diff --git a/datafusion-cli/src/functions.rs b/datafusion-cli/src/functions.rs index 0d7d8f33738fa..76c56d9029d49 100644 --- a/datafusion-cli/src/functions.rs +++ b/datafusion-cli/src/functions.rs @@ -665,8 +665,7 @@ impl TableFunctionImpl for StatisticsCacheFunc { { for (path, entry) in file_statistics_cache.list_entries() { path_arr.push(path.path.to_string()); - table_arr - .push(path.table.map_or_else(|| "".to_string(), |t| t.to_string())); + table_arr.push(path.table.map_or_else(String::new, |t| t.to_string())); file_modified_arr .push(Some(entry.value.meta.last_modified.timestamp_millis())); file_size_bytes_arr.push(entry.value.meta.size); diff --git a/datafusion-examples/examples/data_io/parquet_encrypted_with_kms.rs b/datafusion-examples/examples/data_io/parquet_encrypted_with_kms.rs index 8e92f465eafe9..6193c656149f6 100644 --- a/datafusion-examples/examples/data_io/parquet_encrypted_with_kms.rs +++ b/datafusion-examples/examples/data_io/parquet_encrypted_with_kms.rs @@ -200,7 +200,7 @@ async fn read_encrypted_with_sql(ctx: &SessionContext, table_path: &str) -> Resu extensions_options! { struct EncryptionConfig { /// Comma-separated list of columns to encrypt - pub encrypted_columns: String, default = "".to_owned() + pub encrypted_columns: String, default = String::new() } } diff --git a/datafusion/common/src/config.rs b/datafusion/common/src/config.rs index 347721c43d7cf..92ebddcfb20ed 100644 --- a/datafusion/common/src/config.rs +++ b/datafusion/common/src/config.rs @@ -2000,7 +2000,7 @@ config_namespace! { /// instead of being converted into a [`std::fmt::Error`] pub safe: bool, default = true /// Format string for nulls - pub null: String, default = "".into() + pub null: String, default = String::new() /// Date format for date arrays pub date_format: Option, default = Some("%Y-%m-%d".to_string()) /// Format for DateTime arrays @@ -3399,7 +3399,7 @@ impl Default for ConfigFileEncryptionProperties { config_namespace_with_hashmap! { pub struct ColumnEncryptionProperties { /// Per column encryption key - pub column_key_as_hex: String, default = "".to_string() + pub column_key_as_hex: String, default = String::new() /// Per column encryption key metadata pub column_metadata_as_hex: Option, default = None } @@ -3578,7 +3578,7 @@ pub struct ConfigFileDecryptionProperties { config_namespace_with_hashmap! { pub struct ColumnDecryptionProperties { /// Per column encryption key - pub column_key_as_hex: String, default = "".to_string() + pub column_key_as_hex: String, default = String::new() } } @@ -4548,7 +4548,7 @@ mod tests { let parsed_metadata = table_config.parquet.key_value_metadata.clone(); assert_eq!(parsed_metadata.get("should not exist1"), None); - assert_eq!(parsed_metadata.get("key1"), Some(&Some("".into()))); + assert_eq!(parsed_metadata.get("key1"), Some(&Some(String::new()))); assert_eq!(parsed_metadata.get("key2"), Some(&Some("value2".into()))); assert_eq!( parsed_metadata.get("key3"), diff --git a/datafusion/common/src/error.rs b/datafusion/common/src/error.rs index d1fcb50f73492..5e2df49d95530 100644 --- a/datafusion/common/src/error.rs +++ b/datafusion/common/src/error.rs @@ -555,11 +555,11 @@ impl DataFusionError { return format!("{}{}", Self::BACK_TRACE_SEP, back_trace); } - "".to_owned() + String::new() } #[cfg(not(feature = "backtrace"))] - "".to_owned() + String::new() } /// Return a [`DataFusionErrorBuilder`] to build a [`DataFusionError`] @@ -606,7 +606,7 @@ impl DataFusionError { pub fn message(&self) -> Cow<'_, str> { match *self { DataFusionError::ArrowError(ref desc, ref backtrace) => { - let backtrace = backtrace.clone().unwrap_or_else(|| "".to_owned()); + let backtrace = backtrace.clone().unwrap_or_else(String::new); Cow::Owned(format!("{desc}{backtrace}")) } #[cfg(feature = "parquet")] @@ -614,8 +614,7 @@ impl DataFusionError { DataFusionError::IoError(ref desc) => Cow::Owned(desc.to_string()), #[cfg(feature = "sql")] DataFusionError::SQL(ref desc, ref backtrace) => { - let backtrace: String = - backtrace.clone().unwrap_or_else(|| "".to_owned()); + let backtrace: String = backtrace.clone().unwrap_or_else(String::new); Cow::Owned(format!("{desc:?}{backtrace}")) } DataFusionError::Configuration(ref desc) => Cow::Owned(desc.to_string()), @@ -628,7 +627,7 @@ impl DataFusionError { DataFusionError::Plan(ref desc) => Cow::Owned(desc.to_string()), DataFusionError::SchemaError(ref desc, ref backtrace) => { let backtrace: &str = - &backtrace.as_ref().clone().unwrap_or_else(|| "".to_owned()); + &backtrace.as_ref().clone().unwrap_or_else(String::new); Cow::Owned(format!("{desc}{backtrace}")) } DataFusionError::Execution(ref desc) => Cow::Owned(desc.to_string()), diff --git a/datafusion/common/src/scalar/mod.rs b/datafusion/common/src/scalar/mod.rs index bad526a3a2227..07579c60b292a 100644 --- a/datafusion/common/src/scalar/mod.rs +++ b/datafusion/common/src/scalar/mod.rs @@ -1703,9 +1703,9 @@ impl ScalarValue { | DataType::Date64 => ScalarValue::new_zero(datatype), // String types - DataType::Utf8 => Ok(ScalarValue::Utf8(Some("".to_string()))), - DataType::LargeUtf8 => Ok(ScalarValue::LargeUtf8(Some("".to_string()))), - DataType::Utf8View => Ok(ScalarValue::Utf8View(Some("".to_string()))), + DataType::Utf8 => Ok(ScalarValue::Utf8(Some(String::new()))), + DataType::LargeUtf8 => Ok(ScalarValue::LargeUtf8(Some(String::new()))), + DataType::Utf8View => Ok(ScalarValue::Utf8View(Some(String::new()))), // Binary types DataType::Binary => Ok(ScalarValue::Binary(Some(vec![]))), @@ -5650,7 +5650,7 @@ impl fmt::Display for ScalarValue { match epoch.checked_add_signed(Duration::try_days(v as i64).unwrap()) { Some(date) => date.to_string(), - None => "".to_string(), + None => String::new(), } }) )?, @@ -5661,7 +5661,7 @@ impl fmt::Display for ScalarValue { match epoch.checked_add_signed(Duration::try_milliseconds(v).unwrap()) { Some(date) => date.to_string(), - None => "".to_string(), + None => String::new(), } }) )?, @@ -10640,11 +10640,11 @@ mod tests { // Test string types assert_eq!( ScalarValue::new_default(&DataType::Utf8).unwrap(), - ScalarValue::Utf8(Some("".to_string())) + ScalarValue::Utf8(Some(String::new())) ); assert_eq!( ScalarValue::new_default(&DataType::LargeUtf8).unwrap(), - ScalarValue::LargeUtf8(Some("".to_string())) + ScalarValue::LargeUtf8(Some(String::new())) ); // Test binary types diff --git a/datafusion/core/src/bin/print_functions_docs.rs b/datafusion/core/src/bin/print_functions_docs.rs index 10a259dd8b745..82ccf41750b19 100644 --- a/datafusion/core/src/bin/print_functions_docs.rs +++ b/datafusion/core/src/bin/print_functions_docs.rs @@ -93,7 +93,7 @@ fn print_docs( providers: Vec>, doc_sections: Vec, ) -> Result { - let mut docs = "".to_string(); + let mut docs = String::new(); // Ensure that all providers have documentation let mut providers_with_no_docs = HashSet::new(); diff --git a/datafusion/core/src/datasource/listing_table_factory.rs b/datafusion/core/src/datasource/listing_table_factory.rs index 1e597e38fb5b1..5ebe0882befa4 100644 --- a/datafusion/core/src/datasource/listing_table_factory.rs +++ b/datafusion/core/src/datasource/listing_table_factory.rs @@ -332,7 +332,7 @@ fn get_extension(path: &str) -> String { let res = Path::new(path).extension().and_then(|ext| ext.to_str()); match res { Some(ext) => format!(".{ext}"), - None => "".to_string(), + None => String::new(), } } diff --git a/datafusion/core/src/datasource/physical_plan/csv.rs b/datafusion/core/src/datasource/physical_plan/csv.rs index 7980df87fa576..361e36b214341 100644 --- a/datafusion/core/src/datasource/physical_plan/csv.rs +++ b/datafusion/core/src/datasource/physical_plan/csv.rs @@ -764,7 +764,7 @@ mod tests { // get name of first part let paths = fs::read_dir(&out_dir).unwrap(); - let mut part_0_name: String = "".to_owned(); + let mut part_0_name: String = String::new(); for path in paths { let path = path.unwrap(); let name = path diff --git a/datafusion/core/src/datasource/physical_plan/json.rs b/datafusion/core/src/datasource/physical_plan/json.rs index 6b4361e0c4d07..0309a5ae4bcfb 100644 --- a/datafusion/core/src/datasource/physical_plan/json.rs +++ b/datafusion/core/src/datasource/physical_plan/json.rs @@ -410,7 +410,7 @@ mod tests { // get name of first part let paths = fs::read_dir(&out_dir).unwrap(); - let mut part_0_name: String = "".to_owned(); + let mut part_0_name: String = String::new(); for path in paths { let name = path .unwrap() diff --git a/datafusion/core/tests/fuzz_cases/aggregation_fuzzer/query_builder.rs b/datafusion/core/tests/fuzz_cases/aggregation_fuzzer/query_builder.rs index d32078ec6331f..6b1654ad70080 100644 --- a/datafusion/core/tests/fuzz_cases/aggregation_fuzzer/query_builder.rs +++ b/datafusion/core/tests/fuzz_cases/aggregation_fuzzer/query_builder.rs @@ -306,7 +306,7 @@ impl QueryBuilder { self.null_opt(), ) } else { - ("".to_string(), "".to_string()) + (String::new(), String::new()) }; let function = format!( diff --git a/datafusion/core/tests/fuzz_cases/window_fuzz.rs b/datafusion/core/tests/fuzz_cases/window_fuzz.rs index f69b5e9a41b02..b7a423e59f7a9 100644 --- a/datafusion/core/tests/fuzz_cases/window_fuzz.rs +++ b/datafusion/core/tests/fuzz_cases/window_fuzz.rs @@ -768,7 +768,7 @@ pub(crate) fn make_staggered_batches( let mut rng = StdRng::seed_from_u64(random_seed); let mut input123: Vec<(i32, i32, i32)> = vec![(0, 0, 0); len]; let mut input4: Vec = vec![0; len]; - let mut input5: Vec = vec!["".to_string(); len]; + let mut input5: Vec = vec![String::new(); len]; for v in &mut input123 { *v = ( rng.random_range(0..n_distinct) as i32, diff --git a/datafusion/datasource/src/file_compression_type.rs b/datafusion/datasource/src/file_compression_type.rs index 89efb580652b1..cad89c880ba58 100644 --- a/datafusion/datasource/src/file_compression_type.rs +++ b/datafusion/datasource/src/file_compression_type.rs @@ -65,7 +65,7 @@ impl GetExt for FileCompressionType { BZIP2 => ".bz2".to_owned(), XZ => ".xz".to_owned(), ZSTD => ".zst".to_owned(), - UNCOMPRESSED => "".to_owned(), + UNCOMPRESSED => String::new(), } } } diff --git a/datafusion/expr-common/src/casts.rs b/datafusion/expr-common/src/casts.rs index 3518c02772672..2677d6bd7353f 100644 --- a/datafusion/expr-common/src/casts.rs +++ b/datafusion/expr-common/src/casts.rs @@ -1478,9 +1478,9 @@ mod tests { // Test empty string expect_cast( - ScalarValue::Utf8(Some("".to_string())), + ScalarValue::Utf8(Some(String::new())), DataType::Utf8View, - ExpectedCast::Value(ScalarValue::Utf8View(Some("".to_string()))), + ExpectedCast::Value(ScalarValue::Utf8View(Some(String::new()))), ); // Test large string diff --git a/datafusion/expr/src/expr_schema.rs b/datafusion/expr/src/expr_schema.rs index 75b3a60af1465..8819c7a95e795 100644 --- a/datafusion/expr/src/expr_schema.rs +++ b/datafusion/expr/src/expr_schema.rs @@ -1244,7 +1244,7 @@ mod tests { let placeholder_meta = FieldMetadata::from(placeholder_meta); let expr = Expr::Placeholder(Placeholder::new_with_field( - "".to_string(), + String::new(), Some( Field::new("", DataType::Utf8, true) .with_metadata(placeholder_meta.to_hashmap()) @@ -1269,7 +1269,7 @@ mod tests { // Non-nullable placeholder field should remain non-nullable let expr = Expr::Placeholder(Placeholder::new_with_field( - "".to_string(), + String::new(), Some(Field::new("", DataType::Utf8, false).into()), )); let expr_field = expr.to_field(&schema).unwrap().1; diff --git a/datafusion/expr/src/logical_plan/display.rs b/datafusion/expr/src/logical_plan/display.rs index 3009253f53d11..0710c8b4d7ad0 100644 --- a/datafusion/expr/src/logical_plan/display.rs +++ b/datafusion/expr/src/logical_plan/display.rs @@ -485,7 +485,7 @@ impl<'a, 'b> PgJsonVisitor<'a, 'b> { let filter_expr = filter .as_ref() .map(|expr| format!(" Filter: {expr}")) - .unwrap_or_else(|| "".to_string()); + .unwrap_or_else(String::new); json!({ "Node Type": format!("{} Join", join_type), "Join Constraint": format!("{:?}", join_constraint), diff --git a/datafusion/expr/src/logical_plan/plan.rs b/datafusion/expr/src/logical_plan/plan.rs index 649c82a16ad88..c3a9d43b799ec 100644 --- a/datafusion/expr/src/logical_plan/plan.rs +++ b/datafusion/expr/src/logical_plan/plan.rs @@ -2117,7 +2117,7 @@ impl LogicalPlan { .collect(); format!(" projection=[{}]", names.join(", ")) } - _ => "".to_string(), + _ => String::new(), }; write!(f, "TableScan: {table_name}{projected_fields}")?; @@ -2257,7 +2257,7 @@ impl LogicalPlan { let filter_expr = filter .as_ref() .map(|expr| format!(" Filter: {expr}")) - .unwrap_or_else(|| "".to_string()); + .unwrap_or_else(String::new); let null_aware_expr = if *null_aware { " null_aware" } else { "" }; let join_type = if filter.is_none() @@ -2386,7 +2386,7 @@ impl LogicalPlan { if let Some(sort_expr) = sort_expr { expr_vec_fmt!(sort_expr) } else { - "".to_string() + String::new() }, ), }, @@ -6265,7 +6265,7 @@ mod tests { .unwrap(); let prepared_builder = LogicalPlanBuilder::new(plan) .prepare( - "".to_string(), + String::new(), vec![Field::new("", DataType::Int32, true).into()], ) .unwrap(); diff --git a/datafusion/functions/src/regex/regexpcount.rs b/datafusion/functions/src/regex/regexpcount.rs index 2920b687ed33f..e19c4ea358220 100644 --- a/datafusion/functions/src/regex/regexpcount.rs +++ b/datafusion/functions/src/regex/regexpcount.rs @@ -674,7 +674,7 @@ mod tests { let re = regexp_count_with_scalar_values(&[ ScalarValue::Utf8(Some(value.to_string())), - ScalarValue::Utf8(Some("".to_string())), + ScalarValue::Utf8(Some(String::new())), start_sv.clone(), ]); match re { @@ -686,7 +686,7 @@ mod tests { let re = regexp_count_with_scalar_values(&[ ScalarValue::LargeUtf8(Some(value.to_string())), - ScalarValue::LargeUtf8(Some("".to_string())), + ScalarValue::LargeUtf8(Some(String::new())), start_sv.clone(), ]); match re { @@ -698,7 +698,7 @@ mod tests { let re = regexp_count_with_scalar_values(&[ ScalarValue::Utf8View(Some(value.to_string())), - ScalarValue::Utf8View(Some("".to_string())), + ScalarValue::Utf8View(Some(String::new())), start_sv, ]); match re { diff --git a/datafusion/functions/src/string/ascii.rs b/datafusion/functions/src/string/ascii.rs index db539a4d11719..8e99fb66afad2 100644 --- a/datafusion/functions/src/string/ascii.rs +++ b/datafusion/functions/src/string/ascii.rs @@ -242,7 +242,7 @@ mod tests { fn test_functions() -> Result<()> { test_ascii!(Some(String::from("x")), Ok(Some(120))); test_ascii!(Some(String::from("a")), Ok(Some(97))); - test_ascii!(Some(String::from("")), Ok(Some(0))); + test_ascii!(Some(String::new()), Ok(Some(0))); test_ascii!(Some(String::from("🚀")), Ok(Some(128640))); test_ascii!(Some(String::from("\n")), Ok(Some(10))); test_ascii!(Some(String::from("\t")), Ok(Some(9))); diff --git a/datafusion/functions/src/string/concat.rs b/datafusion/functions/src/string/concat.rs index aa42d918eb4b7..1396a0108a8b3 100644 --- a/datafusion/functions/src/string/concat.rs +++ b/datafusion/functions/src/string/concat.rs @@ -319,7 +319,7 @@ pub(crate) fn simplify_concat(args: Vec) -> Result { } let mut new_args = Vec::with_capacity(args.len()); - let mut contiguous_scalar = "".to_string(); + let mut contiguous_scalar = String::new(); let return_type = { let data_types: Vec<_> = args @@ -369,7 +369,7 @@ pub(crate) fn simplify_concat(args: Vec) -> Result { .push(lit(ScalarValue::Utf8View(Some(contiguous_scalar)))), _ => unreachable!(), } - contiguous_scalar = "".to_string(); + contiguous_scalar = String::new(); } new_args.push(arg); } diff --git a/datafusion/functions/src/string/octet_length.rs b/datafusion/functions/src/string/octet_length.rs index 02df262ee27aa..73d52140d6573 100644 --- a/datafusion/functions/src/string/octet_length.rs +++ b/datafusion/functions/src/string/octet_length.rs @@ -185,9 +185,9 @@ mod tests { ); test_function!( OctetLengthFunc::new(), - vec![ColumnarValue::Scalar(ScalarValue::Utf8(Some( - String::from("") - )))], + vec![ColumnarValue::Scalar(ScalarValue::Utf8( + Some(String::new()) + ))], Ok(Some(0)), i32, Int32, @@ -224,7 +224,7 @@ mod tests { test_function!( OctetLengthFunc::new(), vec![ColumnarValue::Scalar(ScalarValue::Utf8View(Some( - String::from("") + String::new() )))], Ok(Some(0)), i32, diff --git a/datafusion/functions/src/string/replace.rs b/datafusion/functions/src/string/replace.rs index 549b8e1a3b0f9..0f84ed16e446c 100644 --- a/datafusion/functions/src/string/replace.rs +++ b/datafusion/functions/src/string/replace.rs @@ -429,7 +429,7 @@ mod tests { ReplaceFunc::new(), vec![ ColumnarValue::Scalar(ScalarValue::LargeUtf8(Some(String::from("abc")))), - ColumnarValue::Scalar(ScalarValue::LargeUtf8(Some(String::from("")))), + ColumnarValue::Scalar(ScalarValue::LargeUtf8(Some(String::new()))), ColumnarValue::Scalar(ScalarValue::LargeUtf8(Some(String::from("x")))), ], Ok(Some("abc")), diff --git a/datafusion/functions/src/string/split_part.rs b/datafusion/functions/src/string/split_part.rs index 9b73a1af88501..d11bb90c13e9b 100644 --- a/datafusion/functions/src/string/split_part.rs +++ b/datafusion/functions/src/string/split_part.rs @@ -747,7 +747,7 @@ mod tests { SplitPartFunc::new(), vec![ ColumnarValue::Scalar(ScalarValue::Utf8(Some(String::from("a,b")))), - ColumnarValue::Scalar(ScalarValue::Utf8(Some(String::from("")))), + ColumnarValue::Scalar(ScalarValue::Utf8(Some(String::new()))), ColumnarValue::Scalar(ScalarValue::Int64(Some(1))), ], Ok(Some("a,b")), @@ -759,7 +759,7 @@ mod tests { SplitPartFunc::new(), vec![ ColumnarValue::Scalar(ScalarValue::Utf8(Some(String::from("a,b")))), - ColumnarValue::Scalar(ScalarValue::Utf8(Some(String::from("")))), + ColumnarValue::Scalar(ScalarValue::Utf8(Some(String::new()))), ColumnarValue::Scalar(ScalarValue::Int64(Some(2))), ], Ok(Some("")), @@ -797,7 +797,7 @@ mod tests { SplitPartFunc::new(), vec![ ColumnarValue::Scalar(ScalarValue::Utf8(Some(String::from("a,b")))), - ColumnarValue::Scalar(ScalarValue::Utf8(Some(String::from("")))), + ColumnarValue::Scalar(ScalarValue::Utf8(Some(String::new()))), ColumnarValue::Scalar(ScalarValue::Int64(Some(-1))), ], Ok(Some("a,b")), @@ -821,7 +821,7 @@ mod tests { SplitPartFunc::new(), vec![ ColumnarValue::Scalar(ScalarValue::Utf8(Some(String::from("a,b")))), - ColumnarValue::Scalar(ScalarValue::Utf8(Some(String::from("")))), + ColumnarValue::Scalar(ScalarValue::Utf8(Some(String::new()))), ColumnarValue::Scalar(ScalarValue::Int64(Some(-2))), ], Ok(Some("")), diff --git a/datafusion/functions/src/unicode/character_length.rs b/datafusion/functions/src/unicode/character_length.rs index 9f0d952a02636..e92ab2b494a1a 100644 --- a/datafusion/functions/src/unicode/character_length.rs +++ b/datafusion/functions/src/unicode/character_length.rs @@ -228,7 +228,7 @@ mod tests { test_character_length!(Some(String::from("josé")), Ok(Some(4))); // test long strings (more than 12 bytes for StringView) test_character_length!(Some(String::from("joséjoséjoséjosé")), Ok(Some(16))); - test_character_length!(Some(String::from("")), Ok(Some(0))); + test_character_length!(Some(String::new()), Ok(Some(0))); test_character_length!(None, Ok(None)); } diff --git a/datafusion/functions/src/unicode/find_in_set.rs b/datafusion/functions/src/unicode/find_in_set.rs index 5378aaf714f4d..b7156569ab32d 100644 --- a/datafusion/functions/src/unicode/find_in_set.rs +++ b/datafusion/functions/src/unicode/find_in_set.rs @@ -443,7 +443,7 @@ mod tests { test_function!( FindInSetFunc::new(), vec![ - ColumnarValue::Scalar(ScalarValue::Utf8(Some(String::from("")))), + ColumnarValue::Scalar(ScalarValue::Utf8(Some(String::new()))), ColumnarValue::Scalar(ScalarValue::Utf8(Some(String::from("a,b,c")))), ], Ok(Some(0)), @@ -455,7 +455,7 @@ mod tests { FindInSetFunc::new(), vec![ ColumnarValue::Scalar(ScalarValue::Utf8(Some(String::from("a")))), - ColumnarValue::Scalar(ScalarValue::Utf8(Some(String::from("")))), + ColumnarValue::Scalar(ScalarValue::Utf8(Some(String::new()))), ], Ok(Some(0)), i32, diff --git a/datafusion/functions/src/unicode/initcap.rs b/datafusion/functions/src/unicode/initcap.rs index 0332ab5d4427f..ddbfa3eb1936a 100644 --- a/datafusion/functions/src/unicode/initcap.rs +++ b/datafusion/functions/src/unicode/initcap.rs @@ -398,7 +398,7 @@ mod tests { test_function!( InitcapFunc::new(), vec![ColumnarValue::Scalar(ScalarValue::Utf8View(Some( - "".to_string() + String::new() )))], Ok(Some("")), &str, diff --git a/datafusion/functions/src/unicode/left.rs b/datafusion/functions/src/unicode/left.rs index 0788e69d92528..bbf7d9ac3554f 100644 --- a/datafusion/functions/src/unicode/left.rs +++ b/datafusion/functions/src/unicode/left.rs @@ -281,7 +281,7 @@ mod tests { test_function!( LeftFunc::new(), vec![ - ColumnarValue::Scalar(ScalarValue::Utf8View(Some("".to_string()))), + ColumnarValue::Scalar(ScalarValue::Utf8View(Some(String::new()))), ColumnarValue::Scalar(ScalarValue::from(200i64)), ], Ok(Some("")), diff --git a/datafusion/functions/src/unicode/lpad.rs b/datafusion/functions/src/unicode/lpad.rs index 0ffd02714957c..32932697844ee 100644 --- a/datafusion/functions/src/unicode/lpad.rs +++ b/datafusion/functions/src/unicode/lpad.rs @@ -725,7 +725,7 @@ mod tests { test_lpad!( Some("hi".into()), ScalarValue::Int64(Some(5i64)), - Some("".into()), + Some(String::new()), Ok(Some("hi")) ); test_lpad!( diff --git a/datafusion/functions/src/unicode/right.rs b/datafusion/functions/src/unicode/right.rs index 21fb0690a11a2..77d155cbe5aeb 100644 --- a/datafusion/functions/src/unicode/right.rs +++ b/datafusion/functions/src/unicode/right.rs @@ -281,7 +281,7 @@ mod tests { test_function!( RightFunc::new(), vec![ - ColumnarValue::Scalar(ScalarValue::Utf8View(Some("".to_string()))), + ColumnarValue::Scalar(ScalarValue::Utf8View(Some(String::new()))), ColumnarValue::Scalar(ScalarValue::from(200i64)), ], Ok(Some("")), diff --git a/datafusion/physical-plan/src/display.rs b/datafusion/physical-plan/src/display.rs index 44522e76afa31..404be3c210e44 100644 --- a/datafusion/physical-plan/src/display.rs +++ b/datafusion/physical-plan/src/display.rs @@ -681,7 +681,7 @@ impl ExecutionPlanVisitor for GraphvizVisitor<'_, '_> { let label = { format!("{}", Wrapper(plan, self.t)) }; let metrics = match self.show_metrics { - ShowMetrics::None => "".to_string(), + ShowMetrics::None => String::new(), ShowMetrics::Aggregated => { if let Some(metrics) = plan.metrics() { let mut metrics = metrics @@ -722,7 +722,7 @@ impl ExecutionPlanVisitor for GraphvizVisitor<'_, '_> { .map_err(|_e| fmt::Error)?; format!("statistics=[{stats}]") } else { - "".to_string() + String::new() }; let delimiter = if !metrics.is_empty() && !statistics.is_empty() { diff --git a/datafusion/physical-plan/src/filter.rs b/datafusion/physical-plan/src/filter.rs index 12771eec78470..45c6dc72d374a 100644 --- a/datafusion/physical-plan/src/filter.rs +++ b/datafusion/physical-plan/src/filter.rs @@ -521,11 +521,11 @@ impl DisplayAs for FilterExec { .join(", ") ) } else { - "".to_string() + String::new() }; let fetch = self .fetch - .map_or_else(|| "".to_string(), |f| format!(", fetch={f}")); + .map_or_else(String::new, |f| format!(", fetch={f}")); write!( f, "FilterExec: {}{}{}", diff --git a/datafusion/physical-plan/src/joins/hash_join/exec.rs b/datafusion/physical-plan/src/joins/hash_join/exec.rs index 24b70a22e37e5..94875cb6189aa 100644 --- a/datafusion/physical-plan/src/joins/hash_join/exec.rs +++ b/datafusion/physical-plan/src/joins/hash_join/exec.rs @@ -1319,10 +1319,10 @@ impl DisplayAs for HashJoinExec { fn fmt_as(&self, t: DisplayFormatType, f: &mut fmt::Formatter) -> fmt::Result { match t { DisplayFormatType::Default | DisplayFormatType::Verbose => { - let display_filter = self.filter.as_ref().map_or_else( - || "".to_string(), - |f| format!(", filter={}", f.expression()), - ); + let display_filter = self + .filter + .as_ref() + .map_or_else(String::new, |f| format!(", filter={}", f.expression())); let display_projections = if self.contains_projection() { format!( ", projection=[{}]", @@ -1339,7 +1339,7 @@ impl DisplayAs for HashJoinExec { .join(", ") ) } else { - "".to_string() + String::new() }; let display_null_equality = if self.null_equality() == NullEquality::NullEqualsNull { diff --git a/datafusion/physical-plan/src/joins/nested_loop_join.rs b/datafusion/physical-plan/src/joins/nested_loop_join.rs index fd2e0ae21c2ea..d22270c3550b5 100644 --- a/datafusion/physical-plan/src/joins/nested_loop_join.rs +++ b/datafusion/physical-plan/src/joins/nested_loop_join.rs @@ -504,10 +504,10 @@ impl DisplayAs for NestedLoopJoinExec { fn fmt_as(&self, t: DisplayFormatType, f: &mut Formatter) -> std::fmt::Result { match t { DisplayFormatType::Default | DisplayFormatType::Verbose => { - let display_filter = self.filter.as_ref().map_or_else( - || "".to_string(), - |f| format!(", filter={}", f.expression()), - ); + let display_filter = self + .filter + .as_ref() + .map_or_else(String::new, |f| format!(", filter={}", f.expression())); let display_projections = if self.contains_projection() { format!( ", projection=[{}]", @@ -524,7 +524,7 @@ impl DisplayAs for NestedLoopJoinExec { .join(", ") ) } else { - "".to_string() + String::new() }; write!( f, diff --git a/datafusion/physical-plan/src/joins/sort_merge_join/exec.rs b/datafusion/physical-plan/src/joins/sort_merge_join/exec.rs index 911eca0a97928..55d02bccef1c9 100644 --- a/datafusion/physical-plan/src/joins/sort_merge_join/exec.rs +++ b/datafusion/physical-plan/src/joins/sort_merge_join/exec.rs @@ -433,10 +433,10 @@ impl DisplayAs for SortMergeJoinExec { Self::static_name(), self.join_type, on, - self.filter.as_ref().map_or_else( - || "".to_string(), - |f| format!(", filter={}", f.expression()) - ), + self.filter.as_ref().map_or_else(String::new, |f| format!( + ", filter={}", + f.expression() + )), display_null_equality, display_projections, ) diff --git a/datafusion/physical-plan/src/joins/symmetric_hash_join.rs b/datafusion/physical-plan/src/joins/symmetric_hash_join.rs index 99a12796c688e..5665ddef0f9e3 100644 --- a/datafusion/physical-plan/src/joins/symmetric_hash_join.rs +++ b/datafusion/physical-plan/src/joins/symmetric_hash_join.rs @@ -368,10 +368,10 @@ impl DisplayAs for SymmetricHashJoinExec { fn fmt_as(&self, t: DisplayFormatType, f: &mut fmt::Formatter) -> fmt::Result { match t { DisplayFormatType::Default | DisplayFormatType::Verbose => { - let display_filter = self.filter.as_ref().map_or_else( - || "".to_string(), - |f| format!(", filter={}", f.expression()), - ); + let display_filter = self + .filter + .as_ref() + .map_or_else(String::new, |f| format!(", filter={}", f.expression())); let on = self .on .iter() diff --git a/datafusion/physical-plan/src/render_tree.rs b/datafusion/physical-plan/src/render_tree.rs index 40e2763698093..2fb220df8aa33 100644 --- a/datafusion/physical-plan/src/render_tree.rs +++ b/datafusion/physical-plan/src/render_tree.rs @@ -204,7 +204,7 @@ fn create_tree_recursive( if let Some((key, value)) = line.split_once('=') { extra_info.insert(key.to_string(), value.to_string()); } else { - extra_info.insert(line.to_string(), "".to_string()); + extra_info.insert(line.to_string(), String::new()); } } diff --git a/datafusion/proto/tests/cases/roundtrip_logical_plan.rs b/datafusion/proto/tests/cases/roundtrip_logical_plan.rs index 750b20323ad2e..65894f2d437a5 100644 --- a/datafusion/proto/tests/cases/roundtrip_logical_plan.rs +++ b/datafusion/proto/tests/cases/roundtrip_logical_plan.rs @@ -1673,7 +1673,7 @@ async fn roundtrip_logical_plan_prepared_statement_with_metadata() -> Result<()> .unwrap(); let prepared = LogicalPlanBuilder::new(plan) .prepare( - "".to_string(), + String::new(), vec![ Field::new("", DataType::Int32, true) .with_metadata( diff --git a/datafusion/spark/src/function/string/char.rs b/datafusion/spark/src/function/string/char.rs index 5d6de3ae368e3..0f6efd23ee4d0 100644 --- a/datafusion/spark/src/function/string/char.rs +++ b/datafusion/spark/src/function/string/char.rs @@ -86,9 +86,9 @@ fn spark_chr(args: &[ColumnarValue]) -> Result { } ColumnarValue::Scalar(ScalarValue::Int64(Some(value))) => { if value < 0 { - Ok(ColumnarValue::Scalar(ScalarValue::Utf8(Some( - "".to_string(), - )))) + Ok(ColumnarValue::Scalar(ScalarValue::Utf8( + Some(String::new()), + ))) } else { match core::char::from_u32((value % 256) as u32) { Some(ch) => Ok(ColumnarValue::Scalar(ScalarValue::Utf8(Some( diff --git a/datafusion/spark/src/function/string/format_string.rs b/datafusion/spark/src/function/string/format_string.rs index 131d1c14dfe5b..d7c1039623542 100644 --- a/datafusion/spark/src/function/string/format_string.rs +++ b/datafusion/spark/src/function/string/format_string.rs @@ -1817,13 +1817,13 @@ impl ConversionSpecifier { let (prefix, suffix) = if negative && self.negative_in_parentheses { ("(".to_owned(), ")".to_owned()) } else if negative { - ("-".to_owned(), "".to_owned()) + ("-".to_owned(), String::new()) } else if self.force_sign { - ("+".to_owned(), "".to_owned()) + ("+".to_owned(), String::new()) } else if self.space_sign { - (" ".to_owned(), "".to_owned()) + (" ".to_owned(), String::new()) } else { - ("".to_owned(), "".to_owned()) + (String::new(), String::new()) }; self.format_decimal_integer(writer, abs_val, prefix, &suffix); diff --git a/datafusion/spark/src/function/string/length.rs b/datafusion/spark/src/function/string/length.rs index 8c5539a0577d8..8e19e84edfc4c 100644 --- a/datafusion/spark/src/function/string/length.rs +++ b/datafusion/spark/src/function/string/length.rs @@ -270,7 +270,7 @@ mod tests { test_spark_length_string!(Some(String::from("josé")), Ok(Some(4))); // test long strings (more than 12 bytes for StringView) test_spark_length_string!(Some(String::from("joséjoséjoséjosé")), Ok(Some(16))); - test_spark_length_string!(Some(String::from("")), Ok(Some(0))); + test_spark_length_string!(Some(String::new()), Ok(Some(0))); test_spark_length_string!(None, Ok(None)); test_spark_length_binary!(Some(String::from("chars").into_bytes()), Ok(Some(5))); @@ -280,7 +280,7 @@ mod tests { Some(String::from("joséjoséjoséjosé").into_bytes()), Ok(Some(20)) ); - test_spark_length_binary!(Some(String::from("").into_bytes()), Ok(Some(0))); + test_spark_length_binary!(Some(String::new().into_bytes()), Ok(Some(0))); test_spark_length_binary!(None, Ok(None)); Ok(()) diff --git a/datafusion/spark/src/function/url/parse_url.rs b/datafusion/spark/src/function/url/parse_url.rs index c385f43e49343..bed23c24f4276 100644 --- a/datafusion/spark/src/function/url/parse_url.rs +++ b/datafusion/spark/src/function/url/parse_url.rs @@ -404,7 +404,7 @@ mod tests { fn test_parse_path_empty_vs_root() -> Result<()> { assert_eq!( ParseUrl::parse("https://example.com", "PATH", None)?, - Some("".to_string()) + Some(String::new()) ); assert_eq!( ParseUrl::parse("https://example.com/", "PATH", None)?, @@ -430,7 +430,7 @@ mod tests { ); assert_eq!( ParseUrl::parse("http://ex.com?key=", "QUERY", Some("key"))?, - Some("".to_string()) + Some(String::new()) ); assert_eq!( ParseUrl::parse("http://ex.com?keyonly", "QUERY", Some("keyonly"))?, @@ -449,10 +449,10 @@ mod tests { #[test] fn test_parse_empty_path_file() -> Result<()> { - assert_eq!(ParseUrl::parse("", "PATH", None)?, Some("".to_string())); + assert_eq!(ParseUrl::parse("", "PATH", None)?, Some(String::new())); assert_eq!( ParseUrl::parse("http://example.com", "FILE", None)?, - Some("".to_string()) + Some(String::new()) ); assert_eq!( ParseUrl::parse("http://example.com?foo=bar", "FILE", None)?, @@ -460,7 +460,7 @@ mod tests { ); assert_eq!( ParseUrl::parse("http://example.com#fragment", "FILE", None)?, - Some("".to_string()) + Some(String::new()) ); assert_eq!( ParseUrl::parse("http://example.com/?foo=bar", "FILE", None)?, diff --git a/datafusion/sql/src/statement.rs b/datafusion/sql/src/statement.rs index 1a9072212f2f3..7c7e5edfa53bc 100644 --- a/datafusion/sql/src/statement.rs +++ b/datafusion/sql/src/statement.rs @@ -1412,7 +1412,7 @@ impl SqlToRel<'_, S> { .map(|t| { let name = match t.name.clone() { Some(name) => name.value, - None => "".to_string(), + None => String::new(), }; Arc::new(Field::new(name, t.data_type.clone(), true)) }) @@ -3036,7 +3036,7 @@ impl SqlToRel<'_, S> { _ => return plan_err!("Unsupported SHOW FUNCTIONS filter"), } } else { - "".to_string() + String::new() }; // Scalar / aggregate / window functions are resolved by joining diff --git a/test-utils/src/array_gen/string.rs b/test-utils/src/array_gen/string.rs index 896182290ccca..cfc99e2ee7a64 100644 --- a/test-utils/src/array_gen/string.rs +++ b/test-utils/src/array_gen/string.rs @@ -92,7 +92,7 @@ impl StringArrayGenerator { fn random_string(rng: &mut StdRng, max_len: usize) -> String { // pick characters at random (not just ascii) match max_len { - 0 => "".to_string(), + 0 => String::new(), 1 => String::from(rng.random::()), _ => { let len = rng.random_range(1..=max_len); From a6de719a515d2ac94caec97f0c19c820e956b077 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 1 Sep 2026 13:44:21 +0200 Subject: [PATCH 2/6] Enable clippy::ignored_unit_patterns `Ok(())` instead of `Ok(_)` where the payload is `()`, so the pattern stops matching silently if the type ever gains a payload. --- Cargo.toml | 1 - benchmarks/src/cancellation.rs | 4 ++-- benchmarks/src/sql_benchmark.rs | 12 ++++++------ datafusion-cli/src/exec.rs | 4 ++-- datafusion/core/src/datasource/file_format/avro.rs | 2 +- datafusion/core/src/datasource/file_format/csv.rs | 2 +- datafusion/core/src/datasource/file_format/json.rs | 2 +- .../core/src/datasource/file_format/parquet.rs | 2 +- datafusion/core/tests/execution/coop.rs | 4 ++-- datafusion/datasource/src/write/orchestration.rs | 4 ++-- datafusion/execution/src/async_stream.rs | 10 +++++----- datafusion/expr/src/expr_schema.rs | 4 ++-- datafusion/expr/src/type_coercion/functions.rs | 8 ++++---- datafusion/physical-expr-common/src/binary_map.rs | 8 ++++---- .../physical-expr-common/src/binary_view_map.rs | 6 +++--- datafusion/physical-expr/src/expressions/case.rs | 2 +- .../physical-plan/src/aggregates/aggregate_stream.rs | 2 +- .../src/aggregates/grouped_hash_stream.rs | 2 +- datafusion/physical-plan/src/buffer.rs | 2 +- datafusion/physical-plan/src/joins/asof_join.rs | 2 +- .../joins/sort_merge_join/materializing_stream.rs | 2 +- datafusion/physical-plan/src/repartition/mod.rs | 2 +- .../physical-plan/src/sorts/multi_level_merge.rs | 2 +- datafusion/physical-plan/src/sorts/sort.rs | 2 +- datafusion/physical-plan/src/spill/spill_pool.rs | 2 +- datafusion/proto/src/logical_plan/from_proto.rs | 2 +- datafusion/proto/src/physical_plan/mod.rs | 4 ++-- datafusion/proto/src/physical_plan/to_proto.rs | 2 +- datafusion/sql/src/expr/function.rs | 2 +- .../substrait/tests/cases/roundtrip_logical_plan.rs | 2 +- 30 files changed, 52 insertions(+), 53 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 9bbfbffce268b..d2b56a91a620f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -283,7 +283,6 @@ explicit_iter_loop = "allow" # 189 hits float_cmp = "allow" # 8 hits; exact float comparisons are often intentional here from_iter_instead_of_collect = "allow" # 51 hits if_not_else = "allow" # 133 hits -ignored_unit_patterns = "allow" # 52 hits implicit_clone = "allow" # 198 hits implicit_hasher = "allow" # 17 hits; some sites feed arrow APIs that require the default hasher inline_always = "allow" # 45 hits diff --git a/benchmarks/src/cancellation.rs b/benchmarks/src/cancellation.rs index 5f7fdcc43d99d..47bd76e80fc0d 100644 --- a/benchmarks/src/cancellation.rs +++ b/benchmarks/src/cancellation.rs @@ -127,12 +127,12 @@ fn run_test(wait_time: u64, store: Arc) -> Result { let store = Arc::clone(&store); tokio::select! { biased; - _ = async move { + () = async move { datafusion(store).await.unwrap(); } => { println!("matched case doing work"); }, - _ = captured_token.cancelled() => { + () = captured_token.cancelled() => { println!("Received shutdown request"); return; }, diff --git a/benchmarks/src/sql_benchmark.rs b/benchmarks/src/sql_benchmark.rs index 24db7e0a0fb2e..3959b1211560a 100644 --- a/benchmarks/src/sql_benchmark.rs +++ b/benchmarks/src/sql_benchmark.rs @@ -509,7 +509,7 @@ impl SqlBenchmark { while let Some(result) = reader_result { match result { - Ok(_) => { + Ok(()) => { if !is_blank_or_comment_line(&line) { // boxing required because of recursion Box::pin(self.process_line(ctx, &mut reader, &mut line)).await?; @@ -824,7 +824,7 @@ impl BenchmarkDirective { loop { match reader_result { - Some(Ok(_)) => { + Some(Ok(())) => { if is_comment_line(line) { // comment, ignore } else if is_blank_line(line) { @@ -956,7 +956,7 @@ impl BenchmarkDirective { loop { match reader_result { - Some(Ok(_)) => { + Some(Ok(())) => { if line.trim() == "----" { found_break = true; break; @@ -1045,7 +1045,7 @@ impl BenchmarkDirective { loop { match reader_result { - Some(Ok(_)) => { + Some(Ok(())) => { if line.trim() == "----" { found_break = true; break; @@ -1108,7 +1108,7 @@ impl BenchmarkDirective { loop { match reader_result { - Some(Ok(_)) => { + Some(Ok(())) => { if is_comment_line(line) { // Clear the line buffer for the next iteration. line.clear(); @@ -1449,7 +1449,7 @@ fn read_query_from_reader( loop { match reader_result { - Some(Ok(_)) => { + Some(Ok(())) => { if is_comment_line(&line) { // comment, ignore } else if is_blank_line(&line) { diff --git a/datafusion-cli/src/exec.rs b/datafusion-cli/src/exec.rs index 7a749386dbbd0..c0c086e874760 100644 --- a/datafusion-cli/src/exec.rs +++ b/datafusion-cli/src/exec.rs @@ -79,7 +79,7 @@ pub async fn exec_from_lines( query.push_str(line); if line.ends_with(';') { match exec_and_print(ctx, print_options, query).await { - Ok(_) => {} + Ok(()) => {} Err(err) => eprintln!("{err}"), } query = String::new(); @@ -175,7 +175,7 @@ pub async fn exec_from_repl( rl.add_history_entry(line.trim_end())?; tokio::select! { res = exec_and_print(ctx, print_options, line) => match res { - Ok(_) => {} + Ok(()) => {} Err(err) => eprintln!("{err}"), }, _ = signal::ctrl_c() => { diff --git a/datafusion/core/src/datasource/file_format/avro.rs b/datafusion/core/src/datasource/file_format/avro.rs index a8b48cc736c92..14c79f06ff631 100644 --- a/datafusion/core/src/datasource/file_format/avro.rs +++ b/datafusion/core/src/datasource/file_format/avro.rs @@ -60,7 +60,7 @@ mod tests { assert_eq!(11, batch.num_columns()); assert_eq!(2, batch.num_rows()); }) - .fold(0, |acc, _| async move { acc + 1i32 }) + .fold(0, |acc, ()| async move { acc + 1i32 }) .await; assert_eq!(tt_batches, 4 /* 8/2 */); diff --git a/datafusion/core/src/datasource/file_format/csv.rs b/datafusion/core/src/datasource/file_format/csv.rs index 2fb64fd6486e6..c3c983097bdc9 100644 --- a/datafusion/core/src/datasource/file_format/csv.rs +++ b/datafusion/core/src/datasource/file_format/csv.rs @@ -210,7 +210,7 @@ mod tests { assert_eq!(12, batch.num_columns()); assert_eq!(2, batch.num_rows()); }) - .fold(0, |acc, _| async move { acc + 1i32 }) + .fold(0, |acc, ()| async move { acc + 1i32 }) .await; assert_eq!(tt_batches, 50 /* 100/2 */); diff --git a/datafusion/core/src/datasource/file_format/json.rs b/datafusion/core/src/datasource/file_format/json.rs index 1f6f27242e723..02039a880c136 100644 --- a/datafusion/core/src/datasource/file_format/json.rs +++ b/datafusion/core/src/datasource/file_format/json.rs @@ -112,7 +112,7 @@ mod tests { assert_eq!(4, batch.num_columns()); assert_eq!(2, batch.num_rows()); }) - .fold(0, |acc, _| async move { acc + 1i32 }) + .fold(0, |acc, ()| async move { acc + 1i32 }) .await; assert_eq!(tt_batches, 6 /* 12/2 */); diff --git a/datafusion/core/src/datasource/file_format/parquet.rs b/datafusion/core/src/datasource/file_format/parquet.rs index bfcfb74848861..06cf0a8318c35 100644 --- a/datafusion/core/src/datasource/file_format/parquet.rs +++ b/datafusion/core/src/datasource/file_format/parquet.rs @@ -709,7 +709,7 @@ mod tests { assert_eq!(11, batch.num_columns()); assert_eq!(2, batch.num_rows()); }) - .fold(0, |acc, _| async move { acc + 1i32 }) + .fold(0, |acc, ()| async move { acc + 1i32 }) .await; assert_eq!(tt_batches, 4 /* 8/2 */); diff --git a/datafusion/core/tests/execution/coop.rs b/datafusion/core/tests/execution/coop.rs index e02364a0530cc..60e1504bf8bf3 100644 --- a/datafusion/core/tests/execution/coop.rs +++ b/datafusion/core/tests/execution/coop.rs @@ -795,12 +795,12 @@ async fn stream_yields( result = join_handle => { match result { Ok(Poll::Pending) => Yielded::ReadyOrPending, - Ok(Poll::Ready(Ok(_))) => Yielded::ReadyOrPending, + Ok(Poll::Ready(Ok(()))) => Yielded::ReadyOrPending, Ok(Poll::Ready(Err(e))) => Yielded::Err(e), Err(_) => Yielded::Err(exec_datafusion_err!("join error")), } }, - _ = tokio::time::sleep(Duration::from_secs(10)) => { + () = tokio::time::sleep(Duration::from_secs(10)) => { Yielded::Timeout } }; diff --git a/datafusion/datasource/src/write/orchestration.rs b/datafusion/datasource/src/write/orchestration.rs index cd821b3b87897..387e929b9f30d 100644 --- a/datafusion/datasource/src/write/orchestration.rs +++ b/datafusion/datasource/src/write/orchestration.rs @@ -115,7 +115,7 @@ pub(crate) async fn serialize_rb_stream_to_object_store( match task.join().await { Ok(Ok((cnt, bytes))) => { match writer.write_all(&bytes).await { - Ok(_) => (), + Ok(()) => (), Err(e) => { return SerializedRecordBatchResult::failure( None, @@ -142,7 +142,7 @@ pub(crate) async fn serialize_rb_stream_to_object_store( } match serialize_task.join().await { - Ok(Ok(_)) => (), + Ok(Ok(())) => (), Ok(Err(e)) => return SerializedRecordBatchResult::failure(Some(writer), e), Err(_) => { return SerializedRecordBatchResult::failure( diff --git a/datafusion/execution/src/async_stream.rs b/datafusion/execution/src/async_stream.rs index 0462c53a0ffd3..e49f0c6980c29 100644 --- a/datafusion/execution/src/async_stream.rs +++ b/datafusion/execution/src/async_stream.rs @@ -402,7 +402,7 @@ mod test { let s = async_stream(|mut emitter| async move { select! { - _ = do_stuff_async() => emitter.emit(()).await, + () = do_stuff_async() => emitter.emit(()).await, else => emitter.emit(()).await, } }); @@ -422,8 +422,8 @@ mod test { let s = async_stream(|mut emitter| async move { select! { - _ = do_stuff_async() => emitter.emit("hey").await, - _ = more_async_work() => emitter.emit("hey").await, + () = do_stuff_async() => emitter.emit("hey").await, + () = more_async_work() => emitter.emit("hey").await, else => emitter.emit("hey").await, } }); @@ -464,7 +464,7 @@ mod test { pin_mut!(s); for i in 0..3 { - assert_matches!(tx.send(i).await, Ok(_)); + assert_matches!(tx.send(i).await, Ok(())); assert_eq!(Some(i), s.next().await); } @@ -573,7 +573,7 @@ mod test { let _ = async_stream(|mut emitter| async move { select! { - _ = do_stuff_async() => { + () = do_stuff_async() => { let another_s = async_try_stream(|mut inner_emitter| async move { inner_emitter.emit(()).await; Ok(()) diff --git a/datafusion/expr/src/expr_schema.rs b/datafusion/expr/src/expr_schema.rs index 8819c7a95e795..bc5fd4977d1f7 100644 --- a/datafusion/expr/src/expr_schema.rs +++ b/datafusion/expr/src/expr_schema.rs @@ -317,7 +317,7 @@ impl ExprSchemable for Expr { .transpose()?; Ok(match has_nullable { // If a nullable subexpression is found, the result may also be nullable. - Some(_) => true, + Some(()) => true, // If the list is too long, we assume it is nullable. None if list.len() + 1 > MAX_INSPECT_LIMIT => true, // All the subexpressions are non-nullable, so the result must be non-nullable. @@ -385,7 +385,7 @@ impl ExprSchemable for Expr { // There is at least one reachable nullable 'then' expression, so the case // expression itself is nullable. // Use `Result::map` to propagate the error from `nullable_then` if there is one. - nullable_then.map(|_| true) + nullable_then.map(|()| true) } else if let Some(e) = &case.else_expr { // There are no reachable nullable 'then' expressions, so all we still need to // check is the 'else' expression's nullability. diff --git a/datafusion/expr/src/type_coercion/functions.rs b/datafusion/expr/src/type_coercion/functions.rs index 8e86cb3685e90..87272316e1a63 100644 --- a/datafusion/expr/src/type_coercion/functions.rs +++ b/datafusion/expr/src/type_coercion/functions.rs @@ -246,15 +246,15 @@ pub fn value_fields_with_higher_order_udf( current_fields.iter().zip(expected.iter()).enumerate() { match (actual, expected) { - (ValueOrLambda::Value(_), ValueOrLambda::Value(_)) => {} - (ValueOrLambda::Lambda(_), ValueOrLambda::Lambda(_)) => {} - (ValueOrLambda::Value(_), ValueOrLambda::Lambda(_)) => { + (ValueOrLambda::Value(_), ValueOrLambda::Value(())) => {} + (ValueOrLambda::Lambda(_), ValueOrLambda::Lambda(())) => {} + (ValueOrLambda::Value(_), ValueOrLambda::Lambda(())) => { let name = func.name(); return plan_err!( "The function '{name}' expected a lambda at position {i} but received a value" ); } - (ValueOrLambda::Lambda(_), ValueOrLambda::Value(_)) => { + (ValueOrLambda::Lambda(_), ValueOrLambda::Value(())) => { let name = func.name(); return plan_err!( "The function '{name}' expected a value at position {i} but received a lambda" diff --git a/datafusion/physical-expr-common/src/binary_map.rs b/datafusion/physical-expr-common/src/binary_map.rs index 16e00b6549fb4..6024006f3278b 100644 --- a/datafusion/physical-expr-common/src/binary_map.rs +++ b/datafusion/physical-expr-common/src/binary_map.rs @@ -818,7 +818,7 @@ mod tests { let values: ArrayRef = Arc::new(StringArray::from_iter_values( (0..1_000).map(|i| format!("distinct value number {i}")), )); - map.insert_if_new(&values, |_| (), |_| ()); + map.insert_if_new(&values, |_| (), |()| {}); let populated_size = map.size(); assert!(populated_size > INITIAL_BUFFER_CAPACITY); @@ -950,8 +950,8 @@ mod tests { let value = format!("{}:{i}", batch * 1_000 + i); format!("{value:value_len$}") }))); - lazy.insert_if_new(&values, |_| (), |_| ()); - pre_allocated.insert_if_new(&values, |_| (), |_| ()); + lazy.insert_if_new(&values, |_| (), |()| {}); + pre_allocated.insert_if_new(&values, |_| (), |()| {}); assert_eq!(lazy.buffer.len(), pre_allocated.buffer.len()); assert_eq!( @@ -973,7 +973,7 @@ mod tests { let values: ArrayRef = Arc::new(StringArray::from_iter_values( (0..10).map(|i| format!("distinct value number {i}")), )); - lazy.insert_if_new(&values, |_| (), |_| ()); + lazy.insert_if_new(&values, |_| (), |()| {}); assert!( lazy.buffer.capacity() < INITIAL_BUFFER_CAPACITY, diff --git a/datafusion/physical-expr-common/src/binary_view_map.rs b/datafusion/physical-expr-common/src/binary_view_map.rs index 29f4014c5f9a4..7c0cdae11b70f 100644 --- a/datafusion/physical-expr-common/src/binary_view_map.rs +++ b/datafusion/physical-expr-common/src/binary_view_map.rs @@ -904,7 +904,7 @@ mod tests { let values: ArrayRef = Arc::new(StringViewArray::from_iter_values( (0..1_000).map(|i| format!("distinct value number {i}")), )); - map.insert_if_new(&values, |_| (), |_| ()); + map.insert_if_new(&values, |_| (), |()| {}); let warm_size = map.map.allocation_size(); assert!(warm_size > 0); @@ -944,7 +944,7 @@ mod tests { ])); let mut map = ArrowBytesViewMap::new(OutputType::Utf8View); - map.insert_if_new(&values, |_| (), |_| {}); + map.insert_if_new(&values, |_| (), |()| {}); // Make unused vector capacity explicit; the completed buffers were created // by the map's flush path. @@ -989,7 +989,7 @@ mod tests { assert_eq!(map.size() - legacy_size, retained_capacity_delta); let size_after_insert = map.size(); - map.insert_if_new(&values, |_| (), |_| {}); + map.insert_if_new(&values, |_| (), |()| {}); assert_eq!(map.size(), size_after_insert); } diff --git a/datafusion/physical-expr/src/expressions/case.rs b/datafusion/physical-expr/src/expressions/case.rs index dd98029c739a8..94e223398b2b9 100644 --- a/datafusion/physical-expr/src/expressions/case.rs +++ b/datafusion/physical-expr/src/expressions/case.rs @@ -1311,7 +1311,7 @@ impl PhysicalExpr for CaseExpr { // There is at least one reachable nullable 'then' expression, so the case // expression itself is nullable. // Use `Result::map` to propagate the error from `nullable_then` if there is one. - nullable_then.map(|_| true) + nullable_then.map(|()| true) } else if let Some(e) = &self.body.else_expr { // There are no reachable nullable 'then' expressions, so all we still need to // check is the 'else' expression's nullability. diff --git a/datafusion/physical-plan/src/aggregates/aggregate_stream.rs b/datafusion/physical-plan/src/aggregates/aggregate_stream.rs index 23f74e6352a15..4d1609b999035 100644 --- a/datafusion/physical-plan/src/aggregates/aggregate_stream.rs +++ b/datafusion/physical-plan/src/aggregates/aggregate_stream.rs @@ -396,7 +396,7 @@ impl AggregateStream { match result .and_then(|allocated| this.reservation.try_grow(allocated)) { - Ok(_) => continue, + Ok(()) => continue, Err(e) => Err(e), } } diff --git a/datafusion/physical-plan/src/aggregates/grouped_hash_stream.rs b/datafusion/physical-plan/src/aggregates/grouped_hash_stream.rs index 24bb4c16d887c..fa3c44c51089b 100644 --- a/datafusion/physical-plan/src/aggregates/grouped_hash_stream.rs +++ b/datafusion/physical-plan/src/aggregates/grouped_hash_stream.rs @@ -987,7 +987,7 @@ impl GroupedHashAggregateStream { let oom = match self.update_memory_reservation() { Err(e @ DataFusionError::ResourcesExhausted(_)) => e, Err(e) => return Err(e), - Ok(_) => return Ok(None), + Ok(()) => return Ok(None), }; match self.oom_mode { diff --git a/datafusion/physical-plan/src/buffer.rs b/datafusion/physical-plan/src/buffer.rs index df87fee7da087..00656c6e642c0 100644 --- a/datafusion/physical-plan/src/buffer.rs +++ b/datafusion/physical-plan/src/buffer.rs @@ -429,7 +429,7 @@ impl MemoryBufferedStream { // in order to consider aborting the stream let item_or_err = tokio::select! { biased; - _ = batch_tx.closed() => break, + () = batch_tx.closed() => break, // Catch a panic in the input poll so it surfaces as a stream error // instead of dropping `batch_tx` and looking like a clean EOF. polled = AssertUnwindSafe(input.next()).catch_unwind() => { diff --git a/datafusion/physical-plan/src/joins/asof_join.rs b/datafusion/physical-plan/src/joins/asof_join.rs index 3e67e929d0971..96968fd2f23d3 100644 --- a/datafusion/physical-plan/src/joins/asof_join.rs +++ b/datafusion/physical-plan/src/joins/asof_join.rs @@ -585,7 +585,7 @@ async fn collect_right_input( let batches = input .try_fold(Vec::new(), |mut batches, batch| { let batch_size = memory_counter.count_batch(&batch); - futures::future::ready(reservation.try_grow(batch_size).map(|_| { + futures::future::ready(reservation.try_grow(batch_size).map(|()| { metrics.build_mem_used.add(batch_size); batches.push(batch); batches diff --git a/datafusion/physical-plan/src/joins/sort_merge_join/materializing_stream.rs b/datafusion/physical-plan/src/joins/sort_merge_join/materializing_stream.rs index 96b903f63bc1e..eb2c27df7c5ef 100644 --- a/datafusion/physical-plan/src/joins/sort_merge_join/materializing_stream.rs +++ b/datafusion/physical-plan/src/joins/sort_merge_join/materializing_stream.rs @@ -1075,7 +1075,7 @@ impl MaterializingSortMergeJoinStream { fn allocate_reservation(&mut self, mut buffered_batch: BufferedBatch) -> Result<()> { match self.reservation.try_grow(buffered_batch.size_estimation) { - Ok(_) => { + Ok(()) => { buffered_batch.reserved_amount = buffered_batch.size_estimation; self.join_metrics .peak_mem_used() diff --git a/datafusion/physical-plan/src/repartition/mod.rs b/datafusion/physical-plan/src/repartition/mod.rs index 0bbc68641c6e9..cc956b3a36dd5 100644 --- a/datafusion/physical-plan/src/repartition/mod.rs +++ b/datafusion/physical-plan/src/repartition/mod.rs @@ -232,7 +232,7 @@ impl OutputChannel { // across an await point. let (payload, is_memory_batch) = { match self.reservation.try_grow(size) { - Ok(_) => (Ok(RepartitionBatch::Memory(batch)), true), + Ok(()) => (Ok(RepartitionBatch::Memory(batch)), true), Err(_) => match self.spill_writer.push_batch(&batch) { Ok(()) => (Ok(RepartitionBatch::Spilled), false), Err(err) => (Err(err), false), diff --git a/datafusion/physical-plan/src/sorts/multi_level_merge.rs b/datafusion/physical-plan/src/sorts/multi_level_merge.rs index b5aa5c4d54015..3bd97359e118e 100644 --- a/datafusion/physical-plan/src/sorts/multi_level_merge.rs +++ b/datafusion/physical-plan/src/sorts/multi_level_merge.rs @@ -514,7 +514,7 @@ impl MultiLevelMergeBuilder { // this is not and there should be some upper limit to memory // reservation so we won't starve the system. match try_grow_reservation_to_at_least(reservation, total_needed) { - Ok(_) => { + Ok(()) => { number_of_spills_to_read_for_current_phase += 1; } // If we can't grow the reservation, we need to stop diff --git a/datafusion/physical-plan/src/sorts/sort.rs b/datafusion/physical-plan/src/sorts/sort.rs index 490ea7cc85776..4259696a04b64 100644 --- a/datafusion/physical-plan/src/sorts/sort.rs +++ b/datafusion/physical-plan/src/sorts/sort.rs @@ -836,7 +836,7 @@ impl ExternalSorter { let size = get_reserved_bytes_for_record_batch(input)?; match self.reservation.try_grow(size) { - Ok(_) => Ok(()), + Ok(()) => Ok(()), Err(e) => { if self.in_mem_batches.is_empty() { return Err(Self::err_with_oom_context(e)); diff --git a/datafusion/physical-plan/src/spill/spill_pool.rs b/datafusion/physical-plan/src/spill/spill_pool.rs index a3366d6766170..f27c862e6f93a 100644 --- a/datafusion/physical-plan/src/spill/spill_pool.rs +++ b/datafusion/physical-plan/src/spill/spill_pool.rs @@ -1765,7 +1765,7 @@ mod tests { let mut inner = Some(self.inner.read_stream()?); Ok(Box::pin( futures::stream::once(tokio::time::sleep(delay)) - .flat_map(move |_| inner.take().expect("polled once")), + .flat_map(move |()| inner.take().expect("polled once")), )) } diff --git a/datafusion/proto/src/logical_plan/from_proto.rs b/datafusion/proto/src/logical_plan/from_proto.rs index 1eac6f974beed..c1da0b3ee9418 100644 --- a/datafusion/proto/src/logical_plan/from_proto.rs +++ b/datafusion/proto/src/logical_plan/from_proto.rs @@ -205,7 +205,7 @@ pub fn parse_expr( let window_frame = WindowFrame::try_from(window_frame.clone())?; window_frame .regularize_order_bys(&mut order_by) - .map(|_| window_frame) + .map(|()| window_frame) }) .transpose()? .ok_or_else(|| { diff --git a/datafusion/proto/src/physical_plan/mod.rs b/datafusion/proto/src/physical_plan/mod.rs index 887337e291f43..93492ec612ad9 100644 --- a/datafusion/proto/src/physical_plan/mod.rs +++ b/datafusion/proto/src/physical_plan/mod.rs @@ -1358,7 +1358,7 @@ pub trait PhysicalPlanNodeExt: Sized { let mut buf: Vec = vec![]; match codec.try_encode(Arc::clone(&plan_clone), &mut buf, proto_converter) { - Ok(_) => { + Ok(()) => { let inputs: Vec = plan_clone .children() .into_iter() @@ -2065,7 +2065,7 @@ impl ComposedPhysicalExtensionCodec { // find the encoder for (position, codec) in self.codecs.iter().enumerate() { match encode(codec.as_ref(), &mut data) { - Ok(_) => { + Ok(()) => { encoder_position = Some(position as u32); break; } diff --git a/datafusion/proto/src/physical_plan/to_proto.rs b/datafusion/proto/src/physical_plan/to_proto.rs index 5ae57752de676..94fa42b09b48b 100644 --- a/datafusion/proto/src/physical_plan/to_proto.rs +++ b/datafusion/proto/src/physical_plan/to_proto.rs @@ -323,7 +323,7 @@ pub fn serialize_physical_expr_with_converter( } else { let mut buf: Vec = vec![]; match codec.try_encode_expr(value, &mut buf, &ctx) { - Ok(_) => { + Ok(()) => { let inputs: Vec = value .children() .into_iter() diff --git a/datafusion/sql/src/expr/function.rs b/datafusion/sql/src/expr/function.rs index a7f7979a67b36..c2259c714d84c 100644 --- a/datafusion/sql/src/expr/function.rs +++ b/datafusion/sql/src/expr/function.rs @@ -620,7 +620,7 @@ impl SqlToRel<'_, S> { let window_frame: WindowFrame = window_frame.clone().try_into()?; window_frame .regularize_order_bys(&mut order_by) - .map(|_| window_frame) + .map(|()| window_frame) }) .transpose()?; diff --git a/datafusion/substrait/tests/cases/roundtrip_logical_plan.rs b/datafusion/substrait/tests/cases/roundtrip_logical_plan.rs index 3716b0feba3cc..6b58c4a53af17 100644 --- a/datafusion/substrait/tests/cases/roundtrip_logical_plan.rs +++ b/datafusion/substrait/tests/cases/roundtrip_logical_plan.rs @@ -2466,7 +2466,7 @@ fn check_post_join_filters(rel: &Rel) -> Result<()> { // recursively check JoinRels match check_post_join_filters(join.left.as_ref().unwrap().as_ref()) { Err(e) => Err(e), - Ok(_) => { + Ok(()) => { check_post_join_filters(join.right.as_ref().unwrap().as_ref()) } } From 33b2e9e2549f23d6b789574932b7de16e9b67511 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 1 Sep 2026 13:47:16 +0200 Subject: [PATCH 3/6] Enable clippy::redundant_else Dropped `else` blocks after a branch that already diverges, removing one level of indentation at each site. --- Cargo.toml | 1 - datafusion-cli/src/print_format.rs | 5 +- .../examples/udf/simple_udtf.rs | 5 +- datafusion/catalog-listing/src/table.rs | 9 +- datafusion/common/src/test_util.rs | 10 +- .../user_defined_table_functions.rs | 5 +- datafusion/datasource-parquet/src/sink.rs | 75 ++++++----- datafusion/datasource/src/memory.rs | 3 +- .../expr-common/src/interval_arithmetic.rs | 10 +- datafusion/expr/src/predicate_bounds.rs | 8 +- .../expr/src/type_coercion/functions.rs | 29 +++-- .../functions-aggregate/src/correlation.rs | 8 +- .../functions-aggregate/src/first_last.rs | 8 +- datafusion/functions-nested/src/planner.rs | 8 +- datafusion/functions-window/src/nth_value.rs | 3 +- datafusion/functions/src/datetime/common.rs | 3 +- datafusion/functions/src/datetime/date_bin.rs | 3 +- datafusion/optimizer/src/push_down_filter.rs | 3 +- .../simplify_expressions/expr_simplifier.rs | 8 +- .../simplify_expressions/inlist_simplifier.rs | 12 +- .../src/simplify_expressions/utils.rs | 12 +- .../physical-expr/src/equivalence/class.rs | 3 +- .../physical-expr/src/expressions/binary.rs | 22 ++-- .../enforce_distribution.rs | 3 +- .../enforce_sorting/sort_pushdown.rs | 41 +++--- .../physical-plan/src/column_rewriter.rs | 12 +- datafusion/physical-plan/src/limit.rs | 3 +- datafusion/physical-plan/src/test/exec.rs | 9 +- datafusion/physical-plan/src/topk/mod.rs | 11 +- datafusion/physical-plan/src/union.rs | 3 +- datafusion/pruning/src/pruning_predicate.rs | 13 +- datafusion/sql/src/expr/function.rs | 118 ++++++++---------- datafusion/sql/src/expr/value.rs | 3 +- datafusion/sql/src/parser.rs | 6 +- datafusion/sql/src/select.rs | 91 +++++++------- datafusion/sql/src/statement.rs | 3 +- .../src/logical_plan/consumer/expr/literal.rs | 3 +- 37 files changed, 266 insertions(+), 306 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d2b56a91a620f..e1846648632a3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -298,7 +298,6 @@ missing_panics_doc = "allow" # 244 hits must_use_candidate = "allow" # 2726 hits needless_raw_string_hashes = "allow" # 540 hits redundant_closure_for_method_calls = "allow" # 686 hits -redundant_else = "allow" # 48 hits return_self_not_must_use = "allow" # 644 hits semicolon_if_nothing_returned = "allow" # 1353 hits similar_names = "allow" # 228 hits; too many false positives, e.g. `expr`/`exprs` diff --git a/datafusion-cli/src/print_format.rs b/datafusion-cli/src/print_format.rs index 0443a7a289602..d946bf8fa86a7 100644 --- a/datafusion-cli/src/print_format.rs +++ b/datafusion-cli/src/print_format.rs @@ -128,10 +128,9 @@ fn format_batches_with_maxrows( filtered_batches.push(sliced_batch); over_limit = true; break; - } else { - filtered_batches.push(batch.clone()); - row_count += batch.num_rows(); } + filtered_batches.push(batch.clone()); + row_count += batch.num_rows(); } let formatted = diff --git a/datafusion-examples/examples/udf/simple_udtf.rs b/datafusion-examples/examples/udf/simple_udtf.rs index 3b55a0456a0aa..0374e913db35c 100644 --- a/datafusion-examples/examples/udf/simple_udtf.rs +++ b/datafusion-examples/examples/udf/simple_udtf.rs @@ -110,10 +110,9 @@ impl TableProvider for LocalCsvTable { let batch_lines = max_return_lines - lines; batches.push(batch.slice(0, batch_lines)); break; - } else { - batches.push(batch.clone()); - lines += batch_lines; } + batches.push(batch.clone()); + lines += batch_lines; } batches } else { diff --git a/datafusion/catalog-listing/src/table.rs b/datafusion/catalog-listing/src/table.rs index 6c294fe077db4..569a1c9a71325 100644 --- a/datafusion/catalog-listing/src/table.rs +++ b/datafusion/catalog-listing/src/table.rs @@ -425,12 +425,11 @@ fn derive_common_ordering_from_files(file_groups: &[FileGroup]) -> Option 0, so ordering must be valid"); - CurrentOrderingState::SomeOrdering(ordering) } + let ordering = + LexOrdering::new(current.as_ref()[..prefix_len].to_vec()) + .expect("prefix_len > 0, so ordering must be valid"); + CurrentOrderingState::SomeOrdering(ordering) } // If one file has ordering and another doesn't, no common ordering // Return None and log a trace message explaining why diff --git a/datafusion/common/src/test_util.rs b/datafusion/common/src/test_util.rs index 348fe2ef547f0..122f063788717 100644 --- a/datafusion/common/src/test_util.rs +++ b/datafusion/common/src/test_util.rs @@ -285,16 +285,16 @@ pub fn get_data_dir( let trimmed = dir.trim().to_string(); if !trimmed.is_empty() { let pb = PathBuf::from(trimmed); - if pb.is_dir() { - return Ok(pb); + return if pb.is_dir() { + Ok(pb) } else { - return Err(format!( + Err(format!( "the data dir `{}` defined by env {} not found", pb.display(), udf_env ) - .into()); - } + .into()) + }; } } diff --git a/datafusion/core/tests/user_defined/user_defined_table_functions.rs b/datafusion/core/tests/user_defined/user_defined_table_functions.rs index 24205cf8c4010..bbb01b2a76c18 100644 --- a/datafusion/core/tests/user_defined/user_defined_table_functions.rs +++ b/datafusion/core/tests/user_defined/user_defined_table_functions.rs @@ -144,10 +144,9 @@ impl TableProvider for SimpleCsvTable { let batch_lines = max_return_lines as usize - lines; batches.push(batch.slice(0, batch_lines)); break; - } else { - batches.push(batch.clone()); - lines += batch_lines; } + batches.push(batch.clone()); + lines += batch_lines; } batches } else { diff --git a/datafusion/datasource-parquet/src/sink.rs b/datafusion/datasource-parquet/src/sink.rs index 3c66d4dcd74fb..7eca0def43e28 100644 --- a/datafusion/datasource-parquet/src/sink.rs +++ b/datafusion/datasource-parquet/src/sink.rs @@ -693,47 +693,46 @@ fn spawn_parquet_parallel_serialization_task( .await?; current_rg_rows += rb.num_rows(); break; - } else { - let rows_left = max_row_group_rows - current_rg_rows; - let a = rb.slice(0, rows_left); - send_arrays_to_col_writers( - &col_array_channels, - &a, - Arc::clone(&ctx.schema), - ) - .await?; + } + let rows_left = max_row_group_rows - current_rg_rows; + let a = rb.slice(0, rows_left); + send_arrays_to_col_writers( + &col_array_channels, + &a, + Arc::clone(&ctx.schema), + ) + .await?; + + // Signal the parallel column writers that the RowGroup is done, join and finalize RowGroup + // on a separate task, so that we can immediately start on the next RG before waiting + // for the current one to finish. + drop(col_array_channels); + let finalize_rg_task = spawn_rg_join_and_finalize_task( + column_writer_handles, + max_row_group_rows, + &ctx.pool, + encoding_time.clone(), + ); - // Signal the parallel column writers that the RowGroup is done, join and finalize RowGroup - // on a separate task, so that we can immediately start on the next RG before waiting - // for the current one to finish. - drop(col_array_channels); - let finalize_rg_task = spawn_rg_join_and_finalize_task( - column_writer_handles, - max_row_group_rows, - &ctx.pool, - encoding_time.clone(), - ); + // Do not surface error from closed channel (means something + // else hit an error, and the plan is shutting down). + if serialize_tx.send(finalize_rg_task).await.is_err() { + return Ok(()); + } - // Do not surface error from closed channel (means something - // else hit an error, and the plan is shutting down). - if serialize_tx.send(finalize_rg_task).await.is_err() { - return Ok(()); - } + current_rg_rows = 0; + rb = rb.slice(rows_left, rb.num_rows() - rows_left); - current_rg_rows = 0; - rb = rb.slice(rows_left, rb.num_rows() - rows_left); - - row_group_index += 1; - let col_writers = row_group_writer_factory - .create_column_writers(row_group_index)?; - (column_writer_handles, col_array_channels) = - spawn_column_parallel_row_group_writer( - col_writers, - max_buffer_rb, - &ctx.pool, - &encoding_time, - )?; - } + row_group_index += 1; + let col_writers = + row_group_writer_factory.create_column_writers(row_group_index)?; + (column_writer_handles, col_array_channels) = + spawn_column_parallel_row_group_writer( + col_writers, + max_buffer_rb, + &ctx.pool, + &encoding_time, + )?; } } diff --git a/datafusion/datasource/src/memory.rs b/datafusion/datasource/src/memory.rs index 4c79cf4a9851d..f302fedd2b5db 100644 --- a/datafusion/datasource/src/memory.rs +++ b/datafusion/datasource/src/memory.rs @@ -623,9 +623,8 @@ impl MemorySourceConfig { } // Successful repartition. Break inner loop, and return to outer `cnt_to_repartition` loop. break; - } else { - cannot_split_further.push(new_partitions.remove(0)); } + cannot_split_further.push(new_partitions.remove(0)); } } let mut partitions = max_heap diff --git a/datafusion/expr-common/src/interval_arithmetic.rs b/datafusion/expr-common/src/interval_arithmetic.rs index 9f1291353dc29..e1168cf8352ed 100644 --- a/datafusion/expr-common/src/interval_arithmetic.rs +++ b/datafusion/expr-common/src/interval_arithmetic.rs @@ -1410,18 +1410,18 @@ pub fn satisfy_greater( ); if !left.upper.is_null() && left.upper <= right.lower { - if !strict && left.upper == right.lower { + return if !strict && left.upper == right.lower { // Singleton intervals: - return Ok(Some(( + Ok(Some(( Interval::new(left.upper.clone(), left.upper.clone()), Interval::new(left.upper.clone(), left.upper.clone()), - ))); + ))) } else { // Left-hand side: <--======----0------------> // Right-hand side: <------------0--======----> // No intersection, infeasible to propagate: - return Ok(None); - } + Ok(None) + }; } // Only the lower bound of left-hand side and the upper bound of the right-hand diff --git a/datafusion/expr/src/predicate_bounds.rs b/datafusion/expr/src/predicate_bounds.rs index aa947416c87b5..3e7191b6bb33b 100644 --- a/datafusion/expr/src/predicate_bounds.rs +++ b/datafusion/expr/src/predicate_bounds.rs @@ -158,11 +158,11 @@ impl PredicateBoundsEvaluator<'_> { fn is_null(&self, expr: &Expr) -> NullableInterval { // Fast path for literals if let Expr::Literal(scalar, _) = expr { - if scalar.is_null() { - return NullableInterval::TRUE; + return if scalar.is_null() { + NullableInterval::TRUE } else { - return NullableInterval::FALSE; - } + NullableInterval::FALSE + }; } // If `expr` is not nullable, we can be certain `expr` is not null diff --git a/datafusion/expr/src/type_coercion/functions.rs b/datafusion/expr/src/type_coercion/functions.rs index 87272316e1a63..dde7105f9362b 100644 --- a/datafusion/expr/src/type_coercion/functions.rs +++ b/datafusion/expr/src/type_coercion/functions.rs @@ -115,17 +115,17 @@ pub fn fields_with_udf( let type_signature = &signature.type_signature; if current_fields.is_empty() && type_signature != &TypeSignature::UserDefined { - if type_signature.supports_zero_argument() { - return Ok(vec![]); + return if type_signature.supports_zero_argument() { + Ok(vec![]) } else if type_signature.used_to_support_zero_arguments() { // Special error to help during upgrade: https://github.com/apache/datafusion/issues/13763 - return plan_err!( + plan_err!( "'{}' does not support zero arguments. Use TypeSignature::Nullary for zero arguments", func.name() - ); + ) } else { - return plan_err!("'{}' does not support zero arguments", func.name()); - } + plan_err!("'{}' does not support zero arguments", func.name()) + }; } let current_types = current_fields .iter() @@ -438,20 +438,20 @@ pub fn data_types( let type_signature = &signature.type_signature; if current_types.is_empty() && type_signature != &TypeSignature::UserDefined { - if type_signature.supports_zero_argument() { - return Ok(vec![]); + return if type_signature.supports_zero_argument() { + Ok(vec![]) } else if type_signature.used_to_support_zero_arguments() { // Special error to help during upgrade: https://github.com/apache/datafusion/issues/13763 - return plan_err!( + plan_err!( "function '{}' has signature {type_signature} which does not support zero arguments. Use TypeSignature::Nullary for zero arguments", function_name.as_ref() - ); + ) } else { - return plan_err!( + plan_err!( "Function '{}' has signature {type_signature} which does not support zero arguments", function_name.as_ref() - ); - } + ) + }; } let valid_types = @@ -566,9 +566,8 @@ fn get_valid_types_with_udf( func.name(), errors.join(",") ); - } else { - res } + res } _ => get_valid_types(func.name(), signature, current_types)?, }; diff --git a/datafusion/functions-aggregate/src/correlation.rs b/datafusion/functions-aggregate/src/correlation.rs index 21859006ea943..14545ee238cd8 100644 --- a/datafusion/functions-aggregate/src/correlation.rs +++ b/datafusion/functions-aggregate/src/correlation.rs @@ -212,11 +212,11 @@ impl Accumulator for CorrelationAccumulator { && let ScalarValue::Float64(Some(s1)) = stddev1 && let ScalarValue::Float64(Some(s2)) = stddev2 { - if s1 == 0_f64 || s2 == 0_f64 { - return Ok(ScalarValue::Float64(None)); + return if s1 == 0_f64 || s2 == 0_f64 { + Ok(ScalarValue::Float64(None)) } else { - return Ok(ScalarValue::Float64(Some(c / s1 / s2))); - } + Ok(ScalarValue::Float64(Some(c / s1 / s2))) + }; } Ok(ScalarValue::Float64(None)) diff --git a/datafusion/functions-aggregate/src/first_last.rs b/datafusion/functions-aggregate/src/first_last.rs index d36ea5d63074b..0b9e5cb5f9d48 100644 --- a/datafusion/functions-aggregate/src/first_last.rs +++ b/datafusion/functions-aggregate/src/first_last.rs @@ -912,10 +912,9 @@ impl FirstValueAccumulator { } } return Ok(None); - } else { - // If not ignoring nulls, return the first value if it exists. - return Ok((!value.is_empty()).then_some(0)); } + // If not ignoring nulls, return the first value if it exists. + return Ok((!value.is_empty()).then_some(0)); } let sort_columns = ordering_values @@ -1301,9 +1300,8 @@ impl LastValueAccumulator { } } return Ok(None); - } else { - return Ok((!value.is_empty()).then_some(value.len() - 1)); } + return Ok((!value.is_empty()).then_some(value.len() - 1)); } let sort_columns = ordering_values diff --git a/datafusion/functions-nested/src/planner.rs b/datafusion/functions-nested/src/planner.rs index e96fdb7d4baca..8ca7bde758f60 100644 --- a/datafusion/functions-nested/src/planner.rs +++ b/datafusion/functions-nested/src/planner.rs @@ -85,13 +85,13 @@ impl ExprPlanner for NestedFunctionPlanner { let right_list_ndims = list_ndims(&right_type); // if both are list if left_list_ndims > 0 && right_list_ndims > 0 { - if op == BinaryOperator::AtArrow { + return if op == BinaryOperator::AtArrow { // array1 @> array2 -> array_has_all(array1, array2) - return Ok(PlannerResult::Planned(array_has_all(left, right))); + Ok(PlannerResult::Planned(array_has_all(left, right))) } else { // array1 <@ array2 -> array_has_all(array2, array1) - return Ok(PlannerResult::Planned(array_has_all(right, left))); - } + Ok(PlannerResult::Planned(array_has_all(right, left))) + }; } } diff --git a/datafusion/functions-window/src/nth_value.rs b/datafusion/functions-window/src/nth_value.rs index b3678e80f2273..75da3ab443f9e 100644 --- a/datafusion/functions-window/src/nth_value.rs +++ b/datafusion/functions-window/src/nth_value.rs @@ -407,9 +407,8 @@ impl PartitionEvaluator for NthValueEvaluator { state.window_frame_range.end - 1; } return Ok(()); - } else { - // Fall through to the main case because there are no nulls } + // Fall through to the main case because there are no nulls } // Do not memoize for other kinds when nulls are ignored NthValueKind::Last | NthValueKind::Nth => return Ok(()), diff --git a/datafusion/functions/src/datetime/common.rs b/datafusion/functions/src/datetime/common.rs index 118b6b371bc17..39707e907c53d 100644 --- a/datafusion/functions/src/datetime/common.rs +++ b/datafusion/functions/src/datetime/common.rs @@ -517,9 +517,8 @@ where if let Ok(inner) = r { val = Some(Ok(op2(inner))); break; - } else { - val = Some(r); } + val = Some(r); } } diff --git a/datafusion/functions/src/datetime/date_bin.rs b/datafusion/functions/src/datetime/date_bin.rs index 15cdecc3c2842..4e59e87cd1925 100644 --- a/datafusion/functions/src/datetime/date_bin.rs +++ b/datafusion/functions/src/datetime/date_bin.rs @@ -531,9 +531,8 @@ fn date_bin_impl( return not_impl_err!( "DATE_BIN stride does not support combination of month, day and nanosecond intervals" ); - } else { - Interval::Months(months as i64) } + Interval::Months(months as i64) } else { let nanos = (TimeDelta::try_days(days as i64).unwrap() + Duration::nanoseconds(nanos)) diff --git a/datafusion/optimizer/src/push_down_filter.rs b/datafusion/optimizer/src/push_down_filter.rs index 8a1dcc12ef874..623c23cee4672 100644 --- a/datafusion/optimizer/src/push_down_filter.rs +++ b/datafusion/optimizer/src/push_down_filter.rs @@ -1174,9 +1174,8 @@ impl OptimizerRule for PushDownFilter { { filter.input = Arc::new(LogicalPlan::TableScan(scan)); return Ok(Transformed::no(LogicalPlan::Filter(filter))); - } else { - scan.filters = new_scan_filters; } + scan.filters = new_scan_filters; // Compose predicates to be of `Unsupported` or `Inexact` pushdown type, // and also include volatile and subquery-containing filters diff --git a/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs b/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs index 5436bd092163e..e83def89395c2 100644 --- a/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs +++ b/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs @@ -1550,12 +1550,10 @@ impl TreeNodeRewriter for Simplifier<'_> { // CASE WHEN false THEN A ELSE B END --> B if let Some(else_expr) = else_expr { return Ok(Transformed::yes(*else_expr)); - // CASE WHEN false THEN A END --> NULL - } else { - let null = - Expr::Literal(ScalarValue::try_new_null(&out_type)?, None); - return Ok(Transformed::yes(null)); } + // CASE WHEN false THEN A END --> NULL + let null = Expr::Literal(ScalarValue::try_new_null(&out_type)?, None); + return Ok(Transformed::yes(null)); } Transformed::yes(Expr::Case(Case { diff --git a/datafusion/optimizer/src/simplify_expressions/inlist_simplifier.rs b/datafusion/optimizer/src/simplify_expressions/inlist_simplifier.rs index 17112d4f0ae24..a5b27da3d8b18 100644 --- a/datafusion/optimizer/src/simplify_expressions/inlist_simplifier.rs +++ b/datafusion/optimizer/src/simplify_expressions/inlist_simplifier.rs @@ -55,8 +55,8 @@ impl TreeNodeRewriter for ShortenInListSimplifier { ) { let first_val = list[0].clone(); - if negated { - return Ok(Transformed::yes(list.iter().skip(1).cloned().fold( + return if negated { + Ok(Transformed::yes(list.iter().skip(1).cloned().fold( (*expr.clone()).not_eq(first_val), |acc, y| { // Note that `A and B and C and D` is a left-deep tree structure @@ -78,16 +78,16 @@ impl TreeNodeRewriter for ShortenInListSimplifier { // The code below maintain the left-deep tree structure. acc.and((*expr.clone()).not_eq(y)) }, - ))); + ))) } else { - return Ok(Transformed::yes(list.iter().skip(1).cloned().fold( + Ok(Transformed::yes(list.iter().skip(1).cloned().fold( (*expr.clone()).eq(first_val), |acc, y| { // Same reasoning as above acc.or((*expr.clone()).eq(y)) }, - ))); - } + ))) + }; } Ok(Transformed::no(expr)) diff --git a/datafusion/optimizer/src/simplify_expressions/utils.rs b/datafusion/optimizer/src/simplify_expressions/utils.rs index 78d801630c7ba..7ed15be9fa3e3 100644 --- a/datafusion/optimizer/src/simplify_expressions/utils.rs +++ b/datafusion/optimizer/src/simplify_expressions/utils.rs @@ -91,19 +91,19 @@ pub fn delete_xor_in_complex_expr(expr: &Expr, needle: &Expr, is_left: bool) -> if result_expr.normalize_eq(needle) { return needle.clone(); } else if xor_counter % 2 == 0 { - if is_left { - return Expr::BinaryExpr(BinaryExpr::new( + return if is_left { + Expr::BinaryExpr(BinaryExpr::new( Box::new(needle.clone()), Operator::BitwiseXor, Box::new(result_expr), - )); + )) } else { - return Expr::BinaryExpr(BinaryExpr::new( + Expr::BinaryExpr(BinaryExpr::new( Box::new(result_expr), Operator::BitwiseXor, Box::new(needle.clone()), - )); - } + )) + }; } result_expr } diff --git a/datafusion/physical-expr/src/equivalence/class.rs b/datafusion/physical-expr/src/equivalence/class.rs index 06f384ac2db03..63966b4fd5edc 100644 --- a/datafusion/physical-expr/src/equivalence/class.rs +++ b/datafusion/physical-expr/src/equivalence/class.rs @@ -395,9 +395,8 @@ impl EquivalenceGroup { // If this class becomes trivial, remove it entirely: self.remove_class_at_idx(idx); continue; - } else { - cls.constant = None; } + cls.constant = None; } idx += 1; } diff --git a/datafusion/physical-expr/src/expressions/binary.rs b/datafusion/physical-expr/src/expressions/binary.rs index dfb1d136d0ff0..9e237ecf6cb1d 100644 --- a/datafusion/physical-expr/src/expressions/binary.rs +++ b/datafusion/physical-expr/src/expressions/binary.rs @@ -589,20 +589,18 @@ impl PhysicalExpr for BinaryExpr { ); } ColumnarValue::Scalar(scalar) => { - if let ScalarValue::Boolean(v) = scalar { + return if let ScalarValue::Boolean(v) = scalar { // A scalar RHS applies uniformly to all selected rows. if let Some(v) = v { - return Ok(uniform_pre_selection_result( - *v, fill_value, lhs, - )); + Ok(uniform_pre_selection_result(*v, fill_value, lhs)) } else { - return pre_selection_scatter(&mask, None, fill_value); + pre_selection_scatter(&mask, None, fill_value) } } else { - return internal_err!( + internal_err!( "Expected boolean scalar value, found: {right_ret:?}" - ); - } + ) + }; } } } @@ -1259,11 +1257,11 @@ fn check_short_circuit(lhs: &ColumnarValue, op: &Operator) -> ShortCircuitStrate // Return Left for: // - AND with false value // - OR with true value - if (is_and && !is_true) || (!is_and && *is_true) { - return ShortCircuitStrategy::ReturnLeft; + return if (is_and && !is_true) || (!is_and && *is_true) { + ShortCircuitStrategy::ReturnLeft } else { - return ShortCircuitStrategy::ReturnRight; - } + ShortCircuitStrategy::ReturnRight + }; } } } diff --git a/datafusion/physical-optimizer/src/ensure_requirements/enforce_distribution.rs b/datafusion/physical-optimizer/src/ensure_requirements/enforce_distribution.rs index ae9774c9f8c2d..b82edf1969e3c 100644 --- a/datafusion/physical-optimizer/src/ensure_requirements/enforce_distribution.rs +++ b/datafusion/physical-optimizer/src/ensure_requirements/enforce_distribution.rs @@ -234,9 +234,8 @@ pub fn adjust_input_keys_ordering( if aggregate_exec.mode() == &AggregateMode::FinalPartitioned { return reorder_aggregate_keys(requirements, aggregate_exec) .map(Transformed::yes); - } else { - requirements.data.clear(); } + requirements.data.clear(); } else { // Keep everything unchanged return Ok(Transformed::no(requirements)); diff --git a/datafusion/physical-optimizer/src/ensure_requirements/enforce_sorting/sort_pushdown.rs b/datafusion/physical-optimizer/src/ensure_requirements/enforce_sorting/sort_pushdown.rs index d7f556b90d9fe..9450c98ee9603 100644 --- a/datafusion/physical-optimizer/src/ensure_requirements/enforce_sorting/sort_pushdown.rs +++ b/datafusion/physical-optimizer/src/ensure_requirements/enforce_sorting/sort_pushdown.rs @@ -320,28 +320,27 @@ fn pushdown_sorts_helper( distribution_requirement: Distribution::UnspecifiedDistribution, }; return Ok(Transformed::yes(sort_push_down)); - } else { - // Sort was unnecessary, just propagate the stricter fetch and - // ordering requirements. Reset distribution to Unspecified - // because the sort we're removing may have been below a - // partition-merging node (like SortPreservingMergeExec) that - // already satisfies SinglePartition. - sort_push_down.data.fetch = min_fetch(sort_fetch, parent_fetch); - sort_push_down.data.distribution_requirement = - Distribution::UnspecifiedDistribution; - let current_is_stricter = eqp.requirements_compatible( - sort_ordering.clone().into(), - parent_requirement.first().clone(), - ); - sort_push_down.data.ordering_requirement = if current_is_stricter { - Some(OrderingRequirements::from(sort_ordering)) - } else { - Some(parent_requirement) - }; - // Recursive call to helper, so it doesn't transform_down and miss - // the new node (previous child of sort): - return pushdown_sorts_helper(sort_push_down); } + // Sort was unnecessary, just propagate the stricter fetch and + // ordering requirements. Reset distribution to Unspecified + // because the sort we're removing may have been below a + // partition-merging node (like SortPreservingMergeExec) that + // already satisfies SinglePartition. + sort_push_down.data.fetch = min_fetch(sort_fetch, parent_fetch); + sort_push_down.data.distribution_requirement = + Distribution::UnspecifiedDistribution; + let current_is_stricter = eqp.requirements_compatible( + sort_ordering.clone().into(), + parent_requirement.first().clone(), + ); + sort_push_down.data.ordering_requirement = if current_is_stricter { + Some(OrderingRequirements::from(sort_ordering)) + } else { + Some(parent_requirement) + }; + // Recursive call to helper, so it doesn't transform_down and miss + // the new node (previous child of sort): + return pushdown_sorts_helper(sort_push_down); } let can_push_fetch_to_children = can_push_fetch_through(&plan); diff --git a/datafusion/physical-plan/src/column_rewriter.rs b/datafusion/physical-plan/src/column_rewriter.rs index e03f5ab5d3d9d..1caf4877e53e2 100644 --- a/datafusion/physical-plan/src/column_rewriter.rs +++ b/datafusion/physical-plan/src/column_rewriter.rs @@ -51,20 +51,20 @@ impl TreeNodeRewriter for PhysicalColumnRewriter<'_> { node: Self::Node, ) -> datafusion_common::Result> { if let Some(column) = node.downcast_ref::() { - if let Some(new_column) = self.column_map.get(column) { + return if let Some(new_column) = self.column_map.get(column) { // jump to prevent rewriting the new sub-expression again - return Ok(Transformed::new( + Ok(Transformed::new( Arc::clone(new_column), true, TreeNodeRecursion::Jump, - )); + )) } else { // Column not found in mapping - return Err(DataFusionError::Internal(format!( + Err(DataFusionError::Internal(format!( "Column {column:?} not found in column mapping {:?}", self.column_map - ))); - } + ))) + }; } Ok(Transformed::no(node)) } diff --git a/datafusion/physical-plan/src/limit.rs b/datafusion/physical-plan/src/limit.rs index 73d98d8105215..6e029240cd9eb 100644 --- a/datafusion/physical-plan/src/limit.rs +++ b/datafusion/physical-plan/src/limit.rs @@ -672,9 +672,8 @@ impl LimitStream { Poll::Ready(Some(Ok(batch))) => { if batch.num_rows() > 0 { break poll; - } else { - // Continue to poll input stream } + // Continue to poll input stream } Poll::Ready(Some(Err(_e))) => break poll, Poll::Ready(None) => break poll, diff --git a/datafusion/physical-plan/src/test/exec.rs b/datafusion/physical-plan/src/test/exec.rs index f9517469d55ab..043c81012b881 100644 --- a/datafusion/physical-plan/src/test/exec.rs +++ b/datafusion/physical-plan/src/test/exec.rs @@ -1069,12 +1069,11 @@ impl Stream for PanicStream { self.ready = false; let batch = RecordBatch::new_empty(Arc::clone(&self.schema)); return Poll::Ready(Some(Ok(batch))); - } else { - self.ready = true; - // get called again - cx.waker().wake_by_ref(); - return Poll::Pending; } + self.ready = true; + // get called again + cx.waker().wake_by_ref(); + return Poll::Pending; } panic!("PanickingStream did panic: {}", self.partition) } diff --git a/datafusion/physical-plan/src/topk/mod.rs b/datafusion/physical-plan/src/topk/mod.rs index 0ca700cb37655..c97aba3552f5b 100644 --- a/datafusion/physical-plan/src/topk/mod.rs +++ b/datafusion/physical-plan/src/topk/mod.rs @@ -815,13 +815,12 @@ impl TopK { (&batch).record_output(&metrics.baseline); batches.push(Ok(batch)); break; - } else { - let head = batch.slice(0, batch_size); - (&head).record_output(&metrics.baseline); - batches.push(Ok(head)); - let remaining_length = batch.num_rows() - batch_size; - batch = batch.slice(batch_size, remaining_length); } + let head = batch.slice(0, batch_size); + (&head).record_output(&metrics.baseline); + batches.push(Ok(head)); + let remaining_length = batch.num_rows() - batch_size; + batch = batch.slice(batch_size, remaining_length); } } Ok(Box::pin(RecordBatchStreamAdapter::new( diff --git a/datafusion/physical-plan/src/union.rs b/datafusion/physical-plan/src/union.rs index da496b99fc6f7..1652c1a5d660c 100644 --- a/datafusion/physical-plan/src/union.rs +++ b/datafusion/physical-plan/src/union.rs @@ -394,9 +394,8 @@ impl ExecutionPlan for UnionExec { baseline_metrics, None, ))); - } else { - partition -= input.output_partitioning().partition_count(); } + partition -= input.output_partitioning().partition_count(); } warn!("Error in Union: Partition {partition} not found"); diff --git a/datafusion/pruning/src/pruning_predicate.rs b/datafusion/pruning/src/pruning_predicate.rs index c3362c63299e1..c7179d03a262c 100644 --- a/datafusion/pruning/src/pruning_predicate.rs +++ b/datafusion/pruning/src/pruning_predicate.rs @@ -1701,12 +1701,12 @@ fn build_predicate_expression( } if let Some(not) = expr.downcast_ref::() { // match !col (don't do so recursively) - if let Some(col) = not.arg().downcast_ref::() { - return build_single_column_expr(col, schema, required_columns, true) - .unwrap_or_else(|| unhandled_hook.handle(expr)); + return if let Some(col) = not.arg().downcast_ref::() { + build_single_column_expr(col, schema, required_columns, true) + .unwrap_or_else(|| unhandled_hook.handle(expr)) } else { - return unhandled_hook.handle(expr); - } + unhandled_hook.handle(expr) + }; } if let Some(in_list) = expr.downcast_ref::() { // Keep the existing expression shape for lists of at most 20 values. @@ -1751,9 +1751,8 @@ fn build_predicate_expression( max_in_list_size, properties, ); - } else { - return unhandled_hook.handle(expr); } + return unhandled_hook.handle(expr); } let (left, op, right) = { diff --git a/datafusion/sql/src/expr/function.rs b/datafusion/sql/src/expr/function.rs index c2259c714d84c..4417799d3c3e8 100644 --- a/datafusion/sql/src/expr/function.rs +++ b/datafusion/sql/src/expr/function.rs @@ -358,20 +358,19 @@ impl SqlToRel<'_, S> { if name.eq_ignore_ascii_case(inner.name()) { return Ok(Expr::ScalarFunction(inner)); - } else { - // If the function is called by an alias, a verbose string representation is created - // (e.g., "my_alias(arg1, arg2)") and the expression is wrapped in an `Alias` - // to ensure the output column name matches the user's query. - let arg_names = inner - .args - .iter() - .map(|arg| arg.to_string()) - .collect::>() - .join(","); - let verbose_alias = format!("{name}({arg_names})"); - - return Ok(Expr::ScalarFunction(inner).alias(verbose_alias)); } + // If the function is called by an alias, a verbose string representation is created + // (e.g., "my_alias(arg1, arg2)") and the expression is wrapped in an `Alias` + // to ensure the output column name matches the user's query. + let arg_names = inner + .args + .iter() + .map(|arg| arg.to_string()) + .collect::>() + .join(","); + let verbose_alias = format!("{name}({arg_names})"); + + return Ok(Expr::ScalarFunction(inner).alias(verbose_alias)); } if let Some(fm) = self.context_provider.get_higher_order_meta(&name) { @@ -536,20 +535,19 @@ impl SqlToRel<'_, S> { if name.eq_ignore_ascii_case(inner.name()) { return Ok(Expr::HigherOrderFunction(inner)); - } else { - // If the function is called by an alias, a verbose string representation is created - // (e.g., "my_alias(arg1, arg2)") and the expression is wrapped in an `Alias` - // to ensure the output column name matches the user's query. - let arg_names = inner - .args - .iter() - .map(|arg| arg.to_string()) - .collect::>() - .join(","); - let verbose_alias = format!("{name}({arg_names})"); - - return Ok(Expr::HigherOrderFunction(inner).alias(verbose_alias)); } + // If the function is called by an alias, a verbose string representation is created + // (e.g., "my_alias(arg1, arg2)") and the expression is wrapped in an `Alias` + // to ensure the output column name matches the user's query. + let arg_names = inner + .args + .iter() + .map(|arg| arg.to_string()) + .collect::>() + .join(","); + let verbose_alias = format!("{name}({arg_names})"); + + return Ok(Expr::HigherOrderFunction(inner).alias(verbose_alias)); } // Build Unnest expression. @@ -708,21 +706,20 @@ impl SqlToRel<'_, S> { if name.eq_ignore_ascii_case(inner.fun.name()) { return Ok(Expr::WindowFunction(Box::new(inner))); - } else { - // If the function is called by an alias, a verbose string representation is created - // (e.g., "my_alias(arg1, arg2)") and the expression is wrapped in an `Alias` - // to ensure the output column name matches the user's query. - let arg_names = inner - .params - .args - .iter() - .map(|arg| arg.to_string()) - .collect::>() - .join(","); - let verbose_alias = format!("{name}({arg_names})"); - - return Ok(Expr::WindowFunction(Box::new(inner)).alias(verbose_alias)); } + // If the function is called by an alias, a verbose string representation is created + // (e.g., "my_alias(arg1, arg2)") and the expression is wrapped in an `Alias` + // to ensure the output column name matches the user's query. + let arg_names = inner + .params + .args + .iter() + .map(|arg| arg.to_string()) + .collect::>() + .join(","); + let verbose_alias = format!("{name}({arg_names})"); + + return Ok(Expr::WindowFunction(Box::new(inner)).alias(verbose_alias)); } } else { // User defined aggregate functions (UDAF) have precedence in case it has the same name as a scalar built-in function @@ -864,21 +861,20 @@ impl SqlToRel<'_, S> { if name.eq_ignore_ascii_case(inner.func.name()) { return Ok(Expr::AggregateFunction(inner)); - } else { - // If the function is called by an alias, a verbose string representation is created - // (e.g., "my_alias(arg1, arg2)") and the expression is wrapped in an `Alias` - // to ensure the output column name matches the user's query. - let arg_names = inner - .params - .args - .iter() - .map(|arg| arg.to_string()) - .collect::>() - .join(","); - let verbose_alias = format!("{name}({arg_names})"); - - return Ok(Expr::AggregateFunction(inner).alias(verbose_alias)); } + // If the function is called by an alias, a verbose string representation is created + // (e.g., "my_alias(arg1, arg2)") and the expression is wrapped in an `Alias` + // to ensure the output column name matches the user's query. + let arg_names = inner + .params + .args + .iter() + .map(|arg| arg.to_string()) + .collect::>() + .join(","); + let verbose_alias = format!("{name}({arg_names})"); + + return Ok(Expr::AggregateFunction(inner).alias(verbose_alias)); } } @@ -890,19 +886,15 @@ impl SqlToRel<'_, S> { .map(|part| part.as_ident().cloned().ok_or(())) .collect::, ()>>(); if let Ok(ids) = maybe_ids { - if ids.len() == 1 { - return self.sql_identifier_to_expr( + return if ids.len() == 1 { + self.sql_identifier_to_expr( ids.into_iter().next().unwrap(), schema, planner_context, - ); + ) } else { - return self.sql_compound_identifier_to_expr( - ids, - schema, - planner_context, - ); - } + self.sql_compound_identifier_to_expr(ids, schema, planner_context) + }; } } diff --git a/datafusion/sql/src/expr/value.rs b/datafusion/sql/src/expr/value.rs index 1307e917e4251..d0354b319f089 100644 --- a/datafusion/sql/src/expr/value.rs +++ b/datafusion/sql/src/expr/value.rs @@ -296,9 +296,8 @@ fn interval_literal(interval_value: SQLExpr, negative: bool) -> Result { return not_impl_err!( "Unsupported interval argument. Long number not supported: {interval_value:?}" ); - } else { - v.to_string() } + v.to_string() } SQLExpr::UnaryOp { op, expr } => { let negative = match op { diff --git a/datafusion/sql/src/parser.rs b/datafusion/sql/src/parser.rs index fcf4708f1bf94..692d562a6088d 100644 --- a/datafusion/sql/src/parser.rs +++ b/datafusion/sql/src/parser.rs @@ -756,9 +756,8 @@ impl<'a> DFParser<'a> { let token = self.parser.peek_token(); if token == Token::EOF || token == Token::SemiColon { break; - } else { - return self.expected("end of statement or ;", &token)?; } + return self.expected("end of statement or ;", &token)?; } } @@ -1208,9 +1207,8 @@ impl<'a> DFParser<'a> { let token = self.parser.peek_token(); if token == Token::EOF || token == Token::SemiColon { break; - } else { - return self.expected("end of statement or ;", &token)?; } + return self.expected("end of statement or ;", &token)?; } } diff --git a/datafusion/sql/src/select.rs b/datafusion/sql/src/select.rs index bbd9d203eb124..85d6a254cce0f 100644 --- a/datafusion/sql/src/select.rs +++ b/datafusion/sql/src/select.rs @@ -818,57 +818,56 @@ impl SqlToRel<'_, S> { if unnest_columns.is_empty() { break; - } else { - let mut unnest_options = UnnestOptions::new().with_preserve_nulls(false); - - #[allow(clippy::allow_attributes, clippy::mutable_key_type)] - // Expr contains Arc with interior mutability but is intentionally used as hash key - let mut projection_exprs = match &aggr_expr_using_columns { - Some(exprs) => (*exprs).clone(), - None => { - #[allow(clippy::allow_attributes, clippy::mutable_key_type)] - let mut columns = HashSet::new(); - for expr in &aggr_expr { - expr.apply(|expr| { - if let Expr::Column(c) = expr { - columns.insert(Expr::Column(c.clone())); - } - Ok(TreeNodeRecursion::Continue) - }) - // As the closure always returns Ok, this "can't" error - .expect("Unexpected error"); - } - aggr_expr_using_columns = Some(columns.clone()); - columns - } - }; - projection_exprs.extend(inner_projection_exprs); - - let mut unnest_col_vec = vec![]; - - for (col, maybe_list_unnest) in unnest_columns.into_iter() { - if let Some(list_unnest) = maybe_list_unnest { - unnest_options = list_unnest.into_iter().fold( - unnest_options, - |options, unnest_list| { - options.with_recursions(RecursionUnnestOption { - input_column: col.clone(), - output_column: unnest_list.output_column, - depth: unnest_list.depth, - }) - }, - ); + } + let mut unnest_options = UnnestOptions::new().with_preserve_nulls(false); + + #[allow(clippy::allow_attributes, clippy::mutable_key_type)] + // Expr contains Arc with interior mutability but is intentionally used as hash key + let mut projection_exprs = match &aggr_expr_using_columns { + Some(exprs) => (*exprs).clone(), + None => { + #[allow(clippy::allow_attributes, clippy::mutable_key_type)] + let mut columns = HashSet::new(); + for expr in &aggr_expr { + expr.apply(|expr| { + if let Expr::Column(c) = expr { + columns.insert(Expr::Column(c.clone())); + } + Ok(TreeNodeRecursion::Continue) + }) + // As the closure always returns Ok, this "can't" error + .expect("Unexpected error"); } - unnest_col_vec.push(col); + aggr_expr_using_columns = Some(columns.clone()); + columns } + }; + projection_exprs.extend(inner_projection_exprs); - intermediate_plan = LogicalPlanBuilder::from(intermediate_plan) - .project(projection_exprs)? - .unnest_columns_with_options(unnest_col_vec, unnest_options)? - .build()?; + let mut unnest_col_vec = vec![]; - intermediate_select_exprs = outer_projection_exprs; + for (col, maybe_list_unnest) in unnest_columns.into_iter() { + if let Some(list_unnest) = maybe_list_unnest { + unnest_options = list_unnest.into_iter().fold( + unnest_options, + |options, unnest_list| { + options.with_recursions(RecursionUnnestOption { + input_column: col.clone(), + output_column: unnest_list.output_column, + depth: unnest_list.depth, + }) + }, + ); + } + unnest_col_vec.push(col); } + + intermediate_plan = LogicalPlanBuilder::from(intermediate_plan) + .project(projection_exprs)? + .unnest_columns_with_options(unnest_col_vec, unnest_options)? + .build()?; + + intermediate_select_exprs = outer_projection_exprs; } Ok((intermediate_plan, intermediate_select_exprs)) diff --git a/datafusion/sql/src/statement.rs b/datafusion/sql/src/statement.rs index 7c7e5edfa53bc..93d896201af47 100644 --- a/datafusion/sql/src/statement.rs +++ b/datafusion/sql/src/statement.rs @@ -2875,9 +2875,8 @@ impl SqlToRel<'_, S> { return schema_err!(SchemaError::DuplicateUnqualifiedField { name: c, }); - } else { - value_indices[column_index] = Some(i); } + value_indices[column_index] = Some(i); Ok(Arc::clone(table_schema.field(column_index))) }) .collect::>>()?; diff --git a/datafusion/substrait/src/logical_plan/consumer/expr/literal.rs b/datafusion/substrait/src/logical_plan/consumer/expr/literal.rs index b0756ef060ecf..7d8b8eb0c7130 100644 --- a/datafusion/substrait/src/logical_plan/consumer/expr/literal.rs +++ b/datafusion/substrait/src/logical_plan/consumer/expr/literal.rs @@ -393,9 +393,8 @@ pub(crate) fn from_substrait_literal( return substrait_err!( "Cannot set subseconds field of IntervalDayToSecond without setting precision" ); - } else { - 0_i32 } + 0_i32 } Some(PrecisionMode::Precision(0)) => *subseconds as i32 * 1000, Some(PrecisionMode::Precision(3)) => *subseconds as i32, From ee2e5bbd968b6491f6172f69efb1e82a6fa6a2ee Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 1 Sep 2026 13:51:19 +0200 Subject: [PATCH 4/6] Enable clippy::unnested_or_patterns Nested the or-patterns, e.g. `Time32(Microsecond) | Time32(Nanosecond)` -> `Time32(Microsecond | Nanosecond)`, so the shared prefix is written once. --- Cargo.toml | 1 - datafusion/common/src/dfschema.rs | 2 +- datafusion/common/src/scalar/mod.rs | 16 +++--- datafusion/common/src/stats.rs | 56 +++++++++---------- .../benches/preserve_file_partitioning.rs | 6 +- datafusion/core/src/execution/context/mod.rs | 4 +- datafusion/core/src/physical_planner.rs | 8 ++- datafusion/datasource-parquet/src/metadata.rs | 4 +- .../expr-common/src/type_coercion/binary.rs | 53 +++++++----------- datafusion/expr/src/higher_order_function.rs | 2 +- datafusion/expr/src/tree_node.rs | 3 +- .../functions-aggregate-common/src/min_max.rs | 18 ++++-- .../src/approx_distinct.rs | 25 +++++---- datafusion/functions-nested/src/array_has.rs | 4 +- datafusion/functions-nested/src/except.rs | 8 +-- datafusion/functions-nested/src/set_ops.rs | 7 +-- .../functions-table/src/generate_series.rs | 12 ++-- datafusion/functions/src/datetime/to_time.rs | 22 +++++--- datafusion/functions/src/math/round.rs | 2 +- datafusion/functions/src/strings.rs | 18 +++--- datafusion/optimizer/src/decorrelate.rs | 5 +- datafusion/optimizer/src/optimizer.rs | 31 +++++----- .../optimizer/src/propagate_empty_relation.rs | 3 +- .../simplify_predicates.rs | 32 +++++------ .../src/simplify_expressions/utils.rs | 7 +-- datafusion/optimizer/src/utils.rs | 7 +-- .../enforce_sorting/sort_pushdown.rs | 16 +++--- .../group_values/multi_group_by/mod.rs | 6 +- .../src/operator_statistics/mod.rs | 8 ++- datafusion/physical-plan/src/sorts/stream.rs | 2 +- .../spark/src/function/conversion/cast.rs | 8 ++- .../spark/src/function/datetime/date_part.rs | 8 ++- .../spark/src/function/datetime/date_trunc.rs | 8 ++- .../spark/src/function/datetime/time_trunc.rs | 8 ++- .../spark/src/function/datetime/trunc.rs | 8 ++- .../src/function/string/format_string.rs | 8 ++- .../spark/src/function/string/luhn_check.rs | 22 ++++---- datafusion/sql/src/expr/function.rs | 6 +- datafusion/sql/src/expr/mod.rs | 10 ++-- datafusion/sql/src/statement.rs | 2 +- datafusion/sql/tests/sql_integration.rs | 4 +- .../tests/cases/roundtrip_logical_plan.rs | 2 +- 42 files changed, 242 insertions(+), 240 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index e1846648632a3..a943e58b9a583 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -308,7 +308,6 @@ too_many_lines = "allow" # 484 hits trivially_copy_pass_by_ref = "allow" # 74 hits unnecessary_literal_bound = "allow" # 471 hits unnecessary_wraps = "allow" # 427 hits -unnested_or_patterns = "allow" # 68 hits unreadable_literal = "allow" # 502 hits unused_self = "allow" # 69 hits used_underscore_items = "allow" # 28 hits diff --git a/datafusion/common/src/dfschema.rs b/datafusion/common/src/dfschema.rs index a0e2f0590a628..9fd06cd92ca75 100644 --- a/datafusion/common/src/dfschema.rs +++ b/datafusion/common/src/dfschema.rs @@ -395,7 +395,7 @@ impl DFSchema { // field to lookup is qualified but current field is unqualified. (Some(_), None) => false, // field to lookup is unqualified, no need to compare qualifier - (None, Some(_)) | (None, None) => f.name() == name, + (None, Some(_) | None) => f.name() == name, }) .map(|(idx, _)| idx); matches.next() diff --git a/datafusion/common/src/scalar/mod.rs b/datafusion/common/src/scalar/mod.rs index 07579c60b292a..efb1aa99cbdd8 100644 --- a/datafusion/common/src/scalar/mod.rs +++ b/datafusion/common/src/scalar/mod.rs @@ -723,11 +723,11 @@ impl PartialOrd for ScalarValue { (LargeListView(arr1), LargeListView(arr2)) => { partial_cmp_list(arr1.as_ref(), arr2.as_ref()) } - (List(_), _) - | (LargeList(_), _) - | (FixedSizeList(_), _) - | (ListView(_), _) - | (LargeListView(_), _) => None, + ( + List(_) | LargeList(_) | FixedSizeList(_) | ListView(_) + | LargeListView(_), + _, + ) => None, (Struct(struct_arr1), Struct(struct_arr2)) => { partial_cmp_struct(struct_arr1.as_ref(), struct_arr2.as_ref()) } @@ -3191,10 +3191,8 @@ impl ScalarValue { // not supported if the TimeUnit is not valid (Time32 can // only be used with Second and Millisecond, Time64 only // with Microsecond and Nanosecond) - DataType::Time32(TimeUnit::Microsecond) - | DataType::Time32(TimeUnit::Nanosecond) - | DataType::Time64(TimeUnit::Second) - | DataType::Time64(TimeUnit::Millisecond) => { + DataType::Time32(TimeUnit::Microsecond | TimeUnit::Nanosecond) + | DataType::Time64(TimeUnit::Second | TimeUnit::Millisecond) => { return _not_impl_err!( "Unsupported creation of {:?} array from ScalarValue {:?}", data_type, diff --git a/datafusion/common/src/stats.rs b/datafusion/common/src/stats.rs index 1df71e15a277f..5188d251b23b3 100644 --- a/datafusion/common/src/stats.rs +++ b/datafusion/common/src/stats.rs @@ -102,9 +102,8 @@ impl Precision { (Precision::Exact(a), Precision::Exact(b)) => { Precision::Exact(if a >= b { a.clone() } else { b.clone() }) } - (Precision::Inexact(a), Precision::Exact(b)) - | (Precision::Exact(a), Precision::Inexact(b)) - | (Precision::Inexact(a), Precision::Inexact(b)) => { + (Precision::Inexact(a), Precision::Exact(b) | Precision::Inexact(b)) + | (Precision::Exact(a), Precision::Inexact(b)) => { Precision::Inexact(if a >= b { a.clone() } else { b.clone() }) } (_, _) => Precision::Absent, @@ -119,9 +118,8 @@ impl Precision { (Precision::Exact(a), Precision::Exact(b)) => { Precision::Exact(if a >= b { b.clone() } else { a.clone() }) } - (Precision::Inexact(a), Precision::Exact(b)) - | (Precision::Exact(a), Precision::Inexact(b)) - | (Precision::Inexact(a), Precision::Inexact(b)) => { + (Precision::Inexact(a), Precision::Exact(b) | Precision::Inexact(b)) + | (Precision::Exact(a), Precision::Inexact(b)) => { Precision::Inexact(if a >= b { b.clone() } else { a.clone() }) } (_, _) => Precision::Absent, @@ -147,9 +145,8 @@ impl Precision { || Precision::Inexact(a.saturating_add(*b)), Precision::Exact, ), - (Precision::Inexact(a), Precision::Exact(b)) - | (Precision::Exact(a), Precision::Inexact(b)) - | (Precision::Inexact(a), Precision::Inexact(b)) => { + (Precision::Inexact(a), Precision::Exact(b) | Precision::Inexact(b)) + | (Precision::Exact(a), Precision::Inexact(b)) => { Precision::Inexact(a.saturating_add(*b)) } (_, _) => Precision::Absent, @@ -165,9 +162,8 @@ impl Precision { || Precision::Inexact(a.saturating_sub(*b)), Precision::Exact, ), - (Precision::Inexact(a), Precision::Exact(b)) - | (Precision::Exact(a), Precision::Inexact(b)) - | (Precision::Inexact(a), Precision::Inexact(b)) => { + (Precision::Inexact(a), Precision::Exact(b) | Precision::Inexact(b)) + | (Precision::Exact(a), Precision::Inexact(b)) => { Precision::Inexact(a.saturating_sub(*b)) } (_, _) => Precision::Absent, @@ -183,9 +179,8 @@ impl Precision { || Precision::Inexact(a.saturating_mul(*b)), Precision::Exact, ), - (Precision::Inexact(a), Precision::Exact(b)) - | (Precision::Exact(a), Precision::Inexact(b)) - | (Precision::Inexact(a), Precision::Inexact(b)) => { + (Precision::Inexact(a), Precision::Exact(b) | Precision::Inexact(b)) + | (Precision::Exact(a), Precision::Inexact(b)) => { Precision::Inexact(a.saturating_mul(*b)) } (_, _) => Precision::Absent, @@ -240,9 +235,8 @@ impl Precision { .add_checked(b) .map(Precision::Exact) .unwrap_or(Precision::Absent), - (Precision::Inexact(a), Precision::Exact(b)) - | (Precision::Exact(a), Precision::Inexact(b)) - | (Precision::Inexact(a), Precision::Inexact(b)) => a + (Precision::Inexact(a), Precision::Exact(b) | Precision::Inexact(b)) + | (Precision::Exact(a), Precision::Inexact(b)) => a .add_checked(b) .map(Precision::Inexact) .unwrap_or(Precision::Absent), @@ -283,9 +277,8 @@ impl Precision { (Precision::Exact(a), Precision::Exact(b)) => { a.sub(b).map(Precision::Exact).unwrap_or(Precision::Absent) } - (Precision::Inexact(a), Precision::Exact(b)) - | (Precision::Exact(a), Precision::Inexact(b)) - | (Precision::Inexact(a), Precision::Inexact(b)) => a + (Precision::Inexact(a), Precision::Exact(b) | Precision::Inexact(b)) + | (Precision::Exact(a), Precision::Inexact(b)) => a .sub(b) .map(Precision::Inexact) .unwrap_or(Precision::Absent), @@ -302,9 +295,8 @@ impl Precision { .mul_checked(b) .map(Precision::Exact) .unwrap_or(Precision::Absent), - (Precision::Inexact(a), Precision::Exact(b)) - | (Precision::Exact(a), Precision::Inexact(b)) - | (Precision::Inexact(a), Precision::Inexact(b)) => a + (Precision::Inexact(a), Precision::Exact(b) | Precision::Inexact(b)) + | (Precision::Exact(a), Precision::Inexact(b)) => a .mul_checked(b) .map(Precision::Inexact) .unwrap_or(Precision::Absent), @@ -906,9 +898,11 @@ where Precision::Exact(right.clone()) } } - (Precision::Exact(left), Precision::Inexact(right)) - | (Precision::Inexact(left), Precision::Exact(right)) - | (Precision::Inexact(left), Precision::Inexact(right)) => { + ( + Precision::Exact(left) | Precision::Inexact(left), + Precision::Inexact(right), + ) + | (Precision::Inexact(left), Precision::Exact(right)) => { if left <= *right { Precision::Inexact(left) } else { @@ -934,9 +928,11 @@ where Precision::Exact(right.clone()) } } - (Precision::Exact(left), Precision::Inexact(right)) - | (Precision::Inexact(left), Precision::Exact(right)) - | (Precision::Inexact(left), Precision::Inexact(right)) => { + ( + Precision::Exact(left) | Precision::Inexact(left), + Precision::Inexact(right), + ) + | (Precision::Inexact(left), Precision::Exact(right)) => { if left >= *right { Precision::Inexact(left) } else { diff --git a/datafusion/core/benches/preserve_file_partitioning.rs b/datafusion/core/benches/preserve_file_partitioning.rs index c459853d5e05c..0ec4fe508af34 100644 --- a/datafusion/core/benches/preserve_file_partitioning.rs +++ b/datafusion/core/benches/preserve_file_partitioning.rs @@ -88,9 +88,9 @@ impl BenchConfig { fn from_env() -> Self { match std::env::var("BENCH_SIZE").as_deref() { - Ok("small") | Ok("SMALL") => Self::small(), - Ok("medium") | Ok("MEDIUM") => Self::medium(), - Ok("large") | Ok("LARGE") => Self::large(), + Ok("small" | "SMALL") => Self::small(), + Ok("medium" | "MEDIUM") => Self::medium(), + Ok("large" | "LARGE") => Self::large(), _ => { println!("Using SMALL dataset (set BENCH_SIZE=small|medium|large)"); Self::small() diff --git a/datafusion/core/src/execution/context/mod.rs b/datafusion/core/src/execution/context/mod.rs index ff1ad25811440..8dee606b037e6 100644 --- a/datafusion/core/src/execution/context/mod.rs +++ b/datafusion/core/src/execution/context/mod.rs @@ -1012,7 +1012,7 @@ impl SessionContext { match (if_not_exists, schema) { (true, Some(_)) => self.return_empty_dataframe(), - (true, None) | (false, None) => { + (_, None) => { let schema = Arc::new(MemorySchemaProvider::new()); catalog.register_schema(schema_name, schema)?; self.return_empty_dataframe() @@ -1031,7 +1031,7 @@ impl SessionContext { match (if_not_exists, catalog) { (true, Some(_)) => self.return_empty_dataframe(), - (true, None) | (false, None) => { + (_, None) => { let new_catalog = Arc::new(MemoryCatalogProvider::new()); self.state .write() diff --git a/datafusion/core/src/physical_planner.rs b/datafusion/core/src/physical_planner.rs index 882956a63c114..4dd4168c10bbe 100644 --- a/datafusion/core/src/physical_planner.rs +++ b/datafusion/core/src/physical_planner.rs @@ -2481,9 +2481,11 @@ fn is_identity_assignment(expr: &Expr, column_name: &str) -> bool { /// OVER (ORDER BY a RANGES BETWEEN INTERVAL '3 DAY' PRECEDING AND '5 DAY' PRECEDING) are rejected pub fn is_window_frame_bound_valid(window_frame: &WindowFrame) -> bool { match (&window_frame.start_bound, &window_frame.end_bound) { - (WindowFrameBound::Following(_), WindowFrameBound::Preceding(_)) - | (WindowFrameBound::Following(_), WindowFrameBound::CurrentRow) - | (WindowFrameBound::CurrentRow, WindowFrameBound::Preceding(_)) => false, + ( + WindowFrameBound::Following(_) | WindowFrameBound::CurrentRow, + WindowFrameBound::Preceding(_), + ) + | (WindowFrameBound::Following(_), WindowFrameBound::CurrentRow) => false, (WindowFrameBound::Preceding(lhs), WindowFrameBound::Preceding(rhs)) => { !rhs.is_null() && (lhs.is_null() || (lhs >= rhs)) } diff --git a/datafusion/datasource-parquet/src/metadata.rs b/datafusion/datasource-parquet/src/metadata.rs index 213cac24a85be..13266cd24d40c 100644 --- a/datafusion/datasource-parquet/src/metadata.rs +++ b/datafusion/datasource-parquet/src/metadata.rs @@ -699,7 +699,7 @@ impl StatisticsAccumulators<'_> { (Some(max_value), Some(true)) => { max_value.evaluate().ok().map(Precision::Exact) } - (Some(max_value), Some(false)) | (Some(max_value), None) => { + (Some(max_value), Some(false) | None) => { max_value.evaluate().ok().map(Precision::Inexact) } (None, _) => None, @@ -711,7 +711,7 @@ impl StatisticsAccumulators<'_> { (Some(min_value), Some(true)) => { min_value.evaluate().ok().map(Precision::Exact) } - (Some(min_value), Some(false)) | (Some(min_value), None) => { + (Some(min_value), Some(false) | None) => { min_value.evaluate().ok().map(Precision::Inexact) } (None, _) => None, diff --git a/datafusion/expr-common/src/type_coercion/binary.rs b/datafusion/expr-common/src/type_coercion/binary.rs index 381897ae86fdc..f5d6dea7c84af 100644 --- a/datafusion/expr-common/src/type_coercion/binary.rs +++ b/datafusion/expr-common/src/type_coercion/binary.rs @@ -526,18 +526,11 @@ fn bitwise_coercion(left_type: &DataType, right_type: &DataType) -> Option Some(UInt64), (Int64, _) | (_, Int64) - | (UInt32, Int8) - | (Int8, UInt32) - | (UInt32, Int16) - | (Int16, UInt32) - | (UInt32, Int32) - | (Int32, UInt32) => Some(Int64), - (Int32, _) - | (_, Int32) - | (UInt16, Int16) - | (Int16, UInt16) - | (UInt16, Int8) - | (Int8, UInt16) => Some(Int32), + | (UInt32, Int8 | Int16 | Int32) + | (Int8 | Int16 | Int32, UInt32) => Some(Int64), + (Int32, _) | (_, Int32) | (UInt16, Int16 | Int8) | (Int16 | Int8, UInt16) => { + Some(Int32) + } (UInt32, _) | (_, UInt32) => Some(UInt32), (Int16, _) | (_, Int16) | (Int8, UInt8) | (UInt8, Int8) => Some(Int16), (UInt16, _) | (_, UInt16) => Some(UInt16), @@ -1023,20 +1016,18 @@ fn string_temporal_coercion( fn match_rule(l: &DataType, r: &DataType) -> Option { match (l, r) { // Coerce Utf8View/Utf8/LargeUtf8 to Date32/Date64/Time32/Time64/Timestamp - (Utf8, temporal) | (LargeUtf8, temporal) | (Utf8View, temporal) => { - match temporal { - Date32 | Date64 => Some(temporal.clone()), - Time32(_) | Time64(_) => { - if is_time_with_valid_unit(temporal) { - Some(temporal.to_owned()) - } else { - None - } + (Utf8 | LargeUtf8 | Utf8View, temporal) => match temporal { + Date32 | Date64 => Some(temporal.clone()), + Time32(_) | Time64(_) => { + if is_time_with_valid_unit(temporal) { + Some(temporal.to_owned()) + } else { + None } - Timestamp(_, tz) => Some(Timestamp(Nanosecond, tz.clone())), - _ => None, } - } + Timestamp(_, tz) => Some(Timestamp(Nanosecond, tz.clone())), + _ => None, + }, _ => None, } } @@ -1134,20 +1125,14 @@ fn get_wider_decimal_type_cross_variant( { Some(Decimal64(required_precision, s)) } - (Decimal32(_, _), Decimal128(_, _)) - | (Decimal128(_, _), Decimal32(_, _)) - | (Decimal64(_, _), Decimal128(_, _)) - | (Decimal128(_, _), Decimal64(_, _)) + (Decimal32(_, _) | Decimal64(_, _), Decimal128(_, _)) + | (Decimal128(_, _), Decimal32(_, _) | Decimal64(_, _)) if required_precision <= DECIMAL128_MAX_PRECISION => { Some(Decimal128(required_precision, s)) } - (Decimal32(_, _), Decimal256(_, _)) - | (Decimal256(_, _), Decimal32(_, _)) - | (Decimal64(_, _), Decimal256(_, _)) - | (Decimal256(_, _), Decimal64(_, _)) - | (Decimal128(_, _), Decimal256(_, _)) - | (Decimal256(_, _), Decimal128(_, _)) + (Decimal32(_, _) | Decimal64(_, _) | Decimal128(_, _), Decimal256(_, _)) + | (Decimal256(_, _), Decimal32(_, _) | Decimal64(_, _) | Decimal128(_, _)) if required_precision <= DECIMAL256_MAX_PRECISION => { Some(Decimal256(required_precision, s)) diff --git a/datafusion/expr/src/higher_order_function.rs b/datafusion/expr/src/higher_order_function.rs index 9744e5520584a..6da74c12e8f64 100644 --- a/datafusion/expr/src/higher_order_function.rs +++ b/datafusion/expr/src/higher_order_function.rs @@ -1580,7 +1580,7 @@ mod tests { None, ]) } - (1, Some(accumulator)) | (0, Some(accumulator)) => { + (0 | 1, Some(accumulator)) => { // now we can use the merge output as it's accumulator and // as the finish parameter LambdaParametersProgress::Complete(vec![ diff --git a/datafusion/expr/src/tree_node.rs b/datafusion/expr/src/tree_node.rs index 941fd22ea179f..c9e83e0a46f50 100644 --- a/datafusion/expr/src/tree_node.rs +++ b/datafusion/expr/src/tree_node.rs @@ -64,8 +64,7 @@ impl TreeNode for Expr { | Expr::TryCast(TryCast { expr, .. }) | Expr::InSubquery(InSubquery { expr, .. }) | Expr::SetComparison(SetComparison { expr, .. }) => expr.apply_elements(f), - Expr::GroupingSet(GroupingSet::Rollup(exprs)) - | Expr::GroupingSet(GroupingSet::Cube(exprs)) => exprs.apply_elements(f), + Expr::GroupingSet(GroupingSet::Rollup(exprs) | GroupingSet::Cube(exprs)) => exprs.apply_elements(f), Expr::ScalarFunction(ScalarFunction { args, .. }) => { args.apply_elements(f) } diff --git a/datafusion/functions-aggregate-common/src/min_max.rs b/datafusion/functions-aggregate-common/src/min_max.rs index f0a4a52a4a060..d718415d3a596 100644 --- a/datafusion/functions-aggregate-common/src/min_max.rs +++ b/datafusion/functions-aggregate-common/src/min_max.rs @@ -399,12 +399,18 @@ fn min_max_scalar_same_variant( ordering, )) } - (ScalarValue::IntervalYearMonth(_), ScalarValue::IntervalMonthDayNano(_)) - | (ScalarValue::IntervalYearMonth(_), ScalarValue::IntervalDayTime(_)) - | (ScalarValue::IntervalMonthDayNano(_), ScalarValue::IntervalDayTime(_)) - | (ScalarValue::IntervalMonthDayNano(_), ScalarValue::IntervalYearMonth(_)) - | (ScalarValue::IntervalDayTime(_), ScalarValue::IntervalYearMonth(_)) - | (ScalarValue::IntervalDayTime(_), ScalarValue::IntervalMonthDayNano(_)) => { + ( + ScalarValue::IntervalYearMonth(_) | ScalarValue::IntervalDayTime(_), + ScalarValue::IntervalMonthDayNano(_), + ) + | ( + ScalarValue::IntervalYearMonth(_) | ScalarValue::IntervalMonthDayNano(_), + ScalarValue::IntervalDayTime(_), + ) + | ( + ScalarValue::IntervalMonthDayNano(_) | ScalarValue::IntervalDayTime(_), + ScalarValue::IntervalYearMonth(_), + ) => { return min_max_interval_scalar(lhs, rhs, ordering); } (ScalarValue::DurationSecond(lhs), ScalarValue::DurationSecond(rhs)) => { diff --git a/datafusion/functions-aggregate/src/approx_distinct.rs b/datafusion/functions-aggregate/src/approx_distinct.rs index 672150ee9b67e..43f3672adc2e7 100644 --- a/datafusion/functions-aggregate/src/approx_distinct.rs +++ b/datafusion/functions-aggregate/src/approx_distinct.rs @@ -915,17 +915,20 @@ fn is_hll_groups_type(data_type: &DataType) -> bool { | DataType::Int64 | DataType::Date32 | DataType::Date64 - | DataType::Time32(TimeUnit::Second) - | DataType::Time32(TimeUnit::Millisecond) - | DataType::Time64(TimeUnit::Microsecond) - | DataType::Time64(TimeUnit::Nanosecond) - | DataType::Timestamp(TimeUnit::Second, _) - | DataType::Timestamp(TimeUnit::Millisecond, _) - | DataType::Timestamp(TimeUnit::Microsecond, _) - | DataType::Timestamp(TimeUnit::Nanosecond, _) - | DataType::Interval(IntervalUnit::YearMonth) - | DataType::Interval(IntervalUnit::DayTime) - | DataType::Interval(IntervalUnit::MonthDayNano) + | DataType::Time32(TimeUnit::Second | TimeUnit::Millisecond) + | DataType::Time64(TimeUnit::Microsecond | TimeUnit::Nanosecond) + | DataType::Timestamp( + TimeUnit::Second + | TimeUnit::Millisecond + | TimeUnit::Microsecond + | TimeUnit::Nanosecond, + _ + ) + | DataType::Interval( + IntervalUnit::YearMonth + | IntervalUnit::DayTime + | IntervalUnit::MonthDayNano + ) | DataType::Decimal32(_, _) | DataType::Decimal64(_, _) | DataType::Decimal128(_, _) diff --git a/datafusion/functions-nested/src/array_has.rs b/datafusion/functions-nested/src/array_has.rs index 156f9477f5491..4f81a4fcbc1df 100644 --- a/datafusion/functions-nested/src/array_has.rs +++ b/datafusion/functions-nested/src/array_has.rs @@ -141,9 +141,9 @@ impl ScalarUDFImpl for ArrayHas { None, ))); } + // FixedSizeList gets coerced to List Expr::Literal( - // FixedSizeList gets coerced to List - scalar @ ScalarValue::List(_) | scalar @ ScalarValue::LargeList(_), + scalar @ (ScalarValue::List(_) | ScalarValue::LargeList(_)), _, ) => { if let Ok(scalar_values) = diff --git a/datafusion/functions-nested/src/except.rs b/datafusion/functions-nested/src/except.rs index 737a3122bbbae..f5b746d93bd15 100644 --- a/datafusion/functions-nested/src/except.rs +++ b/datafusion/functions-nested/src/except.rs @@ -138,10 +138,10 @@ fn array_except_inner(args: &[ArrayRef]) -> Result { &DataType::new_list(DataType::Null, true), len, )), - (DataType::Null, dt @ DataType::List(_)) - | (DataType::Null, dt @ DataType::LargeList(_)) - | (dt @ DataType::List(_), DataType::Null) - | (dt @ DataType::LargeList(_), DataType::Null) => Ok(new_null_array(dt, len)), + (DataType::Null, dt @ (DataType::List(_) | DataType::LargeList(_))) + | (dt @ (DataType::List(_) | DataType::LargeList(_)), DataType::Null) => { + Ok(new_null_array(dt, len)) + } (DataType::List(field), DataType::List(_)) => { check_datatypes("array_except", &[array1, array2])?; let list1 = array1.as_list::(); diff --git a/datafusion/functions-nested/src/set_ops.rs b/datafusion/functions-nested/src/set_ops.rs index 2214d3d35bb7b..f8991246eac6a 100644 --- a/datafusion/functions-nested/src/set_ops.rs +++ b/datafusion/functions-nested/src/set_ops.rs @@ -519,10 +519,9 @@ fn general_set_op( let len = array1.len(); match (array1.data_type(), array2.data_type()) { (Null, Null) => Ok(new_null_array(&DataType::new_list(Null, true), len)), - (Null, dt @ List(_)) - | (Null, dt @ LargeList(_)) - | (dt @ List(_), Null) - | (dt @ LargeList(_), Null) => Ok(new_null_array(dt, len)), + (Null, dt @ (List(_) | LargeList(_))) | (dt @ (List(_) | LargeList(_)), Null) => { + Ok(new_null_array(dt, len)) + } (List(field), List(_)) => { let array1 = as_list_array(&array1)?; let array2 = as_list_array(&array2)?; diff --git a/datafusion/functions-table/src/generate_series.rs b/datafusion/functions-table/src/generate_series.rs index cf5e026581584..5806e07643728 100644 --- a/datafusion/functions-table/src/generate_series.rs +++ b/datafusion/functions-table/src/generate_series.rs @@ -758,8 +758,7 @@ impl GenerateSeriesFuncImpl { // Parse start date let start_date = match &exprs[0] { Expr::Literal(ScalarValue::Date32(Some(date)), _) => *date, - Expr::Literal(ScalarValue::Date32(None), _) - | Expr::Literal(ScalarValue::Null, _) => { + Expr::Literal(ScalarValue::Date32(None) | ScalarValue::Null, _) => { return Ok(Arc::new(GenerateSeriesTable { schema, args: GenSeriesArgs::ContainsNull { name: self.name }, @@ -776,8 +775,7 @@ impl GenerateSeriesFuncImpl { // Parse end date let end_date = match &exprs[1] { Expr::Literal(ScalarValue::Date32(Some(date)), _) => *date, - Expr::Literal(ScalarValue::Date32(None), _) - | Expr::Literal(ScalarValue::Null, _) => { + Expr::Literal(ScalarValue::Date32(None) | ScalarValue::Null, _) => { return Ok(Arc::new(GenerateSeriesTable { schema, args: GenSeriesArgs::ContainsNull { name: self.name }, @@ -796,8 +794,10 @@ impl GenerateSeriesFuncImpl { Expr::Literal(ScalarValue::IntervalMonthDayNano(Some(interval)), _) => { *interval } - Expr::Literal(ScalarValue::IntervalMonthDayNano(None), _) - | Expr::Literal(ScalarValue::Null, _) => { + Expr::Literal( + ScalarValue::IntervalMonthDayNano(None) | ScalarValue::Null, + _, + ) => { return Ok(Arc::new(GenerateSeriesTable { schema, args: GenSeriesArgs::ContainsNull { name: self.name }, diff --git a/datafusion/functions/src/datetime/to_time.rs b/datafusion/functions/src/datetime/to_time.rs index f5fe59cbb87b0..85eb116cca92c 100644 --- a/datafusion/functions/src/datetime/to_time.rs +++ b/datafusion/functions/src/datetime/to_time.rs @@ -144,9 +144,9 @@ fn string_to_time(args: &[ColumnarValue]) -> Result { let formats = compile_formats(&formats); match &args[0] { - ColumnarValue::Scalar(ScalarValue::Utf8(s)) - | ColumnarValue::Scalar(ScalarValue::LargeUtf8(s)) - | ColumnarValue::Scalar(ScalarValue::Utf8View(s)) => { + ColumnarValue::Scalar( + ScalarValue::Utf8(s) | ScalarValue::LargeUtf8(s) | ScalarValue::Utf8View(s), + ) => { let result = s .as_ref() .map(|s| parse_time_with_formats(s, &formats)) @@ -175,14 +175,18 @@ fn collect_formats(args: &[ColumnarValue]) -> Result> { let mut formats = Vec::with_capacity(args.len() - 1); for (i, arg) in args[1..].iter().enumerate() { match arg { - ColumnarValue::Scalar(ScalarValue::Utf8(Some(s))) - | ColumnarValue::Scalar(ScalarValue::LargeUtf8(Some(s))) - | ColumnarValue::Scalar(ScalarValue::Utf8View(Some(s))) => { + ColumnarValue::Scalar( + ScalarValue::Utf8(Some(s)) + | ScalarValue::LargeUtf8(Some(s)) + | ScalarValue::Utf8View(Some(s)), + ) => { formats.push(s.as_str()); } - ColumnarValue::Scalar(ScalarValue::Utf8(None)) - | ColumnarValue::Scalar(ScalarValue::LargeUtf8(None)) - | ColumnarValue::Scalar(ScalarValue::Utf8View(None)) => { + ColumnarValue::Scalar( + ScalarValue::Utf8(None) + | ScalarValue::LargeUtf8(None) + | ScalarValue::Utf8View(None), + ) => { // Skip null format strings } ColumnarValue::Array(_) => { diff --git a/datafusion/functions/src/math/round.rs b/datafusion/functions/src/math/round.rs index 02c808a4d346e..1ef0919b577d6 100644 --- a/datafusion/functions/src/math/round.rs +++ b/datafusion/functions/src/math/round.rs @@ -1127,7 +1127,7 @@ mod test { assert!(result.is_err()); assert!(matches!( result, - Err(DataFusionError::ArrowError(_, _)) | Err(DataFusionError::Execution(_)) + Err(DataFusionError::ArrowError(_, _) | DataFusionError::Execution(_)) )); } } diff --git a/datafusion/functions/src/strings.rs b/datafusion/functions/src/strings.rs index c788c6fb1f33f..9df357f2a7e73 100644 --- a/datafusion/functions/src/strings.rs +++ b/datafusion/functions/src/strings.rs @@ -1284,9 +1284,11 @@ impl ColumnarValueRef<'_> { convert_to_str: bool, ) -> Result>> { match col { - ColumnarValue::Scalar(ScalarValue::Utf8(maybe_value)) - | ColumnarValue::Scalar(ScalarValue::LargeUtf8(maybe_value)) - | ColumnarValue::Scalar(ScalarValue::Utf8View(maybe_value)) => { + ColumnarValue::Scalar( + ScalarValue::Utf8(maybe_value) + | ScalarValue::LargeUtf8(maybe_value) + | ScalarValue::Utf8View(maybe_value), + ) => { if let Some(s) = maybe_value { *data_size += s.len() * len * size_factor; Ok(Some(ColumnarValueRef::Scalar(s.as_bytes()))) @@ -1294,10 +1296,12 @@ impl ColumnarValueRef<'_> { Ok(None) } } - ColumnarValue::Scalar(ScalarValue::Binary(maybe_value)) - | ColumnarValue::Scalar(ScalarValue::LargeBinary(maybe_value)) - | ColumnarValue::Scalar(ScalarValue::BinaryView(maybe_value)) - | ColumnarValue::Scalar(ScalarValue::FixedSizeBinary(_, maybe_value)) => { + ColumnarValue::Scalar( + ScalarValue::Binary(maybe_value) + | ScalarValue::LargeBinary(maybe_value) + | ScalarValue::BinaryView(maybe_value) + | ScalarValue::FixedSizeBinary(_, maybe_value), + ) => { if let Some(b) = maybe_value { *data_size += b.len() * len * size_factor; Ok(Some(ColumnarValueRef::Scalar(b.as_slice()))) diff --git a/datafusion/optimizer/src/decorrelate.rs b/datafusion/optimizer/src/decorrelate.rs index 0c37f00b64355..e26b50454e4dc 100644 --- a/datafusion/optimizer/src/decorrelate.rs +++ b/datafusion/optimizer/src/decorrelate.rs @@ -618,8 +618,9 @@ fn filter_exprs_evaluation_result_on_empty_batch( let result_expr = simplifier.simplify(result_expr)?; match &result_expr { // evaluate to false or null on empty batch, no need to pull up - Expr::Literal(ScalarValue::Null, _) - | Expr::Literal(ScalarValue::Boolean(Some(false)), _) => None, + Expr::Literal(ScalarValue::Null | ScalarValue::Boolean(Some(false)), _) => { + None + } // evaluate to true on empty batch, need to pull up the expr Expr::Literal(ScalarValue::Boolean(Some(true)), _) => { for (name, exprs) in input_expr_result_map_for_count_bug { diff --git a/datafusion/optimizer/src/optimizer.rs b/datafusion/optimizer/src/optimizer.rs index ca4a6688b50c5..fca7f85ee4ee9 100644 --- a/datafusion/optimizer/src/optimizer.rs +++ b/datafusion/optimizer/src/optimizer.rs @@ -428,13 +428,10 @@ fn map_children_mut Result>( f(Arc::make_mut(input))? } LogicalPlan::Explain(Explain { plan, .. }) => f(Arc::make_mut(plan))?, - LogicalPlan::Ddl(DdlStatement::CreateMemoryTable(CreateMemoryTable { - input, - .. - })) - | LogicalPlan::Ddl(DdlStatement::CreateView(CreateView { input, .. })) => { - f(Arc::make_mut(input))? - } + LogicalPlan::Ddl( + DdlStatement::CreateMemoryTable(CreateMemoryTable { input, .. }) + | DdlStatement::CreateView(CreateView { input, .. }), + ) => f(Arc::make_mut(input))?, LogicalPlan::RecursiveQuery(RecursiveQuery { static_term, recursive_term, @@ -475,15 +472,17 @@ fn map_children_mut Result>( | LogicalPlan::EmptyRelation { .. } | LogicalPlan::Values { .. } | LogicalPlan::DescribeTable(_) - | LogicalPlan::Ddl(DdlStatement::CreateExternalTable(_)) - | LogicalPlan::Ddl(DdlStatement::CreateCatalogSchema(_)) - | LogicalPlan::Ddl(DdlStatement::CreateCatalog(_)) - | LogicalPlan::Ddl(DdlStatement::CreateIndex(_)) - | LogicalPlan::Ddl(DdlStatement::DropTable(_)) - | LogicalPlan::Ddl(DdlStatement::DropView(_)) - | LogicalPlan::Ddl(DdlStatement::DropCatalogSchema(_)) - | LogicalPlan::Ddl(DdlStatement::CreateFunction(_)) - | LogicalPlan::Ddl(DdlStatement::DropFunction(_)) + | LogicalPlan::Ddl( + DdlStatement::CreateExternalTable(_) + | DdlStatement::CreateCatalogSchema(_) + | DdlStatement::CreateCatalog(_) + | DdlStatement::CreateIndex(_) + | DdlStatement::DropTable(_) + | DdlStatement::DropView(_) + | DdlStatement::DropCatalogSchema(_) + | DdlStatement::CreateFunction(_) + | DdlStatement::DropFunction(_), + ) | LogicalPlan::Statement(_) => false, }) } diff --git a/datafusion/optimizer/src/propagate_empty_relation.rs b/datafusion/optimizer/src/propagate_empty_relation.rs index 18ddc361a0692..69fbdab408910 100644 --- a/datafusion/optimizer/src/propagate_empty_relation.rs +++ b/datafusion/optimizer/src/propagate_empty_relation.rs @@ -339,8 +339,7 @@ fn has_empty_grouping_set(group_expr: &[Expr]) -> bool { groups.iter().any(|g| g.is_empty()) } // Both ROLLUP and CUBE always include the empty grouping set (). - Some(Expr::GroupingSet(GroupingSet::Rollup(_))) - | Some(Expr::GroupingSet(GroupingSet::Cube(_))) => true, + Some(Expr::GroupingSet(GroupingSet::Rollup(_) | GroupingSet::Cube(_))) => true, _ => false, } } diff --git a/datafusion/optimizer/src/simplify_expressions/simplify_predicates.rs b/datafusion/optimizer/src/simplify_expressions/simplify_predicates.rs index e7edc34cfe4e6..b29a578a45e60 100644 --- a/datafusion/optimizer/src/simplify_expressions/simplify_predicates.rs +++ b/datafusion/optimizer/src/simplify_expressions/simplify_predicates.rs @@ -285,20 +285,20 @@ mod tests { // Check that the cast predicate is preserved let has_cast_predicate = result.iter().any(|p| { - matches!(p, Expr::BinaryExpr(BinaryExpr { - left, - op: Operator::Lt, - right + matches!(p, Expr::BinaryExpr(BinaryExpr { + left, + op: Operator::Lt, + right }) if matches!(left.as_ref(), Expr::Cast(_)) && right == &Box::new(lit("abc"))) }); assert!(has_cast_predicate, "Cast predicate should be preserved"); // Check that we have the more restrictive column predicate (a < 5) let has_column_predicate = result.iter().any(|p| { - matches!(p, Expr::BinaryExpr(BinaryExpr { - left, - op: Operator::Lt, - right + matches!(p, Expr::BinaryExpr(BinaryExpr { + left, + op: Operator::Lt, + right }) if left == &Box::new(col("a")) && right == &Box::new(lit(5i32))) }); assert!(has_column_predicate, "Should have a < 5 predicate"); @@ -352,20 +352,20 @@ mod tests { // Check for a < 3 let has_a_predicate = result.iter().any(|p| { - matches!(p, Expr::BinaryExpr(BinaryExpr { - left, - op: Operator::Lt, - right + matches!(p, Expr::BinaryExpr(BinaryExpr { + left, + op: Operator::Lt, + right }) if left == &Box::new(col("a")) && right == &Box::new(lit(3i32))) }); assert!(has_a_predicate, "Should have a < 3 predicate"); // Check for b > 20 let has_b_predicate = result.iter().any(|p| { - matches!(p, Expr::BinaryExpr(BinaryExpr { - left, - op: Operator::Gt, - right + matches!(p, Expr::BinaryExpr(BinaryExpr { + left, + op: Operator::Gt, + right }) if left == &Box::new(col("b")) && right == &Box::new(lit(20i32))) }); assert!(has_b_predicate, "Should have b > 20 predicate"); diff --git a/datafusion/optimizer/src/simplify_expressions/utils.rs b/datafusion/optimizer/src/simplify_expressions/utils.rs index 7ed15be9fa3e3..72eaa8f02fab2 100644 --- a/datafusion/optimizer/src/simplify_expressions/utils.rs +++ b/datafusion/optimizer/src/simplify_expressions/utils.rs @@ -240,12 +240,7 @@ pub fn is_eq_and_ne_with_different_literal(eq_expr: &Expr, ne_expr: &Expr) -> bo match expr { Expr::BinaryExpr(BinaryExpr { left, - op: Operator::Eq, - right, - }) - | Expr::BinaryExpr(BinaryExpr { - left, - op: Operator::NotEq, + op: Operator::Eq | Operator::NotEq, right, }) => match (left.as_ref(), right.as_ref()) { (Expr::Literal(_, _), var) => Some((var, left)), diff --git a/datafusion/optimizer/src/utils.rs b/datafusion/optimizer/src/utils.rs index d4ac31e8a517c..8c5394df5cebc 100644 --- a/datafusion/optimizer/src/utils.rs +++ b/datafusion/optimizer/src/utils.rs @@ -183,10 +183,9 @@ pub fn is_restrict_null_predicate<'a>( false } } - ColumnarValue::Scalar(scalar) => matches!( - scalar, - ScalarValue::Boolean(None) | ScalarValue::Boolean(Some(false)) - ), + ColumnarValue::Scalar(scalar) => { + matches!(scalar, ScalarValue::Boolean(None | Some(false))) + } }, ) } diff --git a/datafusion/physical-optimizer/src/ensure_requirements/enforce_sorting/sort_pushdown.rs b/datafusion/physical-optimizer/src/ensure_requirements/enforce_sorting/sort_pushdown.rs index 9450c98ee9603..b51ffe0444531 100644 --- a/datafusion/physical-optimizer/src/ensure_requirements/enforce_sorting/sort_pushdown.rs +++ b/datafusion/physical-optimizer/src/ensure_requirements/enforce_sorting/sort_pushdown.rs @@ -225,14 +225,14 @@ fn stronger_distribution(a: &Distribution, b: &Distribution) -> Distribution { (Distribution::SinglePartition, _) | (_, Distribution::SinglePartition) => { Distribution::SinglePartition } - (Distribution::HashPartitioned(exprs), _) - | (Distribution::KeyPartitioned(exprs), _) => { - Distribution::KeyPartitioned(exprs.clone()) - } - (_, Distribution::HashPartitioned(exprs)) - | (_, Distribution::KeyPartitioned(exprs)) => { - Distribution::KeyPartitioned(exprs.clone()) - } + ( + Distribution::HashPartitioned(exprs) | Distribution::KeyPartitioned(exprs), + _, + ) => Distribution::KeyPartitioned(exprs.clone()), + ( + _, + Distribution::HashPartitioned(exprs) | Distribution::KeyPartitioned(exprs), + ) => Distribution::KeyPartitioned(exprs.clone()), _ => Distribution::UnspecifiedDistribution, } } diff --git a/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/mod.rs b/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/mod.rs index 6c1926b402cee..c8ac3625088f7 100644 --- a/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/mod.rs +++ b/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/mod.rs @@ -969,10 +969,8 @@ fn group_column_supported_type(data_type: &DataType) -> bool { // other unit combinations, so accepting them here would cause a // schema to be routed into GroupValuesColumn and then fail at // intern. Keep these two arms in lockstep with the dispatcher. - | DataType::Time32(TimeUnit::Second) - | DataType::Time32(TimeUnit::Millisecond) - | DataType::Time64(TimeUnit::Microsecond) - | DataType::Time64(TimeUnit::Nanosecond) + | DataType::Time32(TimeUnit::Second | TimeUnit::Millisecond) + | DataType::Time64(TimeUnit::Microsecond | TimeUnit::Nanosecond) | DataType::Timestamp(_, _) | DataType::Duration(_) | DataType::Interval(_) diff --git a/datafusion/physical-plan/src/operator_statistics/mod.rs b/datafusion/physical-plan/src/operator_statistics/mod.rs index b19f4e5fd4693..a5d649ffe6f86 100644 --- a/datafusion/physical-plan/src/operator_statistics/mod.rs +++ b/datafusion/physical-plan/src/operator_statistics/mod.rs @@ -1031,9 +1031,11 @@ impl StatisticsProvider for UnionStatisticsProvider { (Precision::Exact(a), Precision::Exact(b)) => { Precision::Exact(a.saturating_add(b)) } - (Precision::Inexact(a), Precision::Exact(b)) - | (Precision::Exact(a), Precision::Inexact(b)) - | (Precision::Inexact(a), Precision::Inexact(b)) => { + ( + Precision::Inexact(a), + Precision::Exact(b) | Precision::Inexact(b), + ) + | (Precision::Exact(a), Precision::Inexact(b)) => { Precision::Inexact(a.saturating_add(b)) } }) diff --git a/datafusion/physical-plan/src/sorts/stream.rs b/datafusion/physical-plan/src/sorts/stream.rs index bb9c00949369e..4b66157b08540 100644 --- a/datafusion/physical-plan/src/sorts/stream.rs +++ b/datafusion/physical-plan/src/sorts/stream.rs @@ -79,7 +79,7 @@ impl FusedStreams { // Skip empty batches Poll::Ready(Some(Ok(b))) if b.num_rows() == 0 => {} Poll::Ready(Some(Ok(_))) => return poll_result, - Poll::Ready(None) | Poll::Ready(Some(Err(_))) => { + Poll::Ready(None | Some(Err(_))) => { let stream_schema = self.0[stream_idx].get_ref().schema(); // Replace the stream with an empty stream, so we can drop memory usage diff --git a/datafusion/spark/src/function/conversion/cast.rs b/datafusion/spark/src/function/conversion/cast.rs index 45d1b336261d7..6e7fc194e4a49 100644 --- a/datafusion/spark/src/function/conversion/cast.rs +++ b/datafusion/spark/src/function/conversion/cast.rs @@ -177,9 +177,11 @@ fn get_target_type_from_scalar_args( let type_arg = scalar_args.get(1).and_then(|opt| *opt); match type_arg { - Some(ScalarValue::Utf8(Some(s))) - | Some(ScalarValue::LargeUtf8(Some(s))) - | Some(ScalarValue::Utf8View(Some(s))) => parse_target_type(s, timezone), + Some( + ScalarValue::Utf8(Some(s)) + | ScalarValue::LargeUtf8(Some(s)) + | ScalarValue::Utf8View(Some(s)), + ) => parse_target_type(s, timezone), _ => exec_err!( "spark_cast requires second argument to be a string of target data type ex: timestamp" ), diff --git a/datafusion/spark/src/function/datetime/date_part.rs b/datafusion/spark/src/function/datetime/date_part.rs index 50ef1ece9d1a3..c1f4edaca059a 100644 --- a/datafusion/spark/src/function/datetime/date_part.rs +++ b/datafusion/spark/src/function/datetime/date_part.rs @@ -99,9 +99,11 @@ impl ScalarUDFImpl for SparkDatePart { let [part_expr, date_expr] = take_function_args(self.name(), args)?; let part = match part_expr.as_literal() { - Some(ScalarValue::Utf8(Some(v))) - | Some(ScalarValue::Utf8View(Some(v))) - | Some(ScalarValue::LargeUtf8(Some(v))) => v.to_lowercase(), + Some( + ScalarValue::Utf8(Some(v)) + | ScalarValue::Utf8View(Some(v)) + | ScalarValue::LargeUtf8(Some(v)), + ) => v.to_lowercase(), _ => { return internal_err!( "First argument of `DATE_PART` must be non-null scalar Utf8" diff --git a/datafusion/spark/src/function/datetime/date_trunc.rs b/datafusion/spark/src/function/datetime/date_trunc.rs index c8b0fbca36165..5c2c7a4e2e2f7 100644 --- a/datafusion/spark/src/function/datetime/date_trunc.rs +++ b/datafusion/spark/src/function/datetime/date_trunc.rs @@ -97,9 +97,11 @@ impl ScalarUDFImpl for SparkDateTrunc { let [fmt_expr, ts_expr] = take_function_args(self.name(), args)?; let fmt = match fmt_expr.as_literal() { - Some(ScalarValue::Utf8(Some(v))) - | Some(ScalarValue::Utf8View(Some(v))) - | Some(ScalarValue::LargeUtf8(Some(v))) => v.to_lowercase(), + Some( + ScalarValue::Utf8(Some(v)) + | ScalarValue::Utf8View(Some(v)) + | ScalarValue::LargeUtf8(Some(v)), + ) => v.to_lowercase(), _ => { return plan_err!( "First argument of `DATE_TRUNC` must be non-null scalar Utf8" diff --git a/datafusion/spark/src/function/datetime/time_trunc.rs b/datafusion/spark/src/function/datetime/time_trunc.rs index 577c19733a71e..6d5e71271e321 100644 --- a/datafusion/spark/src/function/datetime/time_trunc.rs +++ b/datafusion/spark/src/function/datetime/time_trunc.rs @@ -91,9 +91,11 @@ impl ScalarUDFImpl for SparkTimeTrunc { let fmt_expr = &args[0]; match fmt_expr.as_literal() { - Some(ScalarValue::Utf8(Some(_))) - | Some(ScalarValue::Utf8View(Some(_))) - | Some(ScalarValue::LargeUtf8(Some(_))) => {} + Some( + ScalarValue::Utf8(Some(_)) + | ScalarValue::Utf8View(Some(_)) + | ScalarValue::LargeUtf8(Some(_)), + ) => {} _ => { return plan_err!( "First argument of `TIME_TRUNC` must be non-null scalar Utf8" diff --git a/datafusion/spark/src/function/datetime/trunc.rs b/datafusion/spark/src/function/datetime/trunc.rs index 2c18e3909fffb..128cb62d3cbde 100644 --- a/datafusion/spark/src/function/datetime/trunc.rs +++ b/datafusion/spark/src/function/datetime/trunc.rs @@ -96,9 +96,11 @@ impl ScalarUDFImpl for SparkTrunc { let [dt_expr, fmt_expr] = take_function_args(self.name(), args)?; let fmt = match fmt_expr.as_literal() { - Some(ScalarValue::Utf8(Some(v))) - | Some(ScalarValue::Utf8View(Some(v))) - | Some(ScalarValue::LargeUtf8(Some(v))) => v.to_lowercase(), + Some( + ScalarValue::Utf8(Some(v)) + | ScalarValue::Utf8View(Some(v)) + | ScalarValue::LargeUtf8(Some(v)), + ) => v.to_lowercase(), _ => { return plan_err!( "Second argument of `TRUNC` must be non-null scalar Utf8" diff --git a/datafusion/spark/src/function/string/format_string.rs b/datafusion/spark/src/function/string/format_string.rs index d7c1039623542..5b6de1e2f86b1 100644 --- a/datafusion/spark/src/function/string/format_string.rs +++ b/datafusion/spark/src/function/string/format_string.rs @@ -119,9 +119,11 @@ impl ScalarUDFImpl for FormatStringFunc { ColumnarValue::Scalar(ScalarValue::Utf8View(None)) => { Ok(ColumnarValue::Scalar(ScalarValue::Utf8View(None))) } - ColumnarValue::Scalar(ScalarValue::Utf8(Some(fmt))) - | ColumnarValue::Scalar(ScalarValue::LargeUtf8(Some(fmt))) - | ColumnarValue::Scalar(ScalarValue::Utf8View(Some(fmt))) => { + ColumnarValue::Scalar( + ScalarValue::Utf8(Some(fmt)) + | ScalarValue::LargeUtf8(Some(fmt)) + | ScalarValue::Utf8View(Some(fmt)), + ) => { let formatter = Formatter::parse(fmt, &data_types)?; let mut result = Vec::with_capacity(len.unwrap_or(1)); for i in 0..len.unwrap_or(1) { diff --git a/datafusion/spark/src/function/string/luhn_check.rs b/datafusion/spark/src/function/string/luhn_check.rs index 9241f5e70d085..564c9c223cd0c 100644 --- a/datafusion/spark/src/function/string/luhn_check.rs +++ b/datafusion/spark/src/function/string/luhn_check.rs @@ -101,16 +101,18 @@ impl ScalarUDFImpl for SparkLuhnCheck { exec_err!("Unsupported data type {other:?} for function `luhn_check`") } }, - ColumnarValue::Scalar(ScalarValue::Utf8(Some(s))) - | ColumnarValue::Scalar(ScalarValue::LargeUtf8(Some(s))) - | ColumnarValue::Scalar(ScalarValue::Utf8View(Some(s))) => Ok( - ColumnarValue::Scalar(ScalarValue::Boolean(Some(luhn_check_impl(s)))), - ), - ColumnarValue::Scalar(ScalarValue::Utf8(None)) - | ColumnarValue::Scalar(ScalarValue::LargeUtf8(None)) - | ColumnarValue::Scalar(ScalarValue::Utf8View(None)) => { - Ok(ColumnarValue::Scalar(ScalarValue::Boolean(None))) - } + ColumnarValue::Scalar( + ScalarValue::Utf8(Some(s)) + | ScalarValue::LargeUtf8(Some(s)) + | ScalarValue::Utf8View(Some(s)), + ) => Ok(ColumnarValue::Scalar(ScalarValue::Boolean(Some( + luhn_check_impl(s), + )))), + ColumnarValue::Scalar( + ScalarValue::Utf8(None) + | ScalarValue::LargeUtf8(None) + | ScalarValue::Utf8View(None), + ) => Ok(ColumnarValue::Scalar(ScalarValue::Boolean(None))), other => { exec_err!("Unsupported data type {other:?} for function `luhn_check`") } diff --git a/datafusion/sql/src/expr/function.rs b/datafusion/sql/src/expr/function.rs index 4417799d3c3e8..89b0d6183c8a7 100644 --- a/datafusion/sql/src/expr/function.rs +++ b/datafusion/sql/src/expr/function.rs @@ -289,9 +289,9 @@ impl SqlToRel<'_, S> { && args.iter().all(|arg| { matches!( arg.get_type(schema), - Ok(DataType::List(_)) - | Ok(DataType::LargeList(_)) - | Ok(DataType::FixedSizeList(_, _)) + Ok(DataType::List(_) + | DataType::LargeList(_) + | DataType::FixedSizeList(_, _)) ) }); diff --git a/datafusion/sql/src/expr/mod.rs b/datafusion/sql/src/expr/mod.rs index f1661028ed051..7af76441644ad 100644 --- a/datafusion/sql/src/expr/mod.rs +++ b/datafusion/sql/src/expr/mod.rs @@ -802,11 +802,11 @@ impl SqlToRel<'_, S> { values: Vec, ) -> Result { match values.first() { - Some(SQLExpr::Identifier(_)) - | Some(SQLExpr::Value(_)) - | Some(SQLExpr::CompoundIdentifier(_)) => { - self.parse_struct(schema, planner_context, values, &[]) - } + Some( + SQLExpr::Identifier(_) + | SQLExpr::Value(_) + | SQLExpr::CompoundIdentifier(_), + ) => self.parse_struct(schema, planner_context, values, &[]), None => not_impl_err!("Empty tuple not supported yet"), _ => { not_impl_err!("Only identifiers and literals are supported in tuples") diff --git a/datafusion/sql/src/statement.rs b/datafusion/sql/src/statement.rs index 93d896201af47..150e634d0da63 100644 --- a/datafusion/sql/src/statement.rs +++ b/datafusion/sql/src/statement.rs @@ -3179,7 +3179,7 @@ FROM ( None => Ok(()), // BEGIN TRANSACTION Some(BeginTransactionKind::Transaction) => Ok(()), - Some(BeginTransactionKind::Work) | Some(BeginTransactionKind::Tran) => { + Some(BeginTransactionKind::Work | BeginTransactionKind::Tran) => { not_impl_err!("Transaction kind not supported: {kind:?}") } } diff --git a/datafusion/sql/tests/sql_integration.rs b/datafusion/sql/tests/sql_integration.rs index 9f57aaafb0686..1f025c1d54ab9 100644 --- a/datafusion/sql/tests/sql_integration.rs +++ b/datafusion/sql/tests/sql_integration.rs @@ -5569,7 +5569,7 @@ fn test_using_join_wildcard_schema_semi_anti() { let s_columns = &["s.x1", "s.x2", "s.x3"]; let t_columns = &["t.x1", "t.x2", "t.x3"]; - let sql = "WITH + let sql = "WITH s AS (SELECT 1 AS x1, 2 AS x2, 3 AS x3), t AS (SELECT 1 AS x1, 4 AS x2, 5 AS x3) SELECT * FROM s LEFT SEMI JOIN t USING (x1)"; @@ -5811,7 +5811,7 @@ impl HigherOrderUDFImpl for MockArrayReduce { None, ]) } - (1, Some(accumulator)) | (0, Some(accumulator)) => { + (0 | 1, Some(accumulator)) => { // now we can use the merge output as it's accumulator and // as the finish parameter LambdaParametersProgress::Complete(vec![ diff --git a/datafusion/substrait/tests/cases/roundtrip_logical_plan.rs b/datafusion/substrait/tests/cases/roundtrip_logical_plan.rs index 6b58c4a53af17..dfebf665481b2 100644 --- a/datafusion/substrait/tests/cases/roundtrip_logical_plan.rs +++ b/datafusion/substrait/tests/cases/roundtrip_logical_plan.rs @@ -2502,7 +2502,7 @@ fn check_post_join_filters(rel: &Rel) -> Result<()> { } Ok(()) } - Some(RelType::ExtensionLeaf(_)) | Some(RelType::Read(_)) => Ok(()), + Some(RelType::ExtensionLeaf(_) | RelType::Read(_)) => Ok(()), _ => not_impl_err!( "Unsupported Reltype: {:?} in post join filter check", rel.rel_type From d9e8beb66bac57a00a8de62b96280b3804b93d8f Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 1 Sep 2026 13:55:38 +0200 Subject: [PATCH 5/6] Enable clippy::match_bool `if`/`else` instead of `match` on a bool. Two sites cascaded into `redundant_else`, fixed in the same commit. --- Cargo.toml | 1 - benchmarks/src/sql_benchmark.rs | 75 +++++++++--------- datafusion/catalog-listing/src/helpers.rs | 14 ++-- datafusion/common/src/scalar/mod.rs | 7 +- .../src/datasource/listing_table_factory.rs | 7 +- datafusion/core/src/datasource/provider.rs | 7 +- .../provider_filter_pushdown.rs | 14 ++-- .../physical_optimizer/sanity_checker.rs | 17 ++-- .../user_defined/user_defined_aggregates.rs | 7 +- .../user_defined_scalar_functions.rs | 7 +- datafusion/datasource-parquet/src/source.rs | 7 +- .../datasource/src/file_scan_config/proto.rs | 7 +- datafusion/datasource/src/url.rs | 14 ++-- .../datasource/src/write/orchestration.rs | 20 +++-- datafusion/execution/src/memory_pool/pool.rs | 77 +++++++++---------- datafusion/expr/src/expr.rs | 26 +++---- datafusion/expr/src/udaf.rs | 7 +- datafusion/ffi/src/table_provider.rs | 7 +- datafusion/ffi/src/tests/mod.rs | 7 +- datafusion/ffi/src/udwf/mod.rs | 9 ++- .../functions-aggregate/src/variance.rs | 74 +++++++++--------- .../benches/datetime_expressions/to_char.rs | 7 +- datafusion/functions/src/core/nullif.rs | 7 +- .../src/decorrelate_predicate_subquery.rs | 29 ++++--- .../optimizer/src/optimize_projections/mod.rs | 7 +- .../simplify_expressions/expr_simplifier.rs | 16 ++-- datafusion/physical-expr-common/src/datum.rs | 5 +- datafusion/physical-plan/src/buffer.rs | 7 +- datafusion/physical-plan/src/sorts/cursor.rs | 47 ++++++----- datafusion/physical-plan/src/sorts/sort.rs | 11 +-- datafusion/physical-plan/src/unnest.rs | 7 +- 31 files changed, 290 insertions(+), 264 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index a943e58b9a583..919e4c0e3bf30 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -289,7 +289,6 @@ inline_always = "allow" # 45 hits items_after_statements = "allow" # 171 hits many_single_char_names = "allow" # 12 hits; short names are idiomatic in the numeric kernels map_unwrap_or = "allow" # 198 hits -match_bool = "allow" # 46 hits match_same_arms = "allow" # 261 hits match_wildcard_for_single_variants = "allow" # 132 hits missing_errors_doc = "allow" # 1807 hits diff --git a/benchmarks/src/sql_benchmark.rs b/benchmarks/src/sql_benchmark.rs index 3959b1211560a..fb6672b608ded 100644 --- a/benchmarks/src/sql_benchmark.rs +++ b/benchmarks/src/sql_benchmark.rs @@ -231,52 +231,49 @@ impl SqlBenchmark { let mut local_result = vec![]; for query in run_queries { - match save_results { - true => { - debug!( - "Running query (saving results) {}-{}: {query}", - self.group, self.subgroup - ); + if save_results { + debug!( + "Running query (saving results) {}-{}: {query}", + self.group, self.subgroup + ); - let df = ctx.sql(query).await?; - if !self.expect.is_empty() { - let physical_plan = df.create_physical_plan().await?; - self.validate_expected_plan(&physical_plan)?; - } + let df = ctx.sql(query).await?; + if !self.expect.is_empty() { + let physical_plan = df.create_physical_plan().await?; + self.validate_expected_plan(&physical_plan)?; + } - let result_schema = Arc::new(df.schema().as_arrow().clone()); - let mut batches = df.collect().await?; - let trimmed = query.trim_start(); - - // save the output for select/with queries - if starts_with_ignore_ascii_case(trimmed, "select") - || starts_with_ignore_ascii_case(trimmed, "with") - { - if batches.is_empty() { - batches.push(RecordBatch::new_empty(result_schema)); - } - let row_count_for_query = - batches.iter().map(RecordBatch::num_rows).sum::(); - debug!( - "Persisting {} batches ({} rows)...", - batches.len(), - row_count_for_query - ); - - result_count = row_count_for_query; - local_result = batches; + let result_schema = Arc::new(df.schema().as_arrow().clone()); + let mut batches = df.collect().await?; + let trimmed = query.trim_start(); + + // save the output for select/with queries + if starts_with_ignore_ascii_case(trimmed, "select") + || starts_with_ignore_ascii_case(trimmed, "with") + { + if batches.is_empty() { + batches.push(RecordBatch::new_empty(result_schema)); } - } - false => { + let row_count_for_query = + batches.iter().map(RecordBatch::num_rows).sum::(); debug!( - "Running query (ignoring results) {}-{}: {query}", - self.group, self.subgroup + "Persisting {} batches ({} rows)...", + batches.len(), + row_count_for_query ); - result_count = self - .execute_sql_without_result_buffering(query, ctx) - .await?; + result_count = row_count_for_query; + local_result = batches; } + } else { + debug!( + "Running query (ignoring results) {}-{}: {query}", + self.group, self.subgroup + ); + + result_count = self + .execute_sql_without_result_buffering(query, ctx) + .await?; } } diff --git a/datafusion/catalog-listing/src/helpers.rs b/datafusion/catalog-listing/src/helpers.rs index dc090378a8513..def9f7225ab35 100644 --- a/datafusion/catalog-listing/src/helpers.rs +++ b/datafusion/catalog-listing/src/helpers.rs @@ -213,12 +213,14 @@ pub async fn list_partitions( depth: depth + 1, files: None, }; - match depth < max_depth { - true => match futures.len() < CONCURRENCY_LIMIT { - true => futures.push(child.list(store)), - false => pending.push(child.list(store)), - }, - false => out.push(child), + if depth < max_depth { + if futures.len() < CONCURRENCY_LIMIT { + futures.push(child.list(store)) + } else { + pending.push(child.list(store)) + } + } else { + out.push(child) } } } diff --git a/datafusion/common/src/scalar/mod.rs b/datafusion/common/src/scalar/mod.rs index efb1aa99cbdd8..af1339b88bac0 100644 --- a/datafusion/common/src/scalar/mod.rs +++ b/datafusion/common/src/scalar/mod.rs @@ -4256,9 +4256,10 @@ impl ScalarValue { }; ScalarValue::FixedSizeBinary( size, - match array.is_null(index) { - true => None, - false => Some(array.value(index).into()), + if array.is_null(index) { + None + } else { + Some(array.value(index).into()) }, ) } diff --git a/datafusion/core/src/datasource/listing_table_factory.rs b/datafusion/core/src/datasource/listing_table_factory.rs index 5ebe0882befa4..d3d0c6bdd6a40 100644 --- a/datafusion/core/src/datasource/listing_table_factory.rs +++ b/datafusion/core/src/datasource/listing_table_factory.rs @@ -144,13 +144,14 @@ impl ListingTableFactory { // extension filter is left empty and the explicit paths/globs are used // as provided. let file_extension = if table_paths.len() == 1 { - match first_path.is_collection() { + if first_path.is_collection() { // Setting the extension to be empty instead of allowing the default extension seems // odd, but was done to ensure existing behavior isn't modified. It seems like this // could be refactored to either use the default extension or set the fully expected // extension when compression is included (e.g. ".csv.gz") - true => String::new(), - false => get_extension(&cmd.locations[0]), + String::new() + } else { + get_extension(&cmd.locations[0]) } } else { String::new() diff --git a/datafusion/core/src/datasource/provider.rs b/datafusion/core/src/datasource/provider.rs index e574042813a7b..6ed7b4cc8c06a 100644 --- a/datafusion/core/src/datasource/provider.rs +++ b/datafusion/core/src/datasource/provider.rs @@ -87,9 +87,10 @@ impl DefaultTableFactory { } } - match unbounded { - true => self.stream.create(state, cmd).await, - false => self.listing.create(state, cmd).await, + if unbounded { + self.stream.create(state, cmd).await + } else { + self.listing.create(state, cmd).await } } } diff --git a/datafusion/core/tests/custom_sources_cases/provider_filter_pushdown.rs b/datafusion/core/tests/custom_sources_cases/provider_filter_pushdown.rs index 4ec9747058140..3a5e791bdadfe 100644 --- a/datafusion/core/tests/custom_sources_cases/provider_filter_pushdown.rs +++ b/datafusion/core/tests/custom_sources_cases/provider_filter_pushdown.rs @@ -225,9 +225,10 @@ impl TableProvider for CustomProvider { }; Ok(Arc::new(CustomPlan::new( - match projection.is_empty() { - true => Arc::new(Schema::empty()), - false => self.zero_batch.schema(), + if projection.is_empty() { + Arc::new(Schema::empty()) + } else { + self.zero_batch.schema() }, match int_value { 0 => vec![self.zero_batch.clone()], @@ -237,9 +238,10 @@ impl TableProvider for CustomProvider { ))) } _ => Ok(Arc::new(CustomPlan::new( - match projection.is_empty() { - true => Arc::new(Schema::empty()), - false => self.zero_batch.schema(), + if projection.is_empty() { + Arc::new(Schema::empty()) + } else { + self.zero_batch.schema() }, vec![], ))), diff --git a/datafusion/core/tests/physical_optimizer/sanity_checker.rs b/datafusion/core/tests/physical_optimizer/sanity_checker.rs index 184125dcbe180..ab985c3fdd32b 100644 --- a/datafusion/core/tests/physical_optimizer/sanity_checker.rs +++ b/datafusion/core/tests/physical_optimizer/sanity_checker.rs @@ -51,16 +51,13 @@ async fn register_current_csv( let schema = datafusion::test_util::aggr_test_schema(); let path = format!("{testdata}/csv/aggregate_test_100.csv"); - match infinite { - true => { - let source = FileStreamProvider::new_file(schema, path.into()); - let config = StreamConfig::new(Arc::new(source)); - ctx.register_table(table_name, Arc::new(StreamTable::new(Arc::new(config))))?; - } - false => { - ctx.register_csv(table_name, &path, CsvReadOptions::new().schema(&schema)) - .await?; - } + if infinite { + let source = FileStreamProvider::new_file(schema, path.into()); + let config = StreamConfig::new(Arc::new(source)); + ctx.register_table(table_name, Arc::new(StreamTable::new(Arc::new(config))))?; + } else { + ctx.register_csv(table_name, &path, CsvReadOptions::new().schema(&schema)) + .await?; } Ok(()) diff --git a/datafusion/core/tests/user_defined/user_defined_aggregates.rs b/datafusion/core/tests/user_defined/user_defined_aggregates.rs index d035fa25e1d41..e77d4c183c1df 100644 --- a/datafusion/core/tests/user_defined/user_defined_aggregates.rs +++ b/datafusion/core/tests/user_defined/user_defined_aggregates.rs @@ -992,9 +992,10 @@ impl Accumulator for MetadataBasedAccumulator { } fn evaluate(&mut self) -> Result { - let v = match self.double_output { - true => self.curr_sum * 2, - false => self.curr_sum, + let v = if self.double_output { + self.curr_sum * 2 + } else { + self.curr_sum }; Ok(ScalarValue::from(v)) diff --git a/datafusion/core/tests/user_defined/user_defined_scalar_functions.rs b/datafusion/core/tests/user_defined/user_defined_scalar_functions.rs index 36d094cd0360b..dae491a1fb695 100644 --- a/datafusion/core/tests/user_defined/user_defined_scalar_functions.rs +++ b/datafusion/core/tests/user_defined/user_defined_scalar_functions.rs @@ -1819,9 +1819,10 @@ impl ScalarUDFImpl for ExtensionBasedUdf { // If we have the extension type set, we are outputting a boolean value. // Otherwise we output a string representation of the numeric value. fn print_value(x: i8, as_bool: bool) -> String { - match as_bool { - true => format!("{}", x != 0), - false => format!("{x}"), + if as_bool { + format!("{}", x != 0) + } else { + format!("{x}") } } diff --git a/datafusion/datasource-parquet/src/source.rs b/datafusion/datasource-parquet/src/source.rs index 4872db9fd3329..73e825a9c5566 100644 --- a/datafusion/datasource-parquet/src/source.rs +++ b/datafusion/datasource-parquet/src/source.rs @@ -1199,9 +1199,10 @@ impl ParquetSource { } let table_schema = FileScanConfig::parse_table_schema_from_proto(base_conf)?; - let object_store_url = match base_conf.object_store_url.is_empty() { - false => ObjectStoreUrl::parse(&base_conf.object_store_url)?, - true => ObjectStoreUrl::local_filesystem(), + let object_store_url = if !base_conf.object_store_url.is_empty() { + ObjectStoreUrl::parse(&base_conf.object_store_url)? + } else { + ObjectStoreUrl::local_filesystem() }; let store = ctx .task_ctx() diff --git a/datafusion/datasource/src/file_scan_config/proto.rs b/datafusion/datasource/src/file_scan_config/proto.rs index d2dbb5f475479..863a6729c88d3 100644 --- a/datafusion/datasource/src/file_scan_config/proto.rs +++ b/datafusion/datasource/src/file_scan_config/proto.rs @@ -237,9 +237,10 @@ impl FileScanConfig { .map(TryInto::try_into) .collect::>>()?; - let decoded_object_store_url = match object_store_url.is_empty() { - false => ObjectStoreUrl::parse(object_store_url)?, - true => ObjectStoreUrl::local_filesystem(), + let decoded_object_store_url = if !object_store_url.is_empty() { + ObjectStoreUrl::parse(object_store_url)? + } else { + ObjectStoreUrl::local_filesystem() }; let mut decoded_output_ordering = vec![]; diff --git a/datafusion/datasource/src/url.rs b/datafusion/datasource/src/url.rs index cfb6608ca0a78..14a46c3e61b6b 100644 --- a/datafusion/datasource/src/url.rs +++ b/datafusion/datasource/src/url.rs @@ -436,16 +436,18 @@ async fn list_with_cache<'b>( #[cfg(not(target_arch = "wasm32"))] fn url_from_filesystem_path(s: &str) -> Option { let path = std::path::Path::new(s); - let is_dir = match path.exists() { - true => path.is_dir(), + let is_dir = if path.exists() { + path.is_dir() + } else { // Fallback to inferring from trailing separator - false => std::path::is_separator(s.chars().last()?), + std::path::is_separator(s.chars().last()?) }; let from_absolute_path = |p| { - let first = match is_dir { - true => Url::from_directory_path(p).ok(), - false => Url::from_file_path(p).ok(), + let first = if is_dir { + Url::from_directory_path(p).ok() + } else { + Url::from_file_path(p).ok() }?; // By default from_*_path preserve relative path segments diff --git a/datafusion/datasource/src/write/orchestration.rs b/datafusion/datasource/src/write/orchestration.rs index 387e929b9f30d..12cc033511672 100644 --- a/datafusion/datasource/src/write/orchestration.rs +++ b/datafusion/datasource/src/write/orchestration.rs @@ -216,20 +216,18 @@ pub(crate) async fn stateless_serialize_and_write_files( } if any_errors { - match any_abort_errors { - true => { + if any_abort_errors { + return internal_err!( + "Error encountered during writing to ObjectStore and failed to abort all writers. Partial result may have been written." + ); + } + match triggering_error { + Some(e) => return Err(e), + None => { return internal_err!( - "Error encountered during writing to ObjectStore and failed to abort all writers. Partial result may have been written." + "Unknown Error encountered during writing to ObjectStore. All writers successfully aborted." ); } - false => match triggering_error { - Some(e) => return Err(e), - None => { - return internal_err!( - "Unknown Error encountered during writing to ObjectStore. All writers successfully aborted." - ); - } - }, } } diff --git a/datafusion/execution/src/memory_pool/pool.rs b/datafusion/execution/src/memory_pool/pool.rs index d854cbd627cec..2d57c1576dba0 100644 --- a/datafusion/execution/src/memory_pool/pool.rs +++ b/datafusion/execution/src/memory_pool/pool.rs @@ -219,58 +219,57 @@ impl MemoryPool for FairSpillPool { fn grow(&self, reservation: &MemoryReservation, additional: usize) { let mut state = self.state.lock(); - match reservation.registration.consumer.can_spill { - true => state.spillable += additional, - false => state.unspillable += additional, + if reservation.registration.consumer.can_spill { + state.spillable += additional + } else { + state.unspillable += additional } } fn shrink(&self, reservation: &MemoryReservation, shrink: usize) { let mut state = self.state.lock(); - match reservation.registration.consumer.can_spill { - true => state.spillable -= shrink, - false => state.unspillable -= shrink, + if reservation.registration.consumer.can_spill { + state.spillable -= shrink + } else { + state.unspillable -= shrink } } fn try_grow(&self, reservation: &MemoryReservation, additional: usize) -> Result<()> { let mut state = self.state.lock(); - match reservation.registration.consumer.can_spill { - true => { - // The total amount of memory available to spilling consumers - let spill_available = self.pool_size.saturating_sub(state.unspillable); - - // No spiller may use more than their fraction of the memory available - let available = spill_available - .checked_div(state.num_spill) - .unwrap_or(spill_available); - - if reservation.size() + additional > available { - return Err(insufficient_capacity_err( - reservation, - additional, - available, - self, - )); - } - state.spillable += additional; + if reservation.registration.consumer.can_spill { + // The total amount of memory available to spilling consumers + let spill_available = self.pool_size.saturating_sub(state.unspillable); + + // No spiller may use more than their fraction of the memory available + let available = spill_available + .checked_div(state.num_spill) + .unwrap_or(spill_available); + + if reservation.size() + additional > available { + return Err(insufficient_capacity_err( + reservation, + additional, + available, + self, + )); } - false => { - let available = self - .pool_size - .saturating_sub(state.unspillable + state.spillable); - - if available < additional { - return Err(insufficient_capacity_err( - reservation, - additional, - available, - self, - )); - } - state.unspillable += additional; + state.spillable += additional; + } else { + let available = self + .pool_size + .saturating_sub(state.unspillable + state.spillable); + + if available < additional { + return Err(insufficient_capacity_err( + reservation, + additional, + available, + self, + )); } + state.unspillable += additional; } Ok(()) } diff --git a/datafusion/expr/src/expr.rs b/datafusion/expr/src/expr.rs index ad491c9782637..a2566ded61450 100644 --- a/datafusion/expr/src/expr.rs +++ b/datafusion/expr/src/expr.rs @@ -1929,14 +1929,15 @@ impl Expr { // f_up: unalias on up so we can remove nested aliases like // `(x as foo) as bar` if let Expr::Alias(alias) = expr { - match alias + if alias .metadata .as_ref() .map(|h| h.is_empty()) .unwrap_or(true) { - true => Ok(Transformed::yes(*alias.expr)), - false => Ok(Transformed::no(Expr::Alias(alias))), + Ok(Transformed::yes(*alias.expr)) + } else { + Ok(Transformed::no(Expr::Alias(alias))) } } else { Ok(Transformed::no(expr)) @@ -3571,9 +3572,10 @@ impl Display for Expr { } Expr::ScalarVariable(_, var_names) => write!(f, "{}", var_names.join(".")), Expr::Literal(v, metadata) => { - match metadata.as_ref().map(|m| m.is_empty()).unwrap_or(true) { - false => write!(f, "{v:?} {:?}", metadata.as_ref().unwrap()), - true => write!(f, "{v:?}"), + if !metadata.as_ref().map(|m| m.is_empty()).unwrap_or(true) { + write!(f, "{v:?} {:?}", metadata.as_ref().unwrap()) + } else { + write!(f, "{v:?}") } } Expr::Case(case) => { @@ -3808,15 +3810,13 @@ fn fmt_function( args: &[Expr], display: bool, ) -> fmt::Result { - let args: Vec = match display { - true => args.iter().map(|arg| format!("{arg}")).collect(), - false => args.iter().map(|arg| format!("{arg:?}")).collect(), + let args: Vec = if display { + args.iter().map(|arg| format!("{arg}")).collect() + } else { + args.iter().map(|arg| format!("{arg:?}")).collect() }; - let distinct_str = match distinct { - true => "DISTINCT ", - false => "", - }; + let distinct_str = if distinct { "DISTINCT " } else { "" }; write!(f, "{}({}{})", fun, distinct_str, args.join(", ")) } diff --git a/datafusion/expr/src/udaf.rs b/datafusion/expr/src/udaf.rs index 8f7e9cc6cfc2b..6f286da334e49 100644 --- a/datafusion/expr/src/udaf.rs +++ b/datafusion/expr/src/udaf.rs @@ -1046,9 +1046,10 @@ impl<'a> UdafSchemaNameBuilder<'a> { } if !order_by.is_empty() { - let clause = match supports_within_group_clause { - true => "WITHIN GROUP", - false => "ORDER BY", + let clause = if supports_within_group_clause { + "WITHIN GROUP" + } else { + "ORDER BY" }; schema_name.write_fmt(format_args!( diff --git a/datafusion/ffi/src/table_provider.rs b/datafusion/ffi/src/table_provider.rs index 5103297fcba57..463bfebb0d70e 100644 --- a/datafusion/ffi/src/table_provider.rs +++ b/datafusion/ffi/src/table_provider.rs @@ -592,9 +592,10 @@ impl FFI_TableProvider { schema: schema_fn_wrapper, scan: scan_fn_wrapper, table_type: table_type_fn_wrapper, - supports_filters_pushdown: match can_support_pushdown_filters { - true => Some(supports_filters_pushdown_fn_wrapper), - false => None, + supports_filters_pushdown: if can_support_pushdown_filters { + Some(supports_filters_pushdown_fn_wrapper) + } else { + None }, insert_into: insert_into_fn_wrapper, statistics: statistics_fn_wrapper, diff --git a/datafusion/ffi/src/tests/mod.rs b/datafusion/ffi/src/tests/mod.rs index 7e583b6c5d5bf..b721b2f5835b3 100644 --- a/datafusion/ffi/src/tests/mod.rs +++ b/datafusion/ffi/src/tests/mod.rs @@ -162,9 +162,10 @@ extern "C" fn construct_table_provider( synchronous: bool, codec: FFI_LogicalExtensionCodec, ) -> FFI_TableProvider { - match synchronous { - true => create_sync_table_provider(codec), - false => create_async_table_provider(codec), + if synchronous { + create_sync_table_provider(codec) + } else { + create_async_table_provider(codec) } } diff --git a/datafusion/ffi/src/udwf/mod.rs b/datafusion/ffi/src/udwf/mod.rs index 9ba874dc6b686..b233b517b1154 100644 --- a/datafusion/ffi/src/udwf/mod.rs +++ b/datafusion/ffi/src/udwf/mod.rs @@ -350,11 +350,12 @@ impl WindowUDFImpl for ForeignWindowUDF { ))?; let schema: SchemaRef = schema.into(); - match schema.fields().is_empty() { - true => ffi_err!( + if schema.fields().is_empty() { + ffi_err!( "Unable to retrieve field in WindowUDF via FFI - schema has no fields" - ), - false => Ok(schema.field(0).to_owned().into()), + ) + } else { + Ok(schema.field(0).to_owned().into()) } } } diff --git a/datafusion/functions-aggregate/src/variance.rs b/datafusion/functions-aggregate/src/variance.rs index 072d064f76bd4..90c57a9679698 100644 --- a/datafusion/functions-aggregate/src/variance.rs +++ b/datafusion/functions-aggregate/src/variance.rs @@ -98,27 +98,26 @@ impl AggregateUDFImpl for VarianceSample { fn state_fields(&self, args: StateFieldsArgs) -> Result> { let name = args.name; - match args.is_distinct { - false => Ok(vec![ + if !args.is_distinct { + Ok(vec![ Field::new(format_state_name(name, "count"), DataType::UInt64, true), Field::new(format_state_name(name, "mean"), DataType::Float64, true), Field::new(format_state_name(name, "m2"), DataType::Float64, true), ] .into_iter() .map(Arc::new) - .collect()), - true => { - let field = Field::new_list_field(DataType::Float64, true); - let state_name = "distinct_var"; - Ok(vec![ - Field::new( - format_state_name(name, state_name), - DataType::List(Arc::new(field)), - true, - ) - .into(), - ]) - } + .collect()) + } else { + let field = Field::new_list_field(DataType::Float64, true); + let state_name = "distinct_var"; + Ok(vec![ + Field::new( + format_state_name(name, state_name), + DataType::List(Arc::new(field)), + true, + ) + .into(), + ]) } } @@ -193,30 +192,27 @@ impl AggregateUDFImpl for VariancePopulation { } fn state_fields(&self, args: StateFieldsArgs) -> Result> { - match args.is_distinct { - false => { - let name = args.name; - Ok(vec![ - Field::new(format_state_name(name, "count"), DataType::UInt64, true), - Field::new(format_state_name(name, "mean"), DataType::Float64, true), - Field::new(format_state_name(name, "m2"), DataType::Float64, true), - ] - .into_iter() - .map(Arc::new) - .collect()) - } - true => { - let field = Field::new_list_field(DataType::Float64, true); - let state_name = "distinct_var"; - Ok(vec![ - Field::new( - format_state_name(args.name, state_name), - DataType::List(Arc::new(field)), - true, - ) - .into(), - ]) - } + if !args.is_distinct { + let name = args.name; + Ok(vec![ + Field::new(format_state_name(name, "count"), DataType::UInt64, true), + Field::new(format_state_name(name, "mean"), DataType::Float64, true), + Field::new(format_state_name(name, "m2"), DataType::Float64, true), + ] + .into_iter() + .map(Arc::new) + .collect()) + } else { + let field = Field::new_list_field(DataType::Float64, true); + let state_name = "distinct_var"; + Ok(vec![ + Field::new( + format_state_name(args.name, state_name), + DataType::List(Arc::new(field)), + true, + ) + .into(), + ]) } } diff --git a/datafusion/functions/benches/datetime_expressions/to_char.rs b/datafusion/functions/benches/datetime_expressions/to_char.rs index 2be28806c9e8c..a48eef570dca7 100644 --- a/datafusion/functions/benches/datetime_expressions/to_char.rs +++ b/datafusion/functions/benches/datetime_expressions/to_char.rs @@ -109,9 +109,10 @@ fn pick_date_time_pattern(rng: &mut StdRng) -> String { } fn pick_date_and_date_time_mixed_pattern(rng: &mut StdRng) -> String { - match rng.random_bool(0.5) { - true => pick_date_pattern(rng), - false => pick_date_time_pattern(rng), + if rng.random_bool(0.5) { + pick_date_pattern(rng) + } else { + pick_date_time_pattern(rng) } } diff --git a/datafusion/functions/src/core/nullif.rs b/datafusion/functions/src/core/nullif.rs index f58ae857d4791..309d14e59d193 100644 --- a/datafusion/functions/src/core/nullif.rs +++ b/datafusion/functions/src/core/nullif.rs @@ -138,9 +138,10 @@ fn nullif_func(args: &[ColumnarValue]) -> Result { Ok(ColumnarValue::Array(array)) } (ColumnarValue::Scalar(lhs), ColumnarValue::Scalar(rhs)) => { - let val: ScalarValue = match lhs.eq(rhs) { - true => lhs.data_type().try_into()?, - false => lhs.clone(), + let val: ScalarValue = if lhs.eq(rhs) { + lhs.data_type().try_into()? + } else { + lhs.clone() }; Ok(ColumnarValue::Scalar(val)) diff --git a/datafusion/optimizer/src/decorrelate_predicate_subquery.rs b/datafusion/optimizer/src/decorrelate_predicate_subquery.rs index 5f623f1bef6f6..c0594a2eff86e 100644 --- a/datafusion/optimizer/src/decorrelate_predicate_subquery.rs +++ b/datafusion/optimizer/src/decorrelate_predicate_subquery.rs @@ -274,9 +274,10 @@ fn build_join_top( }) .map_or(Ok(None), |v| v.map(Some))?; - let join_type = match query_info.negated { - true => JoinType::LeftAnti, - false => JoinType::LeftSemi, + let join_type = if query_info.negated { + JoinType::LeftAnti + } else { + JoinType::LeftSemi }; let subquery = query_info.query.subquery.as_ref(); let subquery_alias = alias.next("__correlated_sq"); @@ -556,14 +557,20 @@ impl SubqueryInfo { pub fn expr(self) -> Expr { match self.where_in_expr { - Some(expr) => match self.negated { - true => not_in_subquery(expr, self.query.subquery), - false => in_subquery(expr, self.query.subquery), - }, - None => match self.negated { - true => not_exists(self.query.subquery), - false => exists(self.query.subquery), - }, + Some(expr) => { + if self.negated { + not_in_subquery(expr, self.query.subquery) + } else { + in_subquery(expr, self.query.subquery) + } + } + None => { + if self.negated { + not_exists(self.query.subquery) + } else { + exists(self.query.subquery) + } + } } } } diff --git a/datafusion/optimizer/src/optimize_projections/mod.rs b/datafusion/optimizer/src/optimize_projections/mod.rs index 3bbaf887ca594..bbe79ca4bdf1c 100644 --- a/datafusion/optimizer/src/optimize_projections/mod.rs +++ b/datafusion/optimizer/src/optimize_projections/mod.rs @@ -707,14 +707,15 @@ fn rewrite_expr(expr: Expr, input: &Projection) -> Result> { match expr { // remove any intermediate aliases if they do not carry metadata Expr::Alias(alias) => { - match alias + if alias .metadata .as_ref() .map(|h| h.is_empty()) .unwrap_or(true) { - true => Ok(Transformed::yes(*alias.expr)), - false => Ok(Transformed::no(Expr::Alias(alias))), + Ok(Transformed::yes(*alias.expr)) + } else { + Ok(Transformed::no(Expr::Alias(alias))) } } Expr::Column(col) => { diff --git a/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs b/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs index e83def89395c2..6b58aab34ce9f 100644 --- a/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs +++ b/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs @@ -722,9 +722,10 @@ impl ConstEvaluator { .ok() .and_then(|f| { let m = f.metadata(); - match m.is_empty() { - true => None, - false => Some(FieldMetadata::from(m)), + if m.is_empty() { + None + } else { + Some(FieldMetadata::from(m)) } }); let col_val = match phys_expr.evaluate(&DUMMY_BATCH) { @@ -905,13 +906,14 @@ impl TreeNodeRewriter for Simplifier<'_> { op: Eq, right, }) if (left == right) & !left.is_volatile() => { - Transformed::yes(match !info.nullable(&left)? { - true => lit(true), - false => Expr::BinaryExpr(BinaryExpr { + Transformed::yes(if !info.nullable(&left)? { + lit(true) + } else { + Expr::BinaryExpr(BinaryExpr { left: Box::new(Expr::IsNotNull(left)), op: Or, right: Box::new(lit_bool_null()), - }), + }) }) } diff --git a/datafusion/physical-expr-common/src/datum.rs b/datafusion/physical-expr-common/src/datum.rs index d23fb30db6c4a..a286637dd99de 100644 --- a/datafusion/physical-expr-common/src/datum.rs +++ b/datafusion/physical-expr-common/src/datum.rs @@ -160,10 +160,7 @@ pub fn compare_op_for_nested( assert_or_internal_err!(l_len == r_len || is_l_scalar || is_r_scalar, "len mismatch"); - let len = match is_l_scalar { - true => r_len, - false => l_len, - }; + let len = if is_l_scalar { r_len } else { l_len }; // fast path, if compare with one null and operator is not 'distinct', then we can return null array directly if !matches!(op, Operator::IsDistinctFrom | Operator::IsNotDistinctFrom) diff --git a/datafusion/physical-plan/src/buffer.rs b/datafusion/physical-plan/src/buffer.rs index 00656c6e642c0..7f81b234997b8 100644 --- a/datafusion/physical-plan/src/buffer.rs +++ b/datafusion/physical-plan/src/buffer.rs @@ -749,12 +749,13 @@ mod tests { async fn finished( buffered: &mut MemoryBufferedStream, ) -> Result<(), Box> { - match timeout(Duration::from_millis(1), buffered.next()) + if timeout(Duration::from_millis(1), buffered.next()) .await? .is_none() { - true => Ok(()), - false => internal_err!("Stream should have finished")?, + Ok(()) + } else { + internal_err!("Stream should have finished")? } } diff --git a/datafusion/physical-plan/src/sorts/cursor.rs b/datafusion/physical-plan/src/sorts/cursor.rs index 003de2375ad3f..4357dec657384 100644 --- a/datafusion/physical-plan/src/sorts/cursor.rs +++ b/datafusion/physical-plan/src/sorts/cursor.rs @@ -509,9 +509,10 @@ impl ArrayValues { reservation: MemoryReservation, ) -> Self { assert!(array.len() > 0, "Empty array passed to FieldCursor"); - let null_threshold = match options.nulls_first { - true => array.null_count(), - false => array.len() - array.null_count(), + let null_threshold = if options.nulls_first { + array.null_count() + } else { + array.len() - array.null_count() }; Self { @@ -562,18 +563,27 @@ impl CursorValues for ArrayValues { fn compare(l: &Self, l_idx: usize, r: &Self, r_idx: usize) -> Ordering { match (l.is_null(l_idx), r.is_null(r_idx)) { (true, true) => Ordering::Equal, - (true, false) => match l.options.nulls_first { - true => Ordering::Less, - false => Ordering::Greater, - }, - (false, true) => match l.options.nulls_first { - true => Ordering::Greater, - false => Ordering::Less, - }, - (false, false) => match l.options.descending { - true => T::compare(&r.values, r_idx, &l.values, l_idx), - false => T::compare(&l.values, l_idx, &r.values, r_idx), - }, + (true, false) => { + if l.options.nulls_first { + Ordering::Less + } else { + Ordering::Greater + } + } + (false, true) => { + if l.options.nulls_first { + Ordering::Greater + } else { + Ordering::Less + } + } + (false, false) => { + if l.options.descending { + T::compare(&r.values, r_idx, &l.values, l_idx) + } else { + T::compare(&l.values, l_idx, &r.values, r_idx) + } + } } } @@ -619,9 +629,10 @@ mod tests { values: ScalarBuffer, null_count: usize, ) -> Cursor>> { - let null_threshold = match options.nulls_first { - true => null_count, - false => values.len() - null_count, + let null_threshold = if options.nulls_first { + null_count + } else { + values.len() - null_count }; let memory_pool: Arc = Arc::new(GreedyMemoryPool::new(10000)); diff --git a/datafusion/physical-plan/src/sorts/sort.rs b/datafusion/physical-plan/src/sorts/sort.rs index 4259696a04b64..464c6d77bf377 100644 --- a/datafusion/physical-plan/src/sorts/sort.rs +++ b/datafusion/physical-plan/src/sorts/sort.rs @@ -1388,15 +1388,16 @@ impl ExecutionPlan for SortExec { self: Arc, children: Vec>, ) -> Result> { - match has_same_children_properties(self.as_ref(), &children)? { - true => self.replace_children( + if has_same_children_properties(self.as_ref(), &children)? { + self.replace_children( children, ReplaceChildrenOptions::new(ChildrenPropertiesMode::Keep), - ), - false => self.replace_children( + ) + } else { + self.replace_children( children, ReplaceChildrenOptions::new(ChildrenPropertiesMode::Recompute), - ), + ) } } diff --git a/datafusion/physical-plan/src/unnest.rs b/datafusion/physical-plan/src/unnest.rs index d93e0280515c6..0f631b950983a 100644 --- a/datafusion/physical-plan/src/unnest.rs +++ b/datafusion/physical-plan/src/unnest.rs @@ -1004,9 +1004,10 @@ fn build_batch( // Original batch has the same columns // All unnesting results are written to temp_batch for depth in (1..=max_recursion).rev() { - let input = match depth == max_recursion { - true => batch.columns(), - false => &flatten_arrs, + let input = if depth == max_recursion { + batch.columns() + } else { + &flatten_arrs }; // Only sound for a single non-recursive level: with recursion the deeper // levels' lengths depend on arrays that do not exist yet, which is also why From 6599716a6e40194a562e609d3861ae06bdde5492 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 1 Sep 2026 14:16:50 +0200 Subject: [PATCH 6/6] Enable clippy::missing_fields_in_debug Manual `Debug` impls that skip a field now end in `finish_non_exhaustive()`, so the output says a field was omitted instead of implying the struct only has the ones listed. `Column` keeps `finish()` under an `#[expect]`: its `Debug` output appears verbatim in user-facing error messages. --- Cargo.toml | 1 - datafusion-cli/src/object_storage/instrumented.rs | 2 +- datafusion/common/src/column.rs | 4 ++++ datafusion/core/src/execution/session_state.rs | 2 +- datafusion/core/tests/custom_sources_cases/dml_planning.rs | 6 +++--- .../tests/user_defined/user_defined_scalar_functions.rs | 2 +- datafusion/datasource-csv/src/file_format.rs | 2 +- datafusion/datasource-parquet/src/opener/mod.rs | 2 +- datafusion/datasource/src/decoder.rs | 2 +- datafusion/datasource/src/memory.rs | 2 +- datafusion/execution/src/disk_manager.rs | 4 ++-- datafusion/execution/src/memory_pool/peak_recording.rs | 2 +- datafusion/expr/src/expr_fn.rs | 2 +- datafusion/expr/src/registry.rs | 2 +- datafusion/expr/src/test/function_stub.rs | 2 +- datafusion/physical-expr-common/src/binary_map.rs | 2 +- datafusion/physical-expr-common/src/binary_view_map.rs | 2 +- .../case/literal_lookup_table/primitive_lookup_table.rs | 2 +- .../physical-expr/src/expressions/dynamic_filters/mod.rs | 2 +- datafusion/physical-expr/src/expressions/in_list.rs | 2 +- datafusion/physical-expr/src/higher_order_function.rs | 2 +- datafusion/physical-expr/src/scalar_function.rs | 2 +- datafusion/physical-plan/src/joins/hash_join/exec.rs | 2 +- datafusion/physical-plan/src/joins/nested_loop_join.rs | 2 +- datafusion/physical-plan/src/memory.rs | 2 +- datafusion/physical-plan/src/sorts/stream.rs | 2 +- datafusion/physical-plan/src/stream.rs | 2 +- 27 files changed, 32 insertions(+), 29 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 919e4c0e3bf30..e0c6ca05537ba 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -292,7 +292,6 @@ map_unwrap_or = "allow" # 198 hits match_same_arms = "allow" # 261 hits match_wildcard_for_single_variants = "allow" # 132 hits missing_errors_doc = "allow" # 1807 hits -missing_fields_in_debug = "allow" # 29 hits missing_panics_doc = "allow" # 244 hits must_use_candidate = "allow" # 2726 hits needless_raw_string_hashes = "allow" # 540 hits diff --git a/datafusion-cli/src/object_storage/instrumented.rs b/datafusion-cli/src/object_storage/instrumented.rs index a0321cacb374b..062529c98d2be 100644 --- a/datafusion-cli/src/object_storage/instrumented.rs +++ b/datafusion-cli/src/object_storage/instrumented.rs @@ -499,7 +499,7 @@ impl fmt::Debug for RequestDetails { .field("size", &self.size) .field("range", &self.range) .field("extra_display", &self.extra_display) - .finish() + .finish_non_exhaustive() } } diff --git a/datafusion/common/src/column.rs b/datafusion/common/src/column.rs index b36b8f89779e3..5bd48f37fca79 100644 --- a/datafusion/common/src/column.rs +++ b/datafusion/common/src/column.rs @@ -37,6 +37,10 @@ pub struct Column { pub spans: Spans, } +#[expect( + clippy::missing_fields_in_debug, + reason = "this Debug output appears in user-facing error messages; `spans` is diagnostic bookkeeping and a `..` would only add noise" +)] impl fmt::Debug for Column { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("Column") diff --git a/datafusion/core/src/execution/session_state.rs b/datafusion/core/src/execution/session_state.rs index aa8ba4c3b733b..9cf3e4b27da5f 100644 --- a/datafusion/core/src/execution/session_state.rs +++ b/datafusion/core/src/execution/session_state.rs @@ -2006,7 +2006,7 @@ impl Debug for SessionStateBuilder { .field("higher_order_functions", &self.higher_order_functions) .field("aggregate_functions", &self.aggregate_functions) .field("window_functions", &self.window_functions) - .finish() + .finish_non_exhaustive() } } diff --git a/datafusion/core/tests/custom_sources_cases/dml_planning.rs b/datafusion/core/tests/custom_sources_cases/dml_planning.rs index cb5b134fab04a..670da4c312099 100644 --- a/datafusion/core/tests/custom_sources_cases/dml_planning.rs +++ b/datafusion/core/tests/custom_sources_cases/dml_planning.rs @@ -84,7 +84,7 @@ impl std::fmt::Debug for CaptureDeleteProvider { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("CaptureDeleteProvider") .field("schema", &self.schema) - .finish() + .finish_non_exhaustive() } } @@ -180,7 +180,7 @@ impl std::fmt::Debug for CaptureUpdateProvider { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("CaptureUpdateProvider") .field("schema", &self.schema) - .finish() + .finish_non_exhaustive() } } @@ -254,7 +254,7 @@ impl std::fmt::Debug for CaptureTruncateProvider { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("CaptureTruncateProvider") .field("schema", &self.schema) - .finish() + .finish_non_exhaustive() } } diff --git a/datafusion/core/tests/user_defined/user_defined_scalar_functions.rs b/datafusion/core/tests/user_defined/user_defined_scalar_functions.rs index dae491a1fb695..0d6bc71cef0b0 100644 --- a/datafusion/core/tests/user_defined/user_defined_scalar_functions.rs +++ b/datafusion/core/tests/user_defined/user_defined_scalar_functions.rs @@ -197,7 +197,7 @@ impl std::fmt::Debug for Simple0ArgsScalarUDF { .field("name", &self.name) .field("signature", &self.signature) .field("fun", &"") - .finish() + .finish_non_exhaustive() } } diff --git a/datafusion/datasource-csv/src/file_format.rs b/datafusion/datasource-csv/src/file_format.rs index a8c27369cb9c1..b357dbe6e5483 100644 --- a/datafusion/datasource-csv/src/file_format.rs +++ b/datafusion/datasource-csv/src/file_format.rs @@ -351,7 +351,7 @@ impl Debug for CsvSerializer { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("CsvSerializer") .field("header", &self.header) - .finish() + .finish_non_exhaustive() } } diff --git a/datafusion/datasource-parquet/src/opener/mod.rs b/datafusion/datasource-parquet/src/opener/mod.rs index 25c3bc9a77851..8aaffe06962a9 100644 --- a/datafusion/datasource-parquet/src/opener/mod.rs +++ b/datafusion/datasource-parquet/src/opener/mod.rs @@ -310,7 +310,7 @@ impl fmt::Debug for ParquetMorselizer { .field("preserve_order", &self.preserve_order) .field("enable_page_index", &self.enable_page_index) .field("enable_bloom_filter", &self.enable_bloom_filter) - .finish() + .finish_non_exhaustive() } } diff --git a/datafusion/datasource/src/decoder.rs b/datafusion/datasource/src/decoder.rs index a21aaedc52c3a..f7a7168d1bf8d 100644 --- a/datafusion/datasource/src/decoder.rs +++ b/datafusion/datasource/src/decoder.rs @@ -91,7 +91,7 @@ impl fmt::Debug for DecoderDeserializer { f.debug_struct("Deserializer") .field("buffered_queue", &self.buffered_queue) .field("finalized", &self.finalized) - .finish() + .finish_non_exhaustive() } } diff --git a/datafusion/datasource/src/memory.rs b/datafusion/datasource/src/memory.rs index f302fedd2b5db..d6994c52a3f54 100644 --- a/datafusion/datasource/src/memory.rs +++ b/datafusion/datasource/src/memory.rs @@ -925,7 +925,7 @@ impl Debug for MemSink { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("MemSink") .field("num_partitions", &self.batches.len()) - .finish() + .finish_non_exhaustive() } } diff --git a/datafusion/execution/src/disk_manager.rs b/datafusion/execution/src/disk_manager.rs index 313379f01291f..ce9ee180e9240 100644 --- a/datafusion/execution/src/disk_manager.rs +++ b/datafusion/execution/src/disk_manager.rs @@ -51,7 +51,7 @@ impl Debug for DiskManagerBuilder { f.debug_struct("DiskManagerBuilder") .field("mode", &self.mode) .field("max_temp_directory_size", &self.max_temp_directory_size) - .finish() + .finish_non_exhaustive() } } impl Default for DiskManagerBuilder { @@ -218,7 +218,7 @@ impl Debug for DiskManager { .field("used_disk_space", &self.used_disk_space) .field("active_files_count", &self.active_files_count) .field("factory", &self.factory.is_some()) - .finish() + .finish_non_exhaustive() } } /// Information about the current disk usage for spilling diff --git a/datafusion/execution/src/memory_pool/peak_recording.rs b/datafusion/execution/src/memory_pool/peak_recording.rs index b407cc0eaf36b..652ec3979d764 100644 --- a/datafusion/execution/src/memory_pool/peak_recording.rs +++ b/datafusion/execution/src/memory_pool/peak_recording.rs @@ -166,7 +166,7 @@ impl Debug for PeakRecordingPool { .field("inner", &self.inner) .field("peak", &self.peak_reserved()) .field("max", &self.max_reserved()) - .finish() + .finish_non_exhaustive() } } diff --git a/datafusion/expr/src/expr_fn.rs b/datafusion/expr/src/expr_fn.rs index b1a5a12d155ce..30d5bf70c87a1 100644 --- a/datafusion/expr/src/expr_fn.rs +++ b/datafusion/expr/src/expr_fn.rs @@ -542,7 +542,7 @@ impl Debug for SimpleAggregateUDF { .field("signature", &self.signature) .field("return_type", &self.return_type) .field("fun", &"") - .finish() + .finish_non_exhaustive() } } diff --git a/datafusion/expr/src/registry.rs b/datafusion/expr/src/registry.rs index 4b9744d9573b6..2b5a0c6451210 100644 --- a/datafusion/expr/src/registry.rs +++ b/datafusion/expr/src/registry.rs @@ -422,7 +422,7 @@ impl Debug for ExtensionTypeRegistration { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { f.debug_struct("DefaultExtensionTypeRegistration") .field("type_name", &self.name) - .finish() + .finish_non_exhaustive() } } diff --git a/datafusion/expr/src/test/function_stub.rs b/datafusion/expr/src/test/function_stub.rs index a1f29b649b2f8..6c99d06c9c3cf 100644 --- a/datafusion/expr/src/test/function_stub.rs +++ b/datafusion/expr/src/test/function_stub.rs @@ -220,7 +220,7 @@ impl std::fmt::Debug for Count { f.debug_struct("Count") .field("name", &self.name()) .field("signature", &self.signature) - .finish() + .finish_non_exhaustive() } } diff --git a/datafusion/physical-expr-common/src/binary_map.rs b/datafusion/physical-expr-common/src/binary_map.rs index 6024006f3278b..1633823b14b46 100644 --- a/datafusion/physical-expr-common/src/binary_map.rs +++ b/datafusion/physical-expr-common/src/binary_map.rs @@ -705,7 +705,7 @@ where .field("buffer", &self.buffer) .field("random_state", &self.random_state) .field("hashes_buffer", &self.hashes_buffer) - .finish() + .finish_non_exhaustive() } } diff --git a/datafusion/physical-expr-common/src/binary_view_map.rs b/datafusion/physical-expr-common/src/binary_view_map.rs index 7c0cdae11b70f..181110975c6fc 100644 --- a/datafusion/physical-expr-common/src/binary_view_map.rs +++ b/datafusion/physical-expr-common/src/binary_view_map.rs @@ -613,7 +613,7 @@ where .field("completed_buffers", &self.completed.len()) .field("random_state", &self.random_state) .field("hashes_buffer", &self.hashes_buffer) - .finish() + .finish_non_exhaustive() } } diff --git a/datafusion/physical-expr/src/expressions/case/literal_lookup_table/primitive_lookup_table.rs b/datafusion/physical-expr/src/expressions/case/literal_lookup_table/primitive_lookup_table.rs index 36d282c2a402b..46748b586e28a 100644 --- a/datafusion/physical-expr/src/expressions/case/literal_lookup_table/primitive_lookup_table.rs +++ b/datafusion/physical-expr/src/expressions/case/literal_lookup_table/primitive_lookup_table.rs @@ -46,7 +46,7 @@ where fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("PrimitiveIndexMap") .field("map", &self.map) - .finish() + .finish_non_exhaustive() } } diff --git a/datafusion/physical-expr/src/expressions/dynamic_filters/mod.rs b/datafusion/physical-expr/src/expressions/dynamic_filters/mod.rs index 15544d09e2b56..47089ecd93044 100644 --- a/datafusion/physical-expr/src/expressions/dynamic_filters/mod.rs +++ b/datafusion/physical-expr/src/expressions/dynamic_filters/mod.rs @@ -109,7 +109,7 @@ impl std::fmt::Debug for DynamicFilterPhysicalExpr { .field("state_watch", &self.state_watch) .field("data_type", &self.data_type) .field("nullable", &self.nullable) - .finish() + .finish_non_exhaustive() } } diff --git a/datafusion/physical-expr/src/expressions/in_list.rs b/datafusion/physical-expr/src/expressions/in_list.rs index 0a4ad0b804f0c..5bcf640d26d44 100644 --- a/datafusion/physical-expr/src/expressions/in_list.rs +++ b/datafusion/physical-expr/src/expressions/in_list.rs @@ -62,7 +62,7 @@ impl Debug for InListExpr { .field("expr", &self.expr) .field("list", &self.list) .field("negated", &self.negated) - .finish() + .finish_non_exhaustive() } } diff --git a/datafusion/physical-expr/src/higher_order_function.rs b/datafusion/physical-expr/src/higher_order_function.rs index e28b38bd7c8c1..e926f4501c825 100644 --- a/datafusion/physical-expr/src/higher_order_function.rs +++ b/datafusion/physical-expr/src/higher_order_function.rs @@ -115,7 +115,7 @@ impl Debug for HigherOrderFunctionExpr { .field("args", &self.args) .field("lambda_positions", &lambda_positions) .field("return_field", &self.return_field) - .finish() + .finish_non_exhaustive() } } diff --git a/datafusion/physical-expr/src/scalar_function.rs b/datafusion/physical-expr/src/scalar_function.rs index 6a5ab219aa8dd..757a7ac437a1d 100644 --- a/datafusion/physical-expr/src/scalar_function.rs +++ b/datafusion/physical-expr/src/scalar_function.rs @@ -64,7 +64,7 @@ impl Debug for ScalarFunctionExpr { .field("name", &self.name) .field("args", &self.args) .field("return_field", &self.return_field) - .finish() + .finish_non_exhaustive() } } diff --git a/datafusion/physical-plan/src/joins/hash_join/exec.rs b/datafusion/physical-plan/src/joins/hash_join/exec.rs index 94875cb6189aa..b6146597fe12b 100644 --- a/datafusion/physical-plan/src/joins/hash_join/exec.rs +++ b/datafusion/physical-plan/src/joins/hash_join/exec.rs @@ -913,7 +913,7 @@ impl fmt::Debug for HashJoinExec { .field("null_equality", &self.null_equality) .field("cache", &self.cache) // Explicitly exclude dynamic_filter to avoid runtime state differences in tests - .finish() + .finish_non_exhaustive() } } diff --git a/datafusion/physical-plan/src/joins/nested_loop_join.rs b/datafusion/physical-plan/src/joins/nested_loop_join.rs index d22270c3550b5..3c6f7776d5ca5 100644 --- a/datafusion/physical-plan/src/joins/nested_loop_join.rs +++ b/datafusion/physical-plan/src/joins/nested_loop_join.rs @@ -1749,7 +1749,7 @@ impl std::fmt::Debug for FallbackCoordinator { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { f.debug_struct("FallbackCoordinator") .field("right_partition_count", &self.right_partition_count) - .finish() + .finish_non_exhaustive() } } diff --git a/datafusion/physical-plan/src/memory.rs b/datafusion/physical-plan/src/memory.rs index 0b6bdf4490d8b..72a676a2180c4 100644 --- a/datafusion/physical-plan/src/memory.rs +++ b/datafusion/physical-plan/src/memory.rs @@ -270,7 +270,7 @@ impl fmt::Debug for LazyMemoryExec { f.debug_struct("LazyMemoryExec") .field("schema", &self.schema) .field("batch_generators", &self.batch_generators) - .finish() + .finish_non_exhaustive() } } diff --git a/datafusion/physical-plan/src/sorts/stream.rs b/datafusion/physical-plan/src/sorts/stream.rs index 4b66157b08540..0233759f8c057 100644 --- a/datafusion/physical-plan/src/sorts/stream.rs +++ b/datafusion/physical-plan/src/sorts/stream.rs @@ -229,7 +229,7 @@ impl std::fmt::Debug for FieldCursorStream { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("PrimitiveCursorStream") .field("num_streams", &self.streams) - .finish() + .finish_non_exhaustive() } } diff --git a/datafusion/physical-plan/src/stream.rs b/datafusion/physical-plan/src/stream.rs index bc549f442001c..ea757551625b7 100644 --- a/datafusion/physical-plan/src/stream.rs +++ b/datafusion/physical-plan/src/stream.rs @@ -451,7 +451,7 @@ impl std::fmt::Debug for RecordBatchStreamAdapter { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("RecordBatchStreamAdapter") .field("schema", &self.schema) - .finish() + .finish_non_exhaustive() } }