fix: remove clippy warnings #3940

corwinjoy · 2025-11-19T02:49:55Z

Description

Set a flag to turn off the deprecation warnings. They clutter the error messages and we can turn this back on when we are ready to remove the deprecated functions.
Go through the remaining clippy warnings and either fix them or disable them if it would mean changing the interface.

Related Issue(s)

Closes #3907

Signed-off-by: Corwin Joy <[email protected]>

…ed_imports Signed-off-by: Corwin Joy <[email protected]>

…ippy::needless_borrow) Signed-off-by: Corwin Joy <[email protected]>

…mark default variants (clippy::derivable_impls) Signed-off-by: Corwin Joy <[email protected]>

Signed-off-by: Corwin Joy <[email protected]>

… on IntoIterator (clippy::unnecessary_to_owned, clippy::cloned_ref_to_slice_refs) Signed-off-by: Corwin Joy <[email protected]>

…oid field reassignment (clippy::field_reassign_with_default) Signed-off-by: Corwin Joy <[email protected]>

…borrow) Signed-off-by: Corwin Joy <[email protected]>

…ess_return) Signed-off-by: Corwin Joy <[email protected]>

…(declare_interior_mutable_const) Signed-off-by: Corwin Joy <[email protected]>

… multiple_bound_locations (clippy) Signed-off-by: Corwin Joy <[email protected]>

…orrow) Signed-off-by: Corwin Joy <[email protected]>

Signed-off-by: Corwin Joy <[email protected]>

Remove module_inception warning for schema. Signed-off-by: Corwin Joy <[email protected]>

Signed-off-by: Corwin Joy <[email protected]>

github-actions · 2025-11-19T02:50:17Z

ACTION NEEDED

delta-rs follows the Conventional Commits specification for release automation.

The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification.

corwinjoy · 2025-11-19T02:53:04Z

crates/core/src/kernel/schema/mod.rs


 pub mod cast;
 pub mod partitions;
+#[allow(clippy::module_inception)]


Clippy does not like the repeated name per https://rust-lang.github.io/rust-clippy/rust-1.91.0/index.html#module_inception

But, changing this could break API usage so I just disabled the warning.

codecov · 2025-11-20T13:59:49Z

Codecov Report

❌ Patch coverage is 22.22222% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 26.23%. Comparing base (948396f) to head (78c909e).

Files with missing lines	Patch %	Lines
crates/core/src/operations/constraints.rs	0.00%	5 Missing ⚠️
crates/core/src/kernel/snapshot/mod.rs	0.00%	4 Missing ⚠️
crates/core/src/operations/write/execution.rs	0.00%	2 Missing ⚠️
crates/core/src/delta_datafusion/planner.rs	0.00%	1 Missing ⚠️
crates/core/src/kernel/snapshot/log_data.rs	50.00%	1 Missing ⚠️
crates/core/src/operations/vacuum.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3940      +/-   ##
==========================================
- Coverage   26.24%   26.23%   -0.02%     
==========================================
  Files         124      124              
  Lines       19824    19819       -5     
  Branches    19824    19819       -5     
==========================================
- Hits         5203     5199       -4     
+ Misses      14251    14250       -1     
  Partials      370      370

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

hntd187 · 2025-11-20T17:14:39Z

I'm not generally a fan of this. I think we should fix the deprecations rather than just silence the warnings. That's the entire reason they exist in the first place. Additionally, why are there changes in this PR entirely unrelated to the clippy warnings?

corwinjoy · 2025-11-20T21:42:02Z

@hntd187 Here is my reasoning on the deprecation flag.

Many of these deprecation warnings are similar to

warning: use of deprecated method `kernel::snapshot::EagerSnapshot::file_views_by_partitions`: Use `files` with kernel predicate instead.
    --> crates/core/src/operations/optimize.rs:1057:36
     |
