From 18caf3957cc9336783faaf3aa2a43b86a38c3693 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 21:30:48 +0200 Subject: [PATCH 01/16] chore: sort the workspace clippy lint list alphabetically Pure reordering, no lint is added or removed. Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 87c23cc456651..03b90480fe164 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -209,20 +209,21 @@ url = "2.5.7" uuid = "1.23" zstd = { version = "0.13", default-features = false } +# Keep this list sorted alphabetically. [workspace.lints.clippy] +# https://github.com/apache/datafusion/issues/18881 +allow_attributes = "warn" +assigning_clones = "warn" +inefficient_to_string = "warn" # Detects large stack-allocated futures that may cause stack overflow crashes (see threshold in clippy.toml) large_futures = "warn" -used_underscore_binding = "warn" -or_fun_call = "warn" -unnecessary_lazy_evaluations = "warn" -uninlined_format_args = "warn" -inefficient_to_string = "warn" # https://github.com/apache/datafusion/issues/18503 needless_pass_by_value = "warn" -# https://github.com/apache/datafusion/issues/18881 -allow_attributes = "warn" -assigning_clones = "warn" +or_fun_call = "warn" +uninlined_format_args = "warn" +unnecessary_lazy_evaluations = "warn" unused_async = "warn" +used_underscore_binding = "warn" [workspace.lints.rust] unexpected_cfgs = { level = "warn", check-cfg = [ From c4ee10adebf500492a159826b89439ddcad1baac Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 21:47:30 +0200 Subject: [PATCH 02/16] chore: inherit the workspace lints in the datafusion-proto* crates `datafusion/proto`, `datafusion/proto-common` and `datafusion/proto-models` were the only workspace members without `[lints] workspace = true`, so the workspace lint configuration silently did not apply to them. Enabling it surfaces a handful of violations, fixed here: * `unused_qualifications`: redundant path prefixes (mostly `cargo fix`ed) * `clippy::or_fun_call`: `unwrap_or(HashMap::new())` -> `unwrap_or_default()` * `clippy::needless_pass_by_value`: one `#[expect]` on a public API, plus a crate-level exemption for the integration tests matching what the other DataFusion crates already do for their unit tests The generated protobuf modules opt out of `unused_qualifications` alongside the `clippy::all` opt-out they already had. Co-Authored-By: Claude Opus 5 (1M context) --- datafusion/proto-common/Cargo.toml | 6 +++ datafusion/proto-common/src/from_proto/mod.rs | 4 +- datafusion/proto-common/src/generated/mod.rs | 1 + datafusion/proto-common/src/to_proto/mod.rs | 52 ++++++------------- datafusion/proto-models/Cargo.toml | 6 +++ datafusion/proto-models/src/generated/mod.rs | 1 + datafusion/proto/Cargo.toml | 6 +++ datafusion/proto/src/bytes/mod.rs | 1 + datafusion/proto/src/convert.rs | 2 +- .../proto/src/logical_plan/file_formats.rs | 8 ++- datafusion/proto/src/logical_plan/mod.rs | 6 +-- datafusion/proto/src/logical_plan/to_proto.rs | 6 +-- .../tests/cases/roundtrip_logical_plan.rs | 12 ++--- .../tests/cases/roundtrip_physical_plan.rs | 33 ++++++------ datafusion/proto/tests/proto_integration.rs | 4 ++ 15 files changed, 73 insertions(+), 75 deletions(-) diff --git a/datafusion/proto-common/Cargo.toml b/datafusion/proto-common/Cargo.toml index 46dae36ba40ed..0670d7cbf757f 100644 --- a/datafusion/proto-common/Cargo.toml +++ b/datafusion/proto-common/Cargo.toml @@ -31,6 +31,12 @@ rust-version = { workspace = true } [package.metadata.docs.rs] all-features = true +# Note: add additional linter rules in lib.rs. +# Rust does not support workspace + new linter rules in subcrates yet +# https://github.com/rust-lang/cargo/issues/13157 +[lints] +workspace = true + [lib] name = "datafusion_proto_common" diff --git a/datafusion/proto-common/src/from_proto/mod.rs b/datafusion/proto-common/src/from_proto/mod.rs index 97cc9af230105..1fe4d2ad6a2a7 100644 --- a/datafusion/proto-common/src/from_proto/mod.rs +++ b/datafusion/proto-common/src/from_proto/mod.rs @@ -1259,9 +1259,7 @@ fn vec_to_array(v: Vec) -> [T; N] { } /// Converts a vector of `protobuf::Field`s to `Arc`s. -pub fn parse_proto_fields_to_fields<'a, I>( - fields: I, -) -> std::result::Result, Error> +pub fn parse_proto_fields_to_fields<'a, I>(fields: I) -> Result, Error> where I: IntoIterator, { diff --git a/datafusion/proto-common/src/generated/mod.rs b/datafusion/proto-common/src/generated/mod.rs index 9c2ca9385aa5e..e5b384c9c5b88 100644 --- a/datafusion/proto-common/src/generated/mod.rs +++ b/datafusion/proto-common/src/generated/mod.rs @@ -18,6 +18,7 @@ // This code is generated so we don't want to fix any lint violations manually #[allow(clippy::allow_attributes)] #[allow(clippy::all)] +#[allow(unused_qualifications)] #[rustfmt::skip] pub mod datafusion_proto_common { include!("prost.rs"); diff --git a/datafusion/proto-common/src/to_proto/mod.rs b/datafusion/proto-common/src/to_proto/mod.rs index d2e1ca50c812d..4fa19b5f9561a 100644 --- a/datafusion/proto-common/src/to_proto/mod.rs +++ b/datafusion/proto-common/src/to_proto/mod.rs @@ -115,7 +115,7 @@ impl TryFrom<&DataType> for protobuf::ArrowType { } } -impl TryFrom<&DataType> for protobuf::arrow_type::ArrowTypeEnum { +impl TryFrom<&DataType> for ArrowTypeEnum { type Error = Error; fn try_from(val: &DataType) -> Result { @@ -439,9 +439,7 @@ impl TryFrom<&ScalarValue> for protobuf::ScalarValue { }) } None => Ok(protobuf::ScalarValue { - value: Some(protobuf::scalar_value::Value::NullValue( - (&data_type).try_into()?, - )), + value: Some(Value::NullValue((&data_type).try_into()?)), }), }, ScalarValue::Decimal64(val, p, s) => match *val { @@ -457,9 +455,7 @@ impl TryFrom<&ScalarValue> for protobuf::ScalarValue { }) } None => Ok(protobuf::ScalarValue { - value: Some(protobuf::scalar_value::Value::NullValue( - (&data_type).try_into()?, - )), + value: Some(Value::NullValue((&data_type).try_into()?)), }), }, ScalarValue::Decimal128(val, p, s) => match *val { @@ -475,9 +471,7 @@ impl TryFrom<&ScalarValue> for protobuf::ScalarValue { }) } None => Ok(protobuf::ScalarValue { - value: Some(protobuf::scalar_value::Value::NullValue( - (&data_type).try_into()?, - )), + value: Some(Value::NullValue((&data_type).try_into()?)), }), }, ScalarValue::Decimal256(val, p, s) => match *val { @@ -493,9 +487,7 @@ impl TryFrom<&ScalarValue> for protobuf::ScalarValue { }) } None => Ok(protobuf::ScalarValue { - value: Some(protobuf::scalar_value::Value::NullValue( - (&data_type).try_into()?, - )), + value: Some(Value::NullValue((&data_type).try_into()?)), }), }, ScalarValue::Date64(val) => { @@ -788,8 +780,8 @@ impl From<&Precision> for protobuf::Precision { } } -impl From<&Precision> for protobuf::Precision { - fn from(s: &Precision) -> protobuf::Precision { +impl From<&Precision> for protobuf::Precision { + fn from(s: &Precision) -> protobuf::Precision { match s { Precision::Exact(val) => protobuf::Precision { precision_info: protobuf::PrecisionInfo::Exact.into(), @@ -1076,16 +1068,14 @@ impl TryFrom<&JsonOptions> for protobuf::JsonOptions { /// Creates a scalar protobuf value from an optional value (T), and /// encoding None as the appropriate datatype -fn create_proto_scalar protobuf::scalar_value::Value>( +fn create_proto_scalar Value>( v: Option<&I>, null_arrow_type: &DataType, constructor: T, ) -> Result { let value = v .map(constructor) - .unwrap_or(protobuf::scalar_value::Value::NullValue( - null_arrow_type.try_into()?, - )); + .unwrap_or(Value::NullValue(null_arrow_type.try_into()?)); Ok(protobuf::ScalarValue { value: Some(value) }) } @@ -1141,35 +1131,25 @@ fn encode_scalar_nested_value( match val { ScalarValue::List(_) => Ok(protobuf::ScalarValue { - value: Some(protobuf::scalar_value::Value::ListValue(scalar_list_value)), + value: Some(Value::ListValue(scalar_list_value)), }), ScalarValue::LargeList(_) => Ok(protobuf::ScalarValue { - value: Some(protobuf::scalar_value::Value::LargeListValue( - scalar_list_value, - )), + value: Some(Value::LargeListValue(scalar_list_value)), }), ScalarValue::FixedSizeList(_) => Ok(protobuf::ScalarValue { - value: Some(protobuf::scalar_value::Value::FixedSizeListValue( - scalar_list_value, - )), + value: Some(Value::FixedSizeListValue(scalar_list_value)), }), ScalarValue::ListView(_) => Ok(protobuf::ScalarValue { - value: Some(protobuf::scalar_value::Value::ListViewValue( - scalar_list_value, - )), + value: Some(Value::ListViewValue(scalar_list_value)), }), ScalarValue::LargeListView(_) => Ok(protobuf::ScalarValue { - value: Some(protobuf::scalar_value::Value::LargeListViewValue( - scalar_list_value, - )), + value: Some(Value::LargeListViewValue(scalar_list_value)), }), ScalarValue::Struct(_) => Ok(protobuf::ScalarValue { - value: Some(protobuf::scalar_value::Value::StructValue( - scalar_list_value, - )), + value: Some(Value::StructValue(scalar_list_value)), }), ScalarValue::Map(_) => Ok(protobuf::ScalarValue { - value: Some(protobuf::scalar_value::Value::MapValue(scalar_list_value)), + value: Some(Value::MapValue(scalar_list_value)), }), _ => unreachable!(), } diff --git a/datafusion/proto-models/Cargo.toml b/datafusion/proto-models/Cargo.toml index e37c4a2dba326..d8cf5fcdc3dce 100644 --- a/datafusion/proto-models/Cargo.toml +++ b/datafusion/proto-models/Cargo.toml @@ -31,6 +31,12 @@ rust-version = { workspace = true } [package.metadata.docs.rs] all-features = true +# Note: add additional linter rules in lib.rs. +# Rust does not support workspace + new linter rules in subcrates yet +# https://github.com/rust-lang/cargo/issues/13157 +[lints] +workspace = true + [lib] name = "datafusion_proto_models" diff --git a/datafusion/proto-models/src/generated/mod.rs b/datafusion/proto-models/src/generated/mod.rs index ca32b1500d57b..4362b741d93a9 100644 --- a/datafusion/proto-models/src/generated/mod.rs +++ b/datafusion/proto-models/src/generated/mod.rs @@ -18,6 +18,7 @@ // This code is generated so we don't want to fix any lint violations manually #[allow(clippy::allow_attributes)] #[allow(clippy::all)] +#[allow(unused_qualifications)] #[rustfmt::skip] pub mod datafusion { include!("prost.rs"); diff --git a/datafusion/proto/Cargo.toml b/datafusion/proto/Cargo.toml index 037be27769f4d..dd2cf8e219446 100644 --- a/datafusion/proto/Cargo.toml +++ b/datafusion/proto/Cargo.toml @@ -31,6 +31,12 @@ rust-version = { workspace = true } [package.metadata.docs.rs] all-features = true +# Note: add additional linter rules in lib.rs. +# Rust does not support workspace + new linter rules in subcrates yet +# https://github.com/rust-lang/cargo/issues/13157 +[lints] +workspace = true + [lib] name = "datafusion_proto" diff --git a/datafusion/proto/src/bytes/mod.rs b/datafusion/proto/src/bytes/mod.rs index 2b7d7ed8e849b..ab013f8dd549e 100644 --- a/datafusion/proto/src/bytes/mod.rs +++ b/datafusion/proto/src/bytes/mod.rs @@ -213,6 +213,7 @@ pub fn physical_plan_to_bytes_with_extension_codec( /// Serialize a PhysicalPlan as bytes, using the provided extension codec /// and protobuf converter. +#[expect(clippy::needless_pass_by_value)] // Taking the plan by value is part of the public API pub fn physical_plan_to_bytes_with_proto_converter( plan: Arc, extension_codec: &dyn PhysicalExtensionCodec, diff --git a/datafusion/proto/src/convert.rs b/datafusion/proto/src/convert.rs index cb5c5bd7f8c12..87e9a431dcb80 100644 --- a/datafusion/proto/src/convert.rs +++ b/datafusion/proto/src/convert.rs @@ -40,5 +40,5 @@ pub trait FromProto: Sized { /// versa). Mirrors [`TryFrom`]. pub trait TryFromProto: Sized { type Error; - fn try_from_proto(value: T) -> std::result::Result; + fn try_from_proto(value: T) -> Result; } diff --git a/datafusion/proto/src/logical_plan/file_formats.rs b/datafusion/proto/src/logical_plan/file_formats.rs index 8940b16bf83f5..d35a77abb16ea 100644 --- a/datafusion/proto/src/logical_plan/file_formats.rs +++ b/datafusion/proto/src/logical_plan/file_formats.rs @@ -767,11 +767,9 @@ mod parquet { exec_datafusion_err!("Failed to decode TableParquetOptionsProto: {e:?}") })?; let options = TableParquetOptions::try_from_proto(&proto)?; - Ok(Arc::new( - datafusion_datasource_parquet::file_format::ParquetFormatFactory { - options: Some(options), - }, - )) + Ok(Arc::new(ParquetFormatFactory { + options: Some(options), + })) } fn try_encode_file_format( diff --git a/datafusion/proto/src/logical_plan/mod.rs b/datafusion/proto/src/logical_plan/mod.rs index 732676a3c0a0f..653ae9ab05355 100644 --- a/datafusion/proto/src/logical_plan/mod.rs +++ b/datafusion/proto/src/logical_plan/mod.rs @@ -680,7 +680,7 @@ impl AsLogicalPlan for LogicalPlanNode { )? .build() } - LogicalPlanType::CustomScan(scan) => { + CustomScan(scan) => { let schema: Schema = convert_required!(scan.schema)?; let schema = Arc::new(schema); let mut projection = None; @@ -1272,7 +1272,7 @@ impl AsLogicalPlan for LogicalPlanNode { LogicalPlanType::Dml(dml_node) => { let write_op = from_proto::parse_write_op(dml_node, ctx, extension_codec)?; - Ok(LogicalPlan::Dml(datafusion_expr::DmlStatement::new( + Ok(LogicalPlan::Dml(DmlStatement::new( from_table_reference(dml_node.table_name.as_ref(), "DML ")?, to_table_source(&dml_node.target, ctx, extension_codec)?, write_op, @@ -1479,7 +1479,7 @@ impl AsLogicalPlan for LogicalPlanNode { Ok(LogicalPlanNode { logical_plan_type: Some(LogicalPlanType::CteWorkTableScan( - protobuf::CteWorkTableScanNode { + CteWorkTableScanNode { name, schema: Some(schema), }, diff --git a/datafusion/proto/src/logical_plan/to_proto.rs b/datafusion/proto/src/logical_plan/to_proto.rs index 89de342ff00b7..67c815add8460 100644 --- a/datafusion/proto/src/logical_plan/to_proto.rs +++ b/datafusion/proto/src/logical_plan/to_proto.rs @@ -19,8 +19,6 @@ //! DataFusion logical plans to be serialized and transmitted between //! processes. -use std::collections::HashMap; - use datafusion_common::{NullEquality, SplitPoint, TableReference, UnnestOptions}; use datafusion_expr::dml::{ MergeIntoAction, MergeIntoClause, MergeIntoClauseKind, MergeIntoOp, @@ -230,7 +228,7 @@ pub fn serialize_expr( metadata: metadata .as_ref() .map(|m| m.to_hashmap()) - .unwrap_or(HashMap::new()), + .unwrap_or_default(), }); protobuf::LogicalExprNode { expr_type: Some(ExprType::Alias(alias)), @@ -661,7 +659,7 @@ pub fn serialize_expr( metadata: field .as_ref() .map(|f| f.metadata().clone()) - .unwrap_or(HashMap::new()), + .unwrap_or_default(), })), }, Expr::Lambda(Lambda { params, body }) => protobuf::LogicalExprNode { diff --git a/datafusion/proto/tests/cases/roundtrip_logical_plan.rs b/datafusion/proto/tests/cases/roundtrip_logical_plan.rs index 0e77aa76f4a4d..1418998b436c9 100644 --- a/datafusion/proto/tests/cases/roundtrip_logical_plan.rs +++ b/datafusion/proto/tests/cases/roundtrip_logical_plan.rs @@ -150,7 +150,7 @@ fn roundtrip_expr_test_with_codec( let round_trip: Expr = from_proto::parse_expr(&proto, ctx.task_ctx().as_ref(), codec).unwrap(); - assert_eq!(format!("{:?}", initial_struct), format!("{round_trip:?}")); + assert_eq!(format!("{initial_struct:?}"), format!("{round_trip:?}")); roundtrip_json_test(&proto); } @@ -1704,7 +1704,7 @@ pub mod proto { pub expr: Option, } - #[allow(dead_code)] + #[expect(dead_code)] #[derive(Clone, PartialEq, Eq, ::prost::Message)] pub struct TopKExecProto { #[prost(uint64, tag = "1")] @@ -2517,7 +2517,7 @@ fn roundtrip_null_scalar_values() { for test_case in test_types.into_iter() { let proto_scalar: protobuf::ScalarValue = (&test_case).try_into().unwrap(); let returned_scalar: ScalarValue = (&proto_scalar).try_into().unwrap(); - assert_eq!(format!("{:?}", test_case), format!("{returned_scalar:?}")); + assert_eq!(format!("{test_case:?}"), format!("{returned_scalar:?}")); } } @@ -3024,7 +3024,7 @@ fn roundtrip_scalar_udf_extension_codec() { from_proto::parse_expr(&proto, ctx.task_ctx().as_ref(), &UDFExtensionCodec) .expect("parse expr"); - assert_eq!(format!("{:?}", test_expr), format!("{round_trip:?}")); + assert_eq!(format!("{test_expr:?}"), format!("{round_trip:?}")); roundtrip_json_test(&proto); } @@ -3038,7 +3038,7 @@ fn roundtrip_aggregate_udf_extension_codec() { from_proto::parse_expr(&proto, ctx.task_ctx().as_ref(), &UDFExtensionCodec) .expect("parse expr"); - assert_eq!(format!("{:?}", test_expr), format!("{round_trip:?}")); + assert_eq!(format!("{test_expr:?}"), format!("{round_trip:?}")); roundtrip_json_test(&proto); } @@ -3147,7 +3147,7 @@ fn roundtrip_higher_order_udf_extension_codec() { from_proto::parse_expr(&proto, ctx.task_ctx().as_ref(), &UDFExtensionCodec) .expect("parse expr"); - assert_eq!(format!("{:?}", test_expr), format!("{round_trip:?}")); + assert_eq!(format!("{test_expr:?}"), format!("{round_trip:?}")); roundtrip_json_test(&proto); } diff --git a/datafusion/proto/tests/cases/roundtrip_physical_plan.rs b/datafusion/proto/tests/cases/roundtrip_physical_plan.rs index 19a5ca337d7f6..b22cfd7764a21 100644 --- a/datafusion/proto/tests/cases/roundtrip_physical_plan.rs +++ b/datafusion/proto/tests/cases/roundtrip_physical_plan.rs @@ -282,7 +282,7 @@ fn decode_empty_and_placeholder_row_without_partitions() -> Result<()> { }, ), ] { - let node = protobuf::PhysicalPlanNode { + let node = PhysicalPlanNode { physical_plan_type: Some(physical_plan_type), }; let plan = node.try_into_physical_plan(ctx.task_ctx().as_ref(), &codec)?; @@ -1401,7 +1401,7 @@ fn roundtrip_parquet_exec_with_custom_predicate_expr() -> Result<()> { } fn fmt_sql(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - std::fmt::Display::fmt(self, f) + Display::fmt(self, f) } } @@ -2761,7 +2761,7 @@ fn deprecated_projection_shim_decodes_argument_not_self() -> Result<()> { let session_ctx = SessionContext::new(); let task_ctx = session_ctx.task_ctx(); let decode_ctx = PhysicalPlanDecodeContext::new(task_ctx.as_ref(), &codec); - #[allow(deprecated)] + #[expect(deprecated)] let decoded = unrelated_node.try_into_projection_physical_plan( projection_exec_node, &decode_ctx, @@ -3120,20 +3120,20 @@ fn roundtrip_sort_merge_join() -> Result<()> { Arc::new(Column::new("col_b", schema_right.index_of("col_b")?)) as _, )]; - let filter = datafusion::physical_plan::joins::utils::JoinFilter::new( + let filter = JoinFilter::new( Arc::new(BinaryExpr::new( Arc::new(Column::new("col_a", 1)), Operator::Gt, Arc::new(Column::new("col_b", 0)), )), vec![ - datafusion::physical_plan::joins::utils::ColumnIndex { + ColumnIndex { index: 0, - side: datafusion_common::JoinSide::Left, + side: JoinSide::Left, }, - datafusion::physical_plan::joins::utils::ColumnIndex { + ColumnIndex { index: 0, - side: datafusion_common::JoinSide::Right, + side: JoinSide::Right, }, ], Arc::new(Schema::new(vec![field_a, field_b])), @@ -3339,7 +3339,7 @@ fn roundtrip_hash_table_lookup_expr_to_lit() -> Result<()> { // Create a HashTableLookupExpr - it will be replaced with lit(true) during serialization let hash_map = Arc::new(Map::HashMap(Box::new(JoinHashMapU32::with_capacity(0)))); - let on_columns = vec![datafusion::physical_plan::expressions::col("col", &schema)?]; + let on_columns = vec![col("col", &schema)?]; let lookup_expr: Arc = Arc::new(HashTableLookupExpr::new( on_columns, datafusion::physical_plan::joins::SeededRandomState::with_seed(0), @@ -3419,7 +3419,7 @@ fn custom_proto_converter_intercepts() -> Result<()> { impl PhysicalProtoConverterExtension for CustomConverterInterceptor { fn proto_to_execution_plan( &self, - proto: &protobuf::PhysicalPlanNode, + proto: &PhysicalPlanNode, ctx: &PhysicalPlanDecodeContext<'_>, ) -> Result> { { @@ -3436,7 +3436,7 @@ fn custom_proto_converter_intercepts() -> Result<()> { &self, plan: &Arc, codec: &dyn PhysicalExtensionCodec, - ) -> Result + ) -> Result where Self: Sized, { @@ -3624,7 +3624,7 @@ fn roundtrip_dynamic_filter_expr_pair( /// - `dynamic_filter_2` before serialization /// - `dynamic_filter_1` after serialization /// - `dynamic_filter_2` after serialization -#[allow(clippy::type_complexity)] +#[expect(clippy::type_complexity)] fn roundtrip_dynamic_filter_plan_pair() -> Result<( Arc, Arc, @@ -4670,7 +4670,7 @@ impl ExecutionPlan for CustomExecWithExprs { self.child.schema() } - fn properties(&self) -> &Arc { + fn properties(&self) -> &Arc { self.child.properties() } @@ -4948,7 +4948,7 @@ impl PhysicalExpr for WrapperExpr { })) } fn fmt_sql(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - std::fmt::Display::fmt(self, f) + Display::fmt(self, f) } } @@ -4956,7 +4956,7 @@ impl PhysicalExpr for WrapperExpr { #[derive(Clone, PartialEq, prost::Message)] struct WrapperExprProto { #[prost(message, optional, boxed, tag = "1")] - inner: Option>, + inner: Option>, } #[derive(Debug)] @@ -5054,8 +5054,7 @@ fn extension_codec_expr_participates_in_deduplication() -> Result<()> { // Encode, then round-trip through prost bytes to mimic the wire. let proto = converter.physical_expr_to_proto(&composite, &codec)?; let bytes = proto.encode_to_vec(); - let decoded_proto = - datafusion_proto::protobuf::PhysicalExprNode::decode(bytes.as_slice()).unwrap(); + let decoded_proto = PhysicalExprNode::decode(bytes.as_slice()).unwrap(); let ctx = SessionContext::new(); let task_ctx = ctx.task_ctx(); diff --git a/datafusion/proto/tests/proto_integration.rs b/datafusion/proto/tests/proto_integration.rs index 6ce41c9de71a8..07a72f13ffb82 100644 --- a/datafusion/proto/tests/proto_integration.rs +++ b/datafusion/proto/tests/proto_integration.rs @@ -15,5 +15,9 @@ // specific language governing permissions and limitations // under the License. +// Test helpers take owned values for convenience, matching the `#![cfg_attr(test, ...)]` +// exemption the DataFusion crates apply to their own unit tests. +#![cfg_attr(test, allow(clippy::needless_pass_by_value))] + /// Run all tests that are found in the `cases` directory mod cases; From 5cc8f99704fb004c9b9018ae3c1e65bbae196e87 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 21:50:21 +0200 Subject: [PATCH 03/16] feat: enable a batch of non-default clippy lints workspace-wide All of these are already violation-free across the workspace, so this is pure future-proofing: they only fire on newly written code. They are a subset of the lint set used by https://github.com/emilk/egui, picked because they trigger no (or almost no) violations in DataFusion today. The single existing violation is a deliberate `mem::forget` in an FFI test helper, now marked with `#[expect]`. Part of https://github.com/apache/datafusion/issues/18467 Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 51 +++++++++++++++++++++++++++++++ datafusion/ffi/src/tests/utils.rs | 1 + 2 files changed, 52 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index 03b90480fe164..2bfe382921f80 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -210,20 +210,71 @@ uuid = "1.23" zstd = { version = "0.13", default-features = false } # Keep this list sorted alphabetically. +# See https://github.com/apache/datafusion/issues/18467 for the ongoing effort of +# picking useful non-default lints. [workspace.lints.clippy] # https://github.com/apache/datafusion/issues/18881 allow_attributes = "warn" +as_ptr_cast_mut = "warn" assigning_clones = "warn" +char_lit_as_u8 = "warn" +clear_with_drain = "warn" +coerce_container_to_any = "warn" +decimal_bitwise_operands = "warn" +default_union_representation = "warn" +empty_enum_variants_with_brackets = "warn" +empty_line_after_outer_attr = "warn" +exit = "warn" +fn_to_numeric_cast_any = "warn" +if_let_mutex = "warn" +imprecise_flops = "warn" +index_refutable_slice = "warn" inefficient_to_string = "warn" +infinite_loop = "warn" +invalid_upcast_comparisons = "warn" +ip_constant = "warn" +iter_filter_is_ok = "warn" +iter_filter_is_some = "warn" +iter_on_empty_collections = "warn" # Detects large stack-allocated futures that may cause stack overflow crashes (see threshold in clippy.toml) large_futures = "warn" +large_include_file = "warn" +macro_use_imports = "warn" +manual_ilog2 = "warn" +manual_is_power_of_two = "warn" +manual_ok_or = "warn" +match_wild_err_arm = "warn" +mem_forget = "warn" +mismatching_type_param_order = "warn" +missing_enforced_import_renames = "warn" +mut_mut = "warn" +mutex_integer = "warn" # https://github.com/apache/datafusion/issues/18503 needless_pass_by_value = "warn" +negative_feature_names = "warn" +non_zero_suggestions = "warn" +nonstandard_macro_braces = "warn" or_fun_call = "warn" +path_buf_push_overwrite = "warn" +precedence_bits = "warn" +pub_underscore_fields = "warn" +rc_mutex = "warn" +same_length_and_capacity = "warn" +string_add_assign = "warn" +suspicious_command_arg_space = "warn" +suspicious_xor_used_as_pow = "warn" +trailing_empty_array = "warn" +transmute_ptr_to_ptr = "warn" +uninhabited_references = "warn" uninlined_format_args = "warn" unnecessary_lazy_evaluations = "warn" +unnecessary_self_imports = "warn" unused_async = "warn" +unused_rounding = "warn" used_underscore_binding = "warn" +useless_transmute = "warn" +wildcard_dependencies = "warn" +zero_sized_map_values = "warn" [workspace.lints.rust] unexpected_cfgs = { level = "warn", check-cfg = [ diff --git a/datafusion/ffi/src/tests/utils.rs b/datafusion/ffi/src/tests/utils.rs index b6b50cbce875c..b975a87394efe 100644 --- a/datafusion/ffi/src/tests/utils.rs +++ b/datafusion/ffi/src/tests/utils.rs @@ -83,6 +83,7 @@ pub fn get_module() -> Result { assert_eq!((module.version)(), expected_version); // Leak the library to keep it loaded for the duration of the test + #[expect(clippy::mem_forget)] std::mem::forget(lib); Ok(module) From b5f03760aa1bed13beb3ed12b3b03793691a83ed Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 21:52:09 +0200 Subject: [PATCH 04/16] feat: enable clippy::manual_instant_elapsed Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 1 + datafusion/physical-plan/src/analyze.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 2bfe382921f80..4264f087a1d8b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -241,6 +241,7 @@ large_futures = "warn" large_include_file = "warn" macro_use_imports = "warn" manual_ilog2 = "warn" +manual_instant_elapsed = "warn" manual_is_power_of_two = "warn" manual_ok_or = "warn" match_wild_err_arm = "warn" diff --git a/datafusion/physical-plan/src/analyze.rs b/datafusion/physical-plan/src/analyze.rs index 31e0a27410ff9..85fffc4a1d901 100644 --- a/datafusion/physical-plan/src/analyze.rs +++ b/datafusion/physical-plan/src/analyze.rs @@ -284,7 +284,7 @@ impl ExecutionPlan for AnalyzeExec { } drop(input_stream); - let duration = Instant::now() - start; + let duration = start.elapsed(); create_output_batch( verbose, show_statistics, From a81afe49f865078da341dedd423b9a4e3aa12dad Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 21:53:47 +0200 Subject: [PATCH 05/16] feat: enable clippy::pathbuf_init_then_push Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 1 + datafusion/datasource-parquet/src/metadata.rs | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 4264f087a1d8b..dcebb7bc3cd4c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -257,6 +257,7 @@ non_zero_suggestions = "warn" nonstandard_macro_braces = "warn" or_fun_call = "warn" path_buf_push_overwrite = "warn" +pathbuf_init_then_push = "warn" precedence_bits = "warn" pub_underscore_fields = "warn" rc_mutex = "warn" diff --git a/datafusion/datasource-parquet/src/metadata.rs b/datafusion/datasource-parquet/src/metadata.rs index 56abf52144028..9db5e565caaeb 100644 --- a/datafusion/datasource-parquet/src/metadata.rs +++ b/datafusion/datasource-parquet/src/metadata.rs @@ -1488,8 +1488,8 @@ mod tests { #[test] fn test_distinct_count_from_real_parquet_file() { // Path to test file created by DuckDB with distinct_count statistics - let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR")); - path.push("src/test_data/ndv_test.parquet"); + let path = PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("src/test_data/ndv_test.parquet"); let file = File::open(&path).expect("Failed to open test parquet file"); let reader = From 3cb5e7a3d1d3dd0ad383caf42e72b0c74542c1af Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 21:55:50 +0200 Subject: [PATCH 06/16] feat: enable clippy::ref_option_ref Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 1 + datafusion/functions-nested/src/range.rs | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index dcebb7bc3cd4c..f49c87cc91b8c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -261,6 +261,7 @@ pathbuf_init_then_push = "warn" precedence_bits = "warn" pub_underscore_fields = "warn" rc_mutex = "warn" +ref_option_ref = "warn" same_length_and_capacity = "warn" string_add_assign = "warn" suspicious_command_arg_space = "warn" diff --git a/datafusion/functions-nested/src/range.rs b/datafusion/functions-nested/src/range.rs index 65d9244ecdd4c..0a02a8b7bbd72 100644 --- a/datafusion/functions-nested/src/range.rs +++ b/datafusion/functions-nested/src/range.rs @@ -434,8 +434,8 @@ impl Range { let stop = cast_to_ns(stop)?; let stop = as_timestamp_nanosecond_array(&stop)?; - let start_tz = parse_tz(&start.timezone())?; - let stop_tz = parse_tz(&stop.timezone())?; + let start_tz = parse_tz(start.timezone())?; + let stop_tz = parse_tz(stop.timezone())?; // values are timestamps let values_builder = start @@ -609,8 +609,8 @@ fn generate_range_values( Ok(()) } -fn parse_tz(tz: &Option<&str>) -> Result { - let tz = tz.unwrap_or_else(|| "+00"); +fn parse_tz(tz: Option<&str>) -> Result { + let tz = tz.unwrap_or("+00"); Tz::from_str(tz) .map_err(|op| exec_datafusion_err!("failed to parse timezone {tz}: {:?}", op)) From f93cbf334acda3736b3816d00857b132816915ef Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 21:58:17 +0200 Subject: [PATCH 07/16] feat: enable clippy::debug_assert_with_mut_call `HashTable::find_entry` takes `&mut self`; `find` does the same lookup through `&self`. Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 1 + datafusion/common/src/utils/proxy.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index f49c87cc91b8c..078549f2387da 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -220,6 +220,7 @@ assigning_clones = "warn" char_lit_as_u8 = "warn" clear_with_drain = "warn" coerce_container_to_any = "warn" +debug_assert_with_mut_call = "warn" decimal_bitwise_operands = "warn" default_union_representation = "warn" empty_enum_variants_with_brackets = "warn" diff --git a/datafusion/common/src/utils/proxy.rs b/datafusion/common/src/utils/proxy.rs index 846c928515d60..7661cfa491878 100644 --- a/datafusion/common/src/utils/proxy.rs +++ b/datafusion/common/src/utils/proxy.rs @@ -166,7 +166,7 @@ where if cfg!(debug_assertions) { // In debug mode, check that the element is not already present debug_assert!( - self.find_entry(hash, |y| y == &x).is_err(), + self.find(hash, |y| y == &x).is_none(), "attempted to insert duplicate element into HashTableAllocExt::insert_accounted" ); } From 0743b1d7cfc0b01bf9f5f07d24879499d6166d76 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 21:59:59 +0200 Subject: [PATCH 08/16] feat: enable clippy::str_split_at_newline Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 1 + datafusion/core/tests/physical_optimizer/pushdown_utils.rs | 2 +- datafusion/core/tests/physical_optimizer/test_utils.rs | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 078549f2387da..dfd5d8b81957a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -264,6 +264,7 @@ pub_underscore_fields = "warn" rc_mutex = "warn" ref_option_ref = "warn" same_length_and_capacity = "warn" +str_split_at_newline = "warn" string_add_assign = "warn" suspicious_command_arg_space = "warn" suspicious_xor_used_as_pow = "warn" diff --git a/datafusion/core/tests/physical_optimizer/pushdown_utils.rs b/datafusion/core/tests/physical_optimizer/pushdown_utils.rs index 2ffd1899b3c1d..27a1428a28597 100644 --- a/datafusion/core/tests/physical_optimizer/pushdown_utils.rs +++ b/datafusion/core/tests/physical_optimizer/pushdown_utils.rs @@ -417,7 +417,7 @@ pub fn format_execution_plan(plan: &Arc) -> Vec { } fn format_lines(s: &str) -> Vec { - s.trim().split('\n').map(|s| s.to_string()).collect() + s.trim().lines().map(|s| s.to_string()).collect() } pub fn format_plan_for_test(plan: &Arc) -> String { diff --git a/datafusion/core/tests/physical_optimizer/test_utils.rs b/datafusion/core/tests/physical_optimizer/test_utils.rs index 74230b24e2ab5..097251878949a 100644 --- a/datafusion/core/tests/physical_optimizer/test_utils.rs +++ b/datafusion/core/tests/physical_optimizer/test_utils.rs @@ -801,7 +801,7 @@ pub fn format_execution_plan(plan: &Arc) -> Vec { } fn format_lines(s: &str) -> Vec { - s.trim().split('\n').map(|s| s.to_string()).collect() + s.trim().lines().map(|s| s.to_string()).collect() } /// Create a simple ProjectionExec with column indices (simplified version) From 687fe8c4b8a0d647fbfaab102169a15a169d9e08 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 22:02:28 +0200 Subject: [PATCH 09/16] feat: enable clippy::flat_map_option Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 1 + datafusion/common/src/cse.rs | 2 +- datafusion/expr/src/logical_plan/builder.rs | 2 +- datafusion/physical-expr/src/equivalence/properties/mod.rs | 2 +- datafusion/physical-plan/src/aggregates/mod.rs | 2 +- 5 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index dfd5d8b81957a..ae8888465cb23 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -226,6 +226,7 @@ default_union_representation = "warn" empty_enum_variants_with_brackets = "warn" empty_line_after_outer_attr = "warn" exit = "warn" +flat_map_option = "warn" fn_to_numeric_cast_any = "warn" if_let_mutex = "warn" imprecise_flops = "warn" diff --git a/datafusion/common/src/cse.rs b/datafusion/common/src/cse.rs index 93169d6a02ff1..64dad4f5c038d 100644 --- a/datafusion/common/src/cse.rs +++ b/datafusion/common/src/cse.rs @@ -808,7 +808,7 @@ mod test { ) -> HashSet { id_array .iter_mut() - .flat_map(|(_, id_option)| { + .filter_map(|(_, id_option)| { id_option.as_mut().map(|node_id| { let hash = node_id.hash; node_id.hash = 0; diff --git a/datafusion/expr/src/logical_plan/builder.rs b/datafusion/expr/src/logical_plan/builder.rs index 1f32d9c6da445..6b6ea9c1c72d4 100644 --- a/datafusion/expr/src/logical_plan/builder.rs +++ b/datafusion/expr/src/logical_plan/builder.rs @@ -2177,7 +2177,7 @@ pub fn wrap_projection_for_join_if_necessary( // Expr contains Arc with interior mutability but is intentionally used as hash key let join_key_items = alias_join_keys .iter() - .flat_map(|expr| expr.try_as_col().is_none().then_some(expr)) + .filter(|expr| expr.try_as_col().is_none()) .cloned() .collect::>(); projection.extend(join_key_items); diff --git a/datafusion/physical-expr/src/equivalence/properties/mod.rs b/datafusion/physical-expr/src/equivalence/properties/mod.rs index 22b3382f50638..54269e07f9309 100644 --- a/datafusion/physical-expr/src/equivalence/properties/mod.rs +++ b/datafusion/physical-expr/src/equivalence/properties/mod.rs @@ -1157,7 +1157,7 @@ impl EquivalenceProperties { let indices = mapping .iter() .flat_map(|(_, targets)| { - targets.iter().flat_map(|(target, _)| { + targets.iter().filter_map(|(target, _)| { target.downcast_ref::().map(|c| c.index()) }) }) diff --git a/datafusion/physical-plan/src/aggregates/mod.rs b/datafusion/physical-plan/src/aggregates/mod.rs index 33860d3f51c0b..5aed51c2e2ebc 100644 --- a/datafusion/physical-plan/src/aggregates/mod.rs +++ b/datafusion/physical-plan/src/aggregates/mod.rs @@ -1379,7 +1379,7 @@ impl AggregateExec { group_expr_mapping .iter() .flat_map(|(_, target_cols)| { - target_cols.iter().flat_map(|(expr, _)| { + target_cols.iter().filter_map(|(expr, _)| { expr.downcast_ref::().map(|c| c.index()) }) }) From 935177b817c59d7697310d651373102112daff6f Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 22:05:35 +0200 Subject: [PATCH 10/16] feat: enable clippy::verbose_file_reads Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 1 + datafusion/core/benches/parquet_query_sql.rs | 6 +--- datafusion/datasource-arrow/src/source.rs | 30 ++++++-------------- 3 files changed, 11 insertions(+), 26 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index ae8888465cb23..04e3aa1a1a6e6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -279,6 +279,7 @@ unused_async = "warn" unused_rounding = "warn" used_underscore_binding = "warn" useless_transmute = "warn" +verbose_file_reads = "warn" wildcard_dependencies = "warn" zero_sized_map_values = "warn" diff --git a/datafusion/core/benches/parquet_query_sql.rs b/datafusion/core/benches/parquet_query_sql.rs index 2e7794bfd19b4..9ed261839212f 100644 --- a/datafusion/core/benches/parquet_query_sql.rs +++ b/datafusion/core/benches/parquet_query_sql.rs @@ -32,8 +32,6 @@ use parquet::file::properties::{WriterProperties, WriterVersion}; use rand::distr::Alphanumeric; use rand::distr::uniform::SampleUniform; use rand::prelude::*; -use std::fs::File; -use std::io::Read; use std::ops::Range; use std::path::Path; use std::sync::Arc; @@ -211,9 +209,7 @@ fn criterion_benchmark(c: &mut Criterion) { .unwrap(); // We read the queries from a file so they can be changed without recompiling the benchmark - let mut queries_file = File::open("benches/parquet_query_sql.sql").unwrap(); - let mut queries = String::new(); - queries_file.read_to_string(&mut queries).unwrap(); + let queries = std::fs::read_to_string("benches/parquet_query_sql.sql").unwrap(); for query in queries.split(';') { let query = query.trim(); diff --git a/datafusion/datasource-arrow/src/source.rs b/datafusion/datasource-arrow/src/source.rs index 27533052ce03f..b3a8092c05328 100644 --- a/datafusion/datasource-arrow/src/source.rs +++ b/datafusion/datasource-arrow/src/source.rs @@ -444,7 +444,7 @@ impl From for Arc { #[cfg(test)] mod tests { - use std::{fs::File, io::Read}; + use std::fs::File; use arrow::datatypes::{DataType, Field, Schema}; use arrow_ipc::reader::{FileReader, StreamReader}; @@ -460,11 +460,8 @@ mod tests { for filename in ["example.arrow", "example_stream.arrow"] { let path = format!("tests/data/{filename}"); let path_str = path.as_str(); - let mut file = File::open(path_str)?; - let file_size = file.metadata()?.len(); - - let mut buffer = Vec::new(); - file.read_to_end(&mut buffer)?; + let buffer = std::fs::read(path_str)?; + let file_size = buffer.len() as u64; let bytes = Bytes::from(buffer); let object_store = Arc::new(InMemory::new()); @@ -504,11 +501,8 @@ mod tests { let filename = "example.arrow"; let path = format!("tests/data/{filename}"); let path_str = path.as_str(); - let mut file = File::open(path_str)?; - let file_size = file.metadata()?.len(); - - let mut buffer = Vec::new(); - file.read_to_end(&mut buffer)?; + let buffer = std::fs::read(path_str)?; + let file_size = buffer.len() as u64; let bytes = Bytes::from(buffer); let object_store = Arc::new(InMemory::new()); @@ -545,11 +539,8 @@ mod tests { let filename = "example_stream.arrow"; let path = format!("tests/data/{filename}"); let path_str = path.as_str(); - let mut file = File::open(path_str)?; - let file_size = file.metadata()?.len(); - - let mut buffer = Vec::new(); - file.read_to_end(&mut buffer)?; + let buffer = std::fs::read(path_str)?; + let file_size = buffer.len() as u64; let bytes = Bytes::from(buffer); let object_store = Arc::new(InMemory::new()); @@ -610,11 +601,8 @@ mod tests { let filename = "example_stream.arrow"; let path = format!("tests/data/{filename}"); let path_str = path.as_str(); - let mut file = File::open(path_str)?; - let file_size = file.metadata()?.len(); - - let mut buffer = Vec::new(); - file.read_to_end(&mut buffer)?; + let buffer = std::fs::read(path_str)?; + let file_size = buffer.len() as u64; let bytes = Bytes::from(buffer); let object_store = Arc::new(InMemory::new()); From f1f5d8ee3347ebe7980a3d558328b9cbc2524a22 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 22:08:44 +0200 Subject: [PATCH 11/16] feat: enable clippy::doc_include_without_cfg Gating the `include_str!` docs on `cfg(doc)` means touching the included markdown no longer forces a rebuild of the crate for non-doc builds. Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 1 + datafusion-cli/src/lib.rs | 2 +- datafusion/core/src/lib.rs | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 04e3aa1a1a6e6..9e6dbe617b0a4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -223,6 +223,7 @@ coerce_container_to_any = "warn" debug_assert_with_mut_call = "warn" decimal_bitwise_operands = "warn" default_union_representation = "warn" +doc_include_without_cfg = "warn" empty_enum_variants_with_brackets = "warn" empty_line_after_outer_attr = "warn" exit = "warn" diff --git a/datafusion-cli/src/lib.rs b/datafusion-cli/src/lib.rs index f0b0bc23fd73d..51202818d3c01 100644 --- a/datafusion-cli/src/lib.rs +++ b/datafusion-cli/src/lib.rs @@ -20,7 +20,7 @@ html_favicon_url = "https://raw.githubusercontent.com/apache/datafusion/19fe44cf2f30cbdd63d4a4f52c74055163c6cc38/docs/logos/standalone_logo/logo_original.svg" )] #![cfg_attr(docsrs, feature(doc_cfg))] -#![doc = include_str!("../README.md")] +#![cfg_attr(doc, doc = include_str!("../README.md"))] pub const DATAFUSION_CLI_VERSION: &str = env!("CARGO_PKG_VERSION"); pub mod catalog; diff --git a/datafusion/core/src/lib.rs b/datafusion/core/src/lib.rs index 3170f4be7f683..28ea4b4490c3e 100644 --- a/datafusion/core/src/lib.rs +++ b/datafusion/core/src/lib.rs @@ -761,7 +761,7 @@ //! [`RecordBatch`]: arrow::array::RecordBatch //! [`RecordBatchReader`]: arrow::record_batch::RecordBatchReader //! [`Array`]: arrow::array::Array -#![doc = include_str!("optimizer_rule_reference.md")] +#![cfg_attr(doc, doc = include_str!("optimizer_rule_reference.md"))] extern crate core; #[cfg(feature = "sql")] From 46563cc25622440f86fd93180ffc35ca00cb2a72 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 22:15:48 +0200 Subject: [PATCH 12/16] feat: enable clippy::unnecessary_safety_comment `SAFETY:` comments should sit next to the `unsafe` block they justify. The existing ones either documented safe code (reworded) or were attached to the enclosing `if` rather than the `unsafe` block (moved). Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 1 + datafusion/datasource-json/src/source.rs | 2 +- datafusion/functions-aggregate/src/percentile_cont.rs | 2 +- datafusion/functions/src/string/repeat.rs | 2 +- datafusion/functions/src/unicode/character_length.rs | 2 +- datafusion/optimizer/src/extract_equijoin_predicate.rs | 2 +- datafusion/physical-expr-common/src/binary_map.rs | 2 +- datafusion/physical-plan/src/sorts/cursor.rs | 6 +++--- datafusion/spark/src/function/string/length.rs | 2 +- 9 files changed, 11 insertions(+), 10 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 9e6dbe617b0a4..0a59b0f54b729 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -275,6 +275,7 @@ transmute_ptr_to_ptr = "warn" uninhabited_references = "warn" uninlined_format_args = "warn" unnecessary_lazy_evaluations = "warn" +unnecessary_safety_comment = "warn" unnecessary_self_imports = "warn" unused_async = "warn" unused_rounding = "warn" diff --git a/datafusion/datasource-json/src/source.rs b/datafusion/datasource-json/src/source.rs index 8632d6b942bc1..ee39858e873e1 100644 --- a/datafusion/datasource-json/src/source.rs +++ b/datafusion/datasource-json/src/source.rs @@ -65,7 +65,7 @@ const JSON_CONVERTER_BUFFER_SIZE: usize = 2 * 1024 * 1024; /// A stream wrapper that holds SpawnedTask handles to keep them alive /// until the stream is fully consumed or dropped. /// -/// This ensures cancel-safety: when the stream is dropped, the tasks +/// This makes the stream cancel-safe: when the stream is dropped, the tasks /// are properly aborted via SpawnedTask's Drop implementation. struct JsonArrayStream { inner: ReceiverStream>, diff --git a/datafusion/functions-aggregate/src/percentile_cont.rs b/datafusion/functions-aggregate/src/percentile_cont.rs index 3a98900bbb446..8052247586df7 100644 --- a/datafusion/functions-aggregate/src/percentile_cont.rs +++ b/datafusion/functions-aggregate/src/percentile_cont.rs @@ -63,7 +63,7 @@ use crate::utils::validate_percentile_expr; /// Precision multiplier for linear interpolation calculations. /// -/// This value of 1,000,000 was chosen to balance precision with overflow safety: +/// This value of 1,000,000 was chosen to balance precision against overflow risk: /// - Provides 6 decimal places of precision for the fractional component /// - Small enough to avoid overflow when multiplied with typical numeric values /// - Sufficient precision for most statistical applications diff --git a/datafusion/functions/src/string/repeat.rs b/datafusion/functions/src/string/repeat.rs index a53f1e2e4fc42..09ecfd00168eb 100644 --- a/datafusion/functions/src/string/repeat.rs +++ b/datafusion/functions/src/string/repeat.rs @@ -306,7 +306,7 @@ where // Doubling strategy: copy what we have so far until we reach the target while buffer.len() < src.len() * count { let copy_len = buffer.len().min(src.len() * count - buffer.len()); - // SAFETY: we're copying valid UTF-8 bytes that we already verified + // We are copying valid UTF-8 bytes that we already verified buffer.extend_from_within(..copy_len); } } diff --git a/datafusion/functions/src/unicode/character_length.rs b/datafusion/functions/src/unicode/character_length.rs index 9f0d952a02636..2b99d942f6c0a 100644 --- a/datafusion/functions/src/unicode/character_length.rs +++ b/datafusion/functions/src/unicode/character_length.rs @@ -158,10 +158,10 @@ where } else { let values: Vec<_> = (0..array.len()) .map(|i| { - // Safety: we are iterating with array.len() so the index is always valid if array.is_null(i) { T::default_value() } else { + // Safety: we are iterating with array.len() so the index is always valid let value = unsafe { array.value_unchecked(i) }; if value.is_empty() { T::default_value() diff --git a/datafusion/optimizer/src/extract_equijoin_predicate.rs b/datafusion/optimizer/src/extract_equijoin_predicate.rs index 0a50761e8a9f7..58f9a4cd42a2d 100644 --- a/datafusion/optimizer/src/extract_equijoin_predicate.rs +++ b/datafusion/optimizer/src/extract_equijoin_predicate.rs @@ -95,7 +95,7 @@ impl OptimizerRule for ExtractEquijoinPredicate { && equijoin_predicates.is_empty() && non_equijoin_expr.is_some() { - // SAFETY: checked in the outer `if` + // Checked in the outer `if` let expr = non_equijoin_expr.clone().unwrap(); let (equijoin_predicates, non_equijoin_expr) = split_is_not_distinct_from_and_other_join_predicate( diff --git a/datafusion/physical-expr-common/src/binary_map.rs b/datafusion/physical-expr-common/src/binary_map.rs index ad184d6500d56..e5b77f10d535d 100644 --- a/datafusion/physical-expr-common/src/binary_map.rs +++ b/datafusion/physical-expr-common/src/binary_map.rs @@ -557,7 +557,7 @@ fn single_null_buffer(num_values: usize, null_index: usize) -> NullBuffer { null_builder.append_n_non_nulls(null_index); null_builder.append_null(); null_builder.append_n_non_nulls(num_values - null_index - 1); - // SAFETY: inner builder must be constructed + // The appends above guarantee the inner builder has been constructed. null_builder.finish().unwrap() } diff --git a/datafusion/physical-plan/src/sorts/cursor.rs b/datafusion/physical-plan/src/sorts/cursor.rs index d71eaad663410..f23c242a36285 100644 --- a/datafusion/physical-plan/src/sorts/cursor.rs +++ b/datafusion/physical-plan/src/sorts/cursor.rs @@ -410,10 +410,10 @@ impl CursorValues for StringViewArray { #[inline(always)] fn compare(l: &Self, l_idx: usize, r: &Self, r_idx: usize) -> Ordering { - // SAFETY: Prior assertions guarantee that l_idx and r_idx are valid indices. - // Null-checks are assumed to have been handled in the wrapper (e.g., ArrayValues). - // And the bound is checked in is_finished, it is safe to call get_unchecked if l.data_buffers().is_empty() && r.data_buffers().is_empty() { + // SAFETY: Prior assertions guarantee that l_idx and r_idx are valid indices. + // Null-checks are assumed to have been handled in the wrapper (e.g., ArrayValues). + // And the bound is checked in is_finished, it is safe to call get_unchecked let l_view = unsafe { l.views().get_unchecked(l_idx) }; let r_view = unsafe { r.views().get_unchecked(r_idx) }; return StringViewArray::inline_key_fast(*l_view) diff --git a/datafusion/spark/src/function/string/length.rs b/datafusion/spark/src/function/string/length.rs index 8c5539a0577d8..b3683dac5b83e 100644 --- a/datafusion/spark/src/function/string/length.rs +++ b/datafusion/spark/src/function/string/length.rs @@ -154,10 +154,10 @@ where } else { let values: Vec<_> = (0..array.len()) .map(|i| { - // Safety: we are iterating with array.len() so the index is always valid if array.is_null(i) { i32::default() } else { + // Safety: we are iterating with array.len() so the index is always valid let value = unsafe { array.value_unchecked(i) }; if value.is_empty() { i32::default() From 0f652dcb7fa8d8e67f71e04ac9b527950f767000 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 22:22:17 +0200 Subject: [PATCH 13/16] feat: add a [workspace.lints.rust] set Enables the rustc lint groups and individual lints from https://github.com/emilk/egui/blob/main/Cargo.toml that DataFusion violates at most 10 times today: * `future_incompatible`, `nonstandard_style`, `rust_2018_idioms` (groups) * `rust_2021_prelude_collisions`, `semicolon_in_expressions_from_macros`, `unsafe_op_in_unsafe_fn`, `unused_extern_crates`, `unused_import_braces`, `unused_lifetimes` `elided_lifetimes_in_paths` is part of `rust_2018_idioms` but has ~800 violations, so it is explicitly allowed for now. `trivial_numeric_casts` (31 violations) and `unsafe_code` are left out entirely. The violations fixed here are vestigial `extern crate` items, redundant import braces, and two `rstest` helpers whose lifetime is unused after macro expansion. Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 13 +++++++++++++ datafusion-cli/tests/cli_integration.rs | 4 ++++ datafusion/common/src/rounding.rs | 6 ------ datafusion/core/src/lib.rs | 3 --- .../tests/physical_optimizer/enforce_sorting.rs | 4 ++-- datafusion/expr/src/lib.rs | 2 -- datafusion/functions/benches/atan2.rs | 2 -- datafusion/functions/benches/get_field.rs | 2 -- datafusion/functions/benches/nanvl.rs | 2 -- datafusion/functions/benches/power.rs | 2 -- datafusion/macros/src/user_doc.rs | 1 - datafusion/wasmtest/src/lib.rs | 2 -- 12 files changed, 19 insertions(+), 24 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 0a59b0f54b729..6d41ecc354ab7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -285,12 +285,25 @@ verbose_file_reads = "warn" wildcard_dependencies = "warn" zero_sized_map_values = "warn" +# Keep this list sorted alphabetically. [workspace.lints.rust] +# Part of the `rust_2018_idioms` group, but ~800 violations today: +# https://github.com/apache/datafusion/issues/18467 +elided_lifetimes_in_paths = "allow" +future_incompatible = { level = "warn", priority = -1 } +nonstandard_style = { level = "warn", priority = -1 } +rust_2018_idioms = { level = "warn", priority = -1 } +rust_2021_prelude_collisions = "warn" +semicolon_in_expressions_from_macros = "warn" unexpected_cfgs = { level = "warn", check-cfg = [ 'cfg(datafusion_coop, values("tokio", "tokio_fallback", "per_stream"))', "cfg(coverage)", "cfg(coverage_nightly)", ] } +unsafe_op_in_unsafe_fn = "warn" +unused_extern_crates = "warn" +unused_import_braces = "warn" +unused_lifetimes = "warn" unused_qualifications = "deny" # -------------------- diff --git a/datafusion-cli/tests/cli_integration.rs b/datafusion-cli/tests/cli_integration.rs index 4dc244445a2eb..9b33c1a243711 100644 --- a/datafusion-cli/tests/cli_integration.rs +++ b/datafusion-cli/tests/cli_integration.rs @@ -428,6 +428,8 @@ fn test_cli_format<'a>(#[case] format: &'a str) { #[case("top2", ["--top-memory-consumers", "2"])] #[case("top3_default", [])] #[test] +// `'a` is used by the signature below, but not by the per-case functions `rstest` generates. +#[expect(unused_lifetimes)] fn test_cli_top_memory_consumers<'a>( #[case] snapshot_name: &str, #[case] top_memory_consumers: impl IntoIterator, @@ -446,6 +448,8 @@ fn test_cli_top_memory_consumers<'a>( #[case("no_track", ["--top-memory-consumers", "0"])] #[case("top2", ["--top-memory-consumers", "2"])] #[test] +// `'a` is used by the signature below, but not by the per-case functions `rstest` generates. +#[expect(unused_lifetimes)] fn test_cli_top_memory_consumers_with_mem_pool_type<'a>( #[case] snapshot_name: &str, #[case] top_memory_consumers: impl IntoIterator, diff --git a/datafusion/common/src/rounding.rs b/datafusion/common/src/rounding.rs index 1796143d7cf1a..7da3bd0c6ac07 100644 --- a/datafusion/common/src/rounding.rs +++ b/datafusion/common/src/rounding.rs @@ -37,12 +37,6 @@ const FE_UPWARD: i32 = 0x0800; #[cfg(all(target_arch = "x86_64", not(target_os = "windows")))] const FE_DOWNWARD: i32 = 0x0400; -#[cfg(all( - any(target_arch = "x86_64", target_arch = "aarch64"), - not(target_os = "windows") -))] -extern crate libc; - #[cfg(all( any(target_arch = "x86_64", target_arch = "aarch64"), not(target_os = "windows") diff --git a/datafusion/core/src/lib.rs b/datafusion/core/src/lib.rs index 28ea4b4490c3e..a3d8bff5ccada 100644 --- a/datafusion/core/src/lib.rs +++ b/datafusion/core/src/lib.rs @@ -763,9 +763,6 @@ //! [`Array`]: arrow::array::Array #![cfg_attr(doc, doc = include_str!("optimizer_rule_reference.md"))] -extern crate core; -#[cfg(feature = "sql")] -extern crate sqlparser; /// DataFusion crate version pub const DATAFUSION_VERSION: &str = env!("CARGO_PKG_VERSION"); diff --git a/datafusion/core/tests/physical_optimizer/enforce_sorting.rs b/datafusion/core/tests/physical_optimizer/enforce_sorting.rs index d94253a84aa5f..dd67e4895b1ac 100644 --- a/datafusion/core/tests/physical_optimizer/enforce_sorting.rs +++ b/datafusion/core/tests/physical_optimizer/enforce_sorting.rs @@ -29,7 +29,7 @@ use crate::physical_optimizer::test_utils::{ spr_repartition_exec, stream_exec_ordered, union_exec, }; -use arrow::compute::{SortOptions}; +use arrow::compute::SortOptions; use arrow::datatypes::{DataType, SchemaRef}; use datafusion_common::config::{ConfigOptions, CsvOptions}; use datafusion_common::tree_node::{TreeNode, TransformedResult}; @@ -61,7 +61,7 @@ use datafusion_physical_optimizer::output_requirements::OutputRequirementExec; use datafusion_physical_optimizer::PhysicalOptimizerRule; use datafusion::prelude::*; use arrow::array::{record_batch, Array, ArrayRef, Int32Array, RecordBatch}; -use arrow::datatypes::{Field}; +use arrow::datatypes::Field; use arrow_schema::Schema; use datafusion_execution::TaskContext; use datafusion_catalog::streaming::StreamingTable; diff --git a/datafusion/expr/src/lib.rs b/datafusion/expr/src/lib.rs index 1033952642a2b..b8d30346025c1 100644 --- a/datafusion/expr/src/lib.rs +++ b/datafusion/expr/src/lib.rs @@ -35,8 +35,6 @@ //! //! The [expr_fn] module contains functions for creating expressions. -extern crate core; - mod higher_order_function; mod literal; mod operation; diff --git a/datafusion/functions/benches/atan2.rs b/datafusion/functions/benches/atan2.rs index f1c9756a0cc08..d1f2b3d1332ad 100644 --- a/datafusion/functions/benches/atan2.rs +++ b/datafusion/functions/benches/atan2.rs @@ -15,8 +15,6 @@ // specific language governing permissions and limitations // under the License. -extern crate criterion; - use arrow::datatypes::{DataType, Field, Float32Type, Float64Type}; use arrow::util::bench_util::create_primitive_array; use criterion::{Criterion, criterion_group, criterion_main}; diff --git a/datafusion/functions/benches/get_field.rs b/datafusion/functions/benches/get_field.rs index 8a5fd0a1e2fa9..a274bfb385d64 100644 --- a/datafusion/functions/benches/get_field.rs +++ b/datafusion/functions/benches/get_field.rs @@ -15,8 +15,6 @@ // specific language governing permissions and limitations // under the License. -extern crate criterion; - use arrow::array::{ArrayRef, Int32Builder, MapBuilder, StringBuilder}; use arrow::datatypes::{DataType, Field}; use criterion::{Criterion, criterion_group, criterion_main}; diff --git a/datafusion/functions/benches/nanvl.rs b/datafusion/functions/benches/nanvl.rs index d3d2c7ebff998..830a1ea3888ca 100644 --- a/datafusion/functions/benches/nanvl.rs +++ b/datafusion/functions/benches/nanvl.rs @@ -15,8 +15,6 @@ // specific language governing permissions and limitations // under the License. -extern crate criterion; - use arrow::array::{ArrayRef, Float32Array, Float64Array}; use arrow::datatypes::{DataType, Field}; use criterion::{Criterion, criterion_group, criterion_main}; diff --git a/datafusion/functions/benches/power.rs b/datafusion/functions/benches/power.rs index 5336e42ebe59b..d40f330d76c23 100644 --- a/datafusion/functions/benches/power.rs +++ b/datafusion/functions/benches/power.rs @@ -23,8 +23,6 @@ //! through a Float64 round-trip, which is measurably slower than the //! decimal kernel for the cases the kernel can handle. -extern crate criterion; - use arrow::array::{Decimal128Array, Int64Array}; use arrow::datatypes::{DataType, Field, FieldRef}; use criterion::{Criterion, criterion_group, criterion_main}; diff --git a/datafusion/macros/src/user_doc.rs b/datafusion/macros/src/user_doc.rs index ce9e7d55ef103..bbfe7877fc211 100644 --- a/datafusion/macros/src/user_doc.rs +++ b/datafusion/macros/src/user_doc.rs @@ -21,7 +21,6 @@ )] #![cfg_attr(docsrs, feature(doc_cfg))] -extern crate proc_macro; use datafusion_doc::scalar_doc_sections::doc_sections_const; use proc_macro::TokenStream; use quote::quote; diff --git a/datafusion/wasmtest/src/lib.rs b/datafusion/wasmtest/src/lib.rs index f545ccf19306a..6289a0c3956fb 100644 --- a/datafusion/wasmtest/src/lib.rs +++ b/datafusion/wasmtest/src/lib.rs @@ -22,8 +22,6 @@ )] #![cfg_attr(docsrs, feature(doc_cfg))] -extern crate wasm_bindgen; - use datafusion_common::ScalarValue; use datafusion_expr::lit; use datafusion_expr::simplify::SimplifyContext; From b46aa43ae2396c78758310a6903b3d69e1b79f1e Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 22:28:00 +0200 Subject: [PATCH 14/16] feat: add a [workspace.lints.rustdoc] set Enables the `rustdoc::all` group. Two of its lints have more than 10 violations today and are explicitly allowed for now: `missing_crate_level_docs` (18) and `unescaped_backticks` (28). Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index 6d41ecc354ab7..48eea0e020c9c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -306,6 +306,15 @@ unused_import_braces = "warn" unused_lifetimes = "warn" unused_qualifications = "deny" +# Keep this list sorted alphabetically. +[workspace.lints.rustdoc] +all = { level = "warn", priority = -1 } +broken_intra_doc_links = "warn" +# Part of the `all` group, but has too many violations today to enable: +# https://github.com/apache/datafusion/issues/18467 +missing_crate_level_docs = "allow" +unescaped_backticks = "allow" + # -------------------- # Compilation Profiles # -------------------- From 823aab6bca9551b636e7619e311df33af365a919 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 22:37:33 +0200 Subject: [PATCH 15/16] fix: keep `extern crate libc` in rounding.rs Removing it made `libc` an unused dependency (`cargo machete` CI failure): the crate has no path references, it is only linked so the `fesetround`/`fegetround` symbols declared in this module resolve. Co-Authored-By: Claude Opus 5 (1M context) --- datafusion/common/src/rounding.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/datafusion/common/src/rounding.rs b/datafusion/common/src/rounding.rs index 7da3bd0c6ac07..4b8ad8c41f7e4 100644 --- a/datafusion/common/src/rounding.rs +++ b/datafusion/common/src/rounding.rs @@ -37,6 +37,15 @@ const FE_UPWARD: i32 = 0x0800; #[cfg(all(target_arch = "x86_64", not(target_os = "windows")))] const FE_DOWNWARD: i32 = 0x0400; +// Links `libc`, which provides the `fesetround`/`fegetround` symbols declared below. +// There is no path reference to the crate, so `unused_extern_crates` cannot see the use. +#[expect(unused_extern_crates)] +#[cfg(all( + any(target_arch = "x86_64", target_arch = "aarch64"), + not(target_os = "windows") +))] +extern crate libc; + #[cfg(all( any(target_arch = "x86_64", target_arch = "aarch64"), not(target_os = "windows") From c15931167b328a865ad7befdfd08ec9d28acc2b3 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 22:45:20 +0200 Subject: [PATCH 16/16] chore: self-review fixups * Keep the `LogicalPlanType::` prefix on the `CustomScan` match arm so it matches its sibling arms; drop the variant import instead. * Restore a `SAFETY:` comment on the second `unsafe` block in `cursor.rs`. * Drop `rustdoc::broken_intra_doc_links`, already covered by `rustdoc::all`. Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 1 - datafusion/physical-plan/src/sorts/cursor.rs | 2 ++ datafusion/proto/src/logical_plan/mod.rs | 5 ++--- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 48eea0e020c9c..f1a80d7c19427 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -309,7 +309,6 @@ unused_qualifications = "deny" # Keep this list sorted alphabetically. [workspace.lints.rustdoc] all = { level = "warn", priority = -1 } -broken_intra_doc_links = "warn" # Part of the `all` group, but has too many violations today to enable: # https://github.com/apache/datafusion/issues/18467 missing_crate_level_docs = "allow" diff --git a/datafusion/physical-plan/src/sorts/cursor.rs b/datafusion/physical-plan/src/sorts/cursor.rs index f23c242a36285..f595ec6fcb06b 100644 --- a/datafusion/physical-plan/src/sorts/cursor.rs +++ b/datafusion/physical-plan/src/sorts/cursor.rs @@ -420,6 +420,8 @@ impl CursorValues for StringViewArray { .cmp(&StringViewArray::inline_key_fast(*r_view)); } + // SAFETY: Prior assertions guarantee that l_idx and r_idx are valid indices. + // Null-checks are assumed to have been handled in the wrapper (e.g., ArrayValues). unsafe { GenericByteViewArray::compare_unchecked(l, l_idx, r, r_idx) } } } diff --git a/datafusion/proto/src/logical_plan/mod.rs b/datafusion/proto/src/logical_plan/mod.rs index 653ae9ab05355..a900f86b2476e 100644 --- a/datafusion/proto/src/logical_plan/mod.rs +++ b/datafusion/proto/src/logical_plan/mod.rs @@ -20,7 +20,6 @@ use std::fmt::Debug; use std::sync::Arc; use crate::convert::{FromProto, TryFromProto}; -use crate::protobuf::logical_plan_node::LogicalPlanType::CustomScan; use crate::protobuf::{ ColumnUnnestListItem, ColumnUnnestListRecursion, CteWorkTableScanNode, CustomTableScanNode, DmlNode, SortExprNodeCollection, dml_node, @@ -680,7 +679,7 @@ impl AsLogicalPlan for LogicalPlanNode { )? .build() } - CustomScan(scan) => { + LogicalPlanType::CustomScan(scan) => { let schema: Schema = convert_required!(scan.schema)?; let schema = Arc::new(schema); let mut projection = None; @@ -1506,7 +1505,7 @@ impl AsLogicalPlan for LogicalPlanNode { extension_codec .try_encode_table_provider(table_name, provider, &mut bytes) .map_err(|e| context!("Error serializing custom table", e))?; - let scan = CustomScan(CustomTableScanNode { + let scan = LogicalPlanType::CustomScan(CustomTableScanNode { table_name: Some(protobuf::TableReference::from_proto( table_name.clone(), )),