1057 |     let mut file_stream = snapshot.file_views_by_partitions(log_store,...

I think these came about with the new kernel, but I'm not sure this update has been finished yet, so a new method may not exist.
2. There are so many deprecation warnings that it really clutters the warning space. Making it very hard to spot any new warnings when developing on this project. What I have seen over the last month or so is that the number of clippy warnings is growing, since I think folks have just started to ignore this.
3. Adding a deprecation flag helps to break this PR into more manageable pieces. When we are ready to address the deprecations (not sure if this is possible now due to the kernel stuff), we can turn these back on.

Additionally, why are there changes in this PR entirely unrelated to the clippy warnings?

In terms of your other question, as far as I can tell every one of these changes should be associated with a clippy warning message. I am running a double check to confirm. But, maybe I missed something? If so, could you please point out the incorrect change?

corwinjoy · 2025-11-20T21:43:17Z

@adamreeve @abhiaagarwal

corwinjoy · 2025-11-20T21:56:48Z

crates/benchmarks/src/main.rs

 async fn run_merge_case(case: &MergeTestCase, parquet_dir: &Path) -> anyhow::Result<()> {
    let tmp_dir = tempfile::tempdir()?;
-    let (source, table) = prepare_source_and_table(&case.params, &tmp_dir, &parquet_dir).await?;
+    let (source, table) = prepare_source_and_table(&case.params, &tmp_dir, parquet_dir).await?;


Crate: crates/benchmarks File: crates/benchmarks/src/main.rs (run_merge_case) Clippy warning fixed: "this expression creates a reference which is immediately dereferenced by the compiler --> crates/benchmarks/src/main.rs:200:76 help: change this to: parquet_dir" Change: removed the needless borrow and passed parquet_dir directly to prepare_source_and_table.

corwinjoy · 2025-11-20T21:57:12Z

crates/core/src/delta_datafusion/planner.rs

 use crate::operations::write::metrics::WriteMetricExtensionPlanner;

-const DELTA_EXTENSION_PLANNERS: LazyLock<Vec<Arc<dyn ExtensionPlanner + Send + Sync>>> =
+static DELTA_EXTENSION_PLANNERS: LazyLock<Vec<Arc<dyn ExtensionPlanner + Send + Sync>>> =


Crate: crates/core File: crates/core/src/delta_datafusion/planner.rs Clippy warnings fixed: "named constant with interior mutability --> crates/core/src/delta_datafusion/planner.rs:42:7 = help: did you mean to make this a static item" and "named constant with interior mutability --> crates/core/src/delta_datafusion/planner.rs:52:7" Change: converted the LazyLock const items to static (e.g., static DELTA_EXTENSION_PLANNERS and static DELTA_PLANNER) so interior mutability is handled correctly.

Where is this modified such that interior mutability is relevant here?

Here is the full warning message. I tried just assigning to a local variable but the fix is a bit trickier than that I guess.

warning: borrow of a named constant with interior mutability --> crates/core/src/delta_datafusion/planner.rs:98:28 | 98 | for ext_planner in DELTA_EXTENSION_PLANNERS.iter() { | ^^^^^^^^^^^^^^^^^^^^^^^^ | = note: there is a compiler inserted call to `Deref::deref` here = help: this lint can be silenced by assigning the value to a local variable before borrowing = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.91.0/index.html#borrow_interior_mutable_const = note: `#[warn(clippy::borrow_interior_mutable_const)]` on by default

@hntd187 I could use some help on a better way to fix this error since simple assignment to a temporary doesn't quite do it. Here are the docs on this
https://rust-lang.github.io/rust-clippy/rust-1.91.0/index.html#borrow_interior_mutable_const
So, potentially, we could mark this to be ignored but that doesn't seem too great.

My point was I don't believe this lint is correct. We don't modify this value here, or anywhere I'm aware of. Perhaps just

let planners = DELTA_EXTENSION_PLANNERS.clone(); for planner in planners.iter() { ... }

might work but I'd have to look into it myself.

Hmm. That also doesn't work. But, I think this should actually be a static here because of the use of LazyLock.
const here means a compile-time constant. (See: https://stackoverflow.com/questions/37405835/populating-a-static-const-with-an-environment-variable-at-runtime-in-rust). But LazyLock is something different; it is actually initialized on first usage.
https://doc.rust-lang.org/std/sync/struct.LazyLock.html

corwinjoy · 2025-11-20T21:58:00Z

crates/core/src/kernel/arrow/engine_ext.rs

    Ok(ScanMetadataArrow {
        scan_files,
-        scan_file_transforms,
+        _scan_file_transforms: scan_file_transforms,


Crate: crates/core File: crates/core/src/kernel/arrow/engine_ext.rs (ScanMetadataArrow) Clippy warning fixed: "field scan_file_transforms is never read --> crates/core/src/kernel/arrow/engine_ext.rs:44:5" Change: renamed the field to _scan_file_transforms and updated construction to _scan_file_transforms: scan_file_transforms to mark it intentionally unused.

corwinjoy · 2025-11-20T21:59:11Z

crates/core/src/kernel/models/actions.rs

-    fn default() -> Self {
-        Self::Serializable
-    }
-}


Crate: crates/core File: crates/core/src/kernel/models/actions.rs Clippy warnings fixed: "associated function new is never used --> crates/core/src/kernel/models/actions.rs:271:19" and "this impl can be derived --> crates/core/src/kernel/models/actions.rs:706:1" Change: added #[allow(dead_code)] to the unused ProtocolInner::new method; replaced manual impl Default with #[derive(Default)] and marked default variants with #[default] for StorageType and IsolationLevel per clippy suggestions.

corwinjoy · 2025-11-20T22:00:21Z

crates/core/src/kernel/snapshot/log_data.rs

                if let Some(mut accumulator) = accumulator {
                    return accumulator
-                        .update_batch(&[array.clone()])
+                        .update_batch(std::slice::from_ref(array))


Crate: crates/core File: crates/core/src/kernel/snapshot/log_data.rs Clippy warnings fixed: "unnecessary use of to_vec --> crates/core/src/kernel/snapshot/log_data.rs:102:18 help: use: self.data.iter().cloned()" and "this call to clone can be replaced with std::slice::from_ref --> crates/core/src/kernel/snapshot/log_data.rs:214:39 help: try: std::slice::from_ref(array)" Change: updated IntoIterator signature to carry the lifetime (impl<'a> IntoIterator for LogDataHandler<'a> and type IntoIter = Box<dyn Iterator<Item = Self::Item> + 'a>), replaced .to_vec().into_iter() with self.data.iter().cloned(), and replaced .update_batch(&[array.clone()]) with .update_batch(std::slice::from_ref(array)).

corwinjoy · 2025-11-20T22:00:42Z

crates/core/src/kernel/snapshot/mod.rs

+        let config = DeltaTableConfig {
+            require_files,
+            ..Default::default()
+        };


Crate: crates/core File: crates/core/src/kernel/snapshot/mod.rs (resolve_snapshot) Clippy warning fixed: "field assignment outside of initializer for an instance created with Default::default() --> crates/core/src/kernel/snapshot/mod.rs:483:9 note: consider initializing the variable with DeltaTableConfig { require_files: require_files, ..Default::default() }" Change: constructed config with struct update syntax: let config = DeltaTableConfig { require_files, ..Default::default() }; instead of creating default and reassigning a field.

corwinjoy · 2025-11-20T22:01:04Z

crates/core/src/kernel/snapshot/stream.rs


    /// Spawn task that will be aborted if this builder (or the stream
    /// built from it) are dropped
+    #[allow(dead_code)]


Crate: crates/core File: crates/core/src/kernel/snapshot/stream.rs (ReceiverStreamBuilder) Clippy warning fixed: "method spawn is never used --> crates/core/src/kernel/snapshot/stream.rs:92:12" Change: added #[allow(dead_code)] to the unused spawn method to silence the dead-code lint.

Not changed since I don't want to potentially break the API.

corwinjoy · 2025-11-20T22:01:21Z

crates/core/src/logstore/config.rs

    K: AsRef<str> + Into<String>,
    V: AsRef<str> + Into<String>,
-    T: TryUpdateKey,
+    T: TryUpdateKey + std::fmt::Debug,


Crate: crates/core File: crates/core/src/logstore/config.rs (try_parse_impl signature) Clippy warning fixed: "bound is defined in more than one place --> crates/core/src/logstore/config.rs:239:30 = help: for further information visit https://rust-lang.github.io/rust-clippy/...#multiple_bound_locations" Change: consolidated type bounds by moving std::fmt::Debug into the where-clause so T bounds are defined in one place: pub(super) fn try_parse_impl<T, K, V, I>(...) -> ... where ... T: TryUpdateKey + std::fmt::Debug, ....

corwinjoy · 2025-11-20T22:01:57Z

crates/core/src/operations/constraints.rs

-                .iter()
-                .map(|(_, sql)| Constraint::new("*", sql))
+                .values()
+                .map(|sql| Constraint::new("*", sql))


Crate: crates/core File: crates/core/src/operations/constraints.rs Clippy warnings fixed: "iterating on a map's keys --> crates/core/src/operations/constraints.rs:131:17 help: try: this.check_constraints.keys().map(|name| ...)" and "iterating on a map's values --> crates/core/src/operations/constraints.rs:179:56 help: try: constraints_sql_mapper.values().map(|sql| ...)" Change: iterated over keys/values directly: replaced .iter().map(|(name, )| ...) with .keys().cloned().map(|name| ...), and .iter().map(|(, sql)| ...) with .values().map(|sql| ...).

corwinjoy · 2025-11-20T22:02:23Z

crates/core/src/operations/vacuum.rs


        let expired_tombstones =
-            get_stale_files(&snapshot, retention_period, now_millis, &self.log_store).await?;
+            get_stale_files(snapshot, retention_period, now_millis, &self.log_store).await?;


Crate: crates/core File: crates/core/src/operations/vacuum.rs Clippy warning fixed: "this expression creates a reference which is immediately dereferenced by the compiler --> crates/core/src/operations/vacuum.rs:282:29 help: change this to: snapshot" Change: removed the needless borrow and passed snapshot directly to get_stale_files(snapshot, ...).

corwinjoy · 2025-11-20T22:02:55Z

crates/core/src/operations/write/execution.rs

        };

-        return Ok((actions, metrics));
+        Ok((actions, metrics))


Crate: crates/core File: crates/core/src/operations/write/execution.rs Clippy warnings fixed: "unneeded return statement --> crates/core/src/operations/write/execution.rs:372:9" and "unneeded return statement --> crates/core/src/operations/write/execution.rs:564:9" Change: removed redundant return keywords and returned the expressions directly (Ok((actions, metrics))).

corwinjoy · 2025-11-20T22:03:16Z

crates/core/src/operations/write/mod.rs


-    #[error("Failed to execute write task: {source}")]
-    WriteTask { source: tokio::task::JoinError },
-


Crate: crates/core File: crates/core/src/operations/write/mod.rs Clippy warnings fixed: "unused import: crate::delta_datafusion::planner::DeltaPlanner --> crates/core/src/operations/write/mod.rs:59:5" and "variant WriteTask is never constructed --> crates/core/src/operations/write/mod.rs:86:5" Change: removed the unused DeltaPlanner import and deleted the unused WriteTask { source: tokio::task::JoinError } enum variant.

corwinjoy · 2025-11-20T22:07:00Z

crates/core/src/kernel/models/actions.rs


 impl ProtocolInner {
    /// Create a new protocol action
+    #[allow(dead_code)]


Crate: crates/core File: crates/core/src/kernel/models/actions.rs Clippy warnings fixed: "associated function new is never used --> crates/core/src/kernel/models/actions.rs:271:19"

Not removed because this could break the API.

corwinjoy · 2025-11-20T22:08:19Z

OK. With the help of copilot I have added comments listing the warning message and what was done for each of these.

corwinjoy added 15 commits November 18, 2025 17:13

Turn off "deprecated" warnings.

e24b720

Signed-off-by: Corwin Joy <[email protected]>

core(write): remove unused DeltaPlanner import to satisfy clippy unus…

1caf2f4

…ed_imports Signed-off-by: Corwin Joy <[email protected]>

core(vacuum): remove needless borrow when calling get_stale_files (cl…

45702d1

…ippy::needless_borrow) Signed-off-by: Corwin Joy <[email protected]>

core(actions): derive Default for StorageType and IsolationLevel and …

65dc10a

…mark default variants (clippy::derivable_impls) Signed-off-by: Corwin Joy <[email protected]>

core(constraints): use keys()/values() to satisfy clippy iter_kv_map

57fc4aa

Signed-off-by: Corwin Joy <[email protected]>

core(log_data): avoid unnecessary to_vec and cloned ref; fix lifetime…

6650677

… on IntoIterator (clippy::unnecessary_to_owned, clippy::cloned_ref_to_slice_refs) Signed-off-by: Corwin Joy <[email protected]>

core(snapshot): initialize DeltaTableConfig with struct literal to av…

7fe2c91

…oid field reassignment (clippy::field_reassign_with_default) Signed-off-by: Corwin Joy <[email protected]>

core(log_data): remove needless borrow in from_ref (clippy::needless_…

6e16009

…borrow) Signed-off-by: Corwin Joy <[email protected]>

core(write): remove needless returns in execution plan (clippy::needl…

602a00e

…ess_return) Signed-off-by: Corwin Joy <[email protected]>

core(datafusion): use static LazyLock for planners to satisfy clippy …

52f2deb

…(declare_interior_mutable_const) Signed-off-by: Corwin Joy <[email protected]>

core(logstore): consolidate generic bounds in try_parse_impl to avoid…

ddb723e

… multiple_bound_locations (clippy) Signed-off-by: Corwin Joy <[email protected]>

benchmarks: remove needless borrow of parquet_dir (clippy::needless_b…

40af313

…orrow) Signed-off-by: Corwin Joy <[email protected]>

Fix unused scan_file_transforms

db38dc3

Signed-off-by: Corwin Joy <[email protected]>

Remove or allow dead schema.

70ad4aa

Remove module_inception warning for schema. Signed-off-by: Corwin Joy <[email protected]>

cargo fmt

78c909e

Signed-off-by: Corwin Joy <[email protected]>

corwinjoy requested review from hntd187, roeap and rtyler as code owners November 19, 2025 02:49

github-project-automation bot added this to delta-rust Nov 19, 2025

github-actions bot added the binding/rust Issues for the Rust crate label Nov 19, 2025

corwinjoy changed the title ~~fix: Remove clippy warnings~~ fix: remove clippy warnings Nov 19, 2025

corwinjoy commented Nov 19, 2025

View reviewed changes

corwinjoy commented Nov 20, 2025

View reviewed changes


		#[error("Failed to execute write task: {source}")]
		WriteTask { source: tokio::task::JoinError },

fix: remove clippy warnings #3940

Are you sure you want to change the base?

fix: remove clippy warnings #3940

Uh oh!

Conversation

corwinjoy commented Nov 19, 2025

Description

Related Issue(s)

Uh oh!

github-actions bot commented Nov 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

hntd187 commented Nov 20, 2025

Uh oh!

corwinjoy commented Nov 20, 2025

Uh oh!

corwinjoy commented Nov 20, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

corwinjoy commented Nov 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Nov 20, 2025 •

edited

Loading