chore: grab-bag of compiler warning removals

Signed-off-by: R. Tyler Croy <rtyler@brokenco.de>

Author: rtyler

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

@@ -13,6 +13,11 @@ use crate::TableProperty;
 pub use delta_kernel::actions::{Metadata, Protocol};
 
 /// Please don't use, this API will be leaving shortly!
+///
+/// Since the adoption of delta-kernel-rs we lost the direct ability to create [Metadata] actions
+/// which is required for some use-cases.
+///
+/// Upstream tracked here: <https://github.com/delta-io/delta-kernel-rs/issues/1055>
 pub fn new_metadata(
     schema: &StructType,
     partition_columns: impl IntoIterator<Item = impl ToString>,
@@ -263,7 +268,7 @@ impl Default for ProtocolInner {
 
 impl ProtocolInner {
     /// Create a new protocol action
-    pub fn new(min_reader_version: i32, min_writer_version: i32) -> Self {
+    pub(crate) fn new(min_reader_version: i32, min_writer_version: i32) -> Self {
         Self {
             min_reader_version,
             min_writer_version,

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

@@ -39,8 +39,6 @@ use futures::{StreamExt, TryStreamExt};
 use object_store::path::Path;
 use object_store::ObjectStore;
 use serde_json::Deserializer;
-use tokio::task::spawn_blocking;
-use tracing::Instrument;
 use url::Url;
 
 use super::{Action, CommitInfo, Metadata, Protocol};

crates/core/src/kernel/transaction/application.rs

@@ -1,11 +1,11 @@
+#[cfg(feature = "datafusion")]
 #[cfg(test)]
 mod tests {
     use crate::{
         checkpoints, ensure_table_uri, kernel::transaction::CommitProperties, kernel::Transaction,
         protocol::SaveMode, writer::test_utils::get_record_batch, DeltaOps, DeltaTableBuilder,
     };
 
-    #[cfg(feature = "datafusion")]
     #[tokio::test]
     async fn test_app_txn_workload() {
         // Test that the transaction ids can be read from different scenarios

crates/core/src/logstore/config.rs

@@ -300,6 +300,7 @@ pub fn str_is_truthy(val: &str) -> bool {
 #[cfg(test)]
 mod tests {
     use super::*;
+    #[cfg(feature = "cloud")]
     use std::time::Duration;
 
     // Test retry config parsing

crates/core/src/operations/create.rs

@@ -404,7 +404,7 @@ mod tests {
     use super::*;
     use crate::operations::DeltaOps;
     use crate::table::config::TableProperty;
-    use crate::writer::test_utils::{get_delta_schema, get_record_batch};
+    use crate::writer::test_utils::get_delta_schema;
     use tempfile::TempDir;
 
     #[tokio::test]
@@ -517,138 +517,141 @@ mod tests {
     }
 
     #[cfg(feature = "datafusion")]
-    #[tokio::test]
-    async fn test_create_table_save_mode() {
-        let tmp_dir = tempfile::tempdir().unwrap();
-
-        let schema = get_delta_schema();
-        let table = CreateBuilder::new()
-            .with_location(tmp_dir.path().to_str().unwrap())
-            .with_columns(schema.fields().cloned())
-            .await
-            .unwrap();
-        assert_eq!(table.version(), Some(0));
-        let first_id = table.snapshot().unwrap().metadata().id().to_string();
-
-        let log_store = table.log_store;
-
-        // Check an error is raised when a table exists at location
-        let table = CreateBuilder::new()
-            .with_log_store(log_store.clone())
-            .with_columns(schema.fields().cloned())
-            .with_save_mode(SaveMode::ErrorIfExists)
-            .await;
-        assert!(table.is_err());
-
-        // Check current table is returned when ignore option is chosen.
-        let table = CreateBuilder::new()
-            .with_log_store(log_store.clone())
-            .with_columns(schema.fields().cloned())
-            .with_save_mode(SaveMode::Ignore)
-            .await
-            .unwrap();
-        assert_eq!(table.snapshot().unwrap().metadata().id(), first_id);
-
-        // Check table is overwritten
-        let table = CreateBuilder::new()
-            .with_log_store(log_store)
-            .with_columns(schema.fields().cloned())
-            .with_save_mode(SaveMode::Overwrite)
-            .await
-            .unwrap();
-        assert_ne!(table.snapshot().unwrap().metadata().id(), first_id)
-    }
-
-    #[cfg(feature = "datafusion")]
-    #[tokio::test]
-    async fn test_create_or_replace_existing_table() {
-        let batch = get_record_batch(None, false);
-        let schema = get_delta_schema();
-        let table = DeltaOps::new_in_memory()
-            .write(vec![batch.clone()])
-            .with_save_mode(SaveMode::ErrorIfExists)
-            .await
-            .unwrap();
-        let state = table.snapshot().unwrap();
-        assert_eq!(state.version(), 0);
-        assert_eq!(state.log_data().num_files(), 1);
-
-        let mut table = DeltaOps(table)
-            .create()
-            .with_columns(schema.fields().cloned())
-            .with_save_mode(SaveMode::Overwrite)
-            .await
-            .unwrap();
-        table.load().await.unwrap();
-        let state = table.snapshot().unwrap();
-        assert_eq!(state.version(), 1);
-        // Checks if files got removed after overwrite
-        assert_eq!(state.log_data().num_files(), 0);
-    }
-
-    #[tokio::test]
-    #[cfg(feature = "datafusion")]
-    async fn test_create_or_replace_existing_table_partitioned() {
-        let batch = get_record_batch(None, false);
-        let schema = get_delta_schema();
-        let table = DeltaOps::new_in_memory()
-            .write(vec![batch.clone()])
-            .with_save_mode(SaveMode::ErrorIfExists)
-            .await
-            .unwrap();
-        let state = table.snapshot().unwrap();
-        assert_eq!(state.version(), 0);
-        assert_eq!(state.log_data().num_files(), 1);
-
-        let mut table = DeltaOps(table)
-            .create()
-            .with_columns(schema.fields().cloned())
-            .with_save_mode(SaveMode::Overwrite)
-            .with_partition_columns(vec!["id"])
-            .await
-            .unwrap();
-        table.load().await.unwrap();
-        let state = table.snapshot().unwrap();
-        assert_eq!(state.version(), 1);
-        // Checks if files got removed after overwrite
-        assert_eq!(state.log_data().num_files(), 0);
-    }
-
-    #[tokio::test]
-    async fn test_create_table_metadata_raise_if_key_not_exists() {
-        let schema = get_delta_schema();
-        let config: HashMap<String, Option<String>> =
-            vec![("key".to_string(), Some("value".to_string()))]
-                .into_iter()
-                .collect();
+    mod datafusion_tests {
+        use super::*;
+
+        use crate::writer::test_utils::get_record_batch;
+        #[tokio::test]
+        async fn test_create_table_save_mode() {
+            let tmp_dir = tempfile::tempdir().unwrap();
+
+            let schema = get_delta_schema();
+            let table = CreateBuilder::new()
+                .with_location(tmp_dir.path().to_str().unwrap())
+                .with_columns(schema.fields().cloned())
+                .await
+                .unwrap();
+            assert_eq!(table.version(), Some(0));
+            let first_id = table.snapshot().unwrap().metadata().id().to_string();
+
+            let log_store = table.log_store;
+
+            // Check an error is raised when a table exists at location
+            let table = CreateBuilder::new()
+                .with_log_store(log_store.clone())
+                .with_columns(schema.fields().cloned())
+                .with_save_mode(SaveMode::ErrorIfExists)
+                .await;
+            assert!(table.is_err());
+
+            // Check current table is returned when ignore option is chosen.
+            let table = CreateBuilder::new()
+                .with_log_store(log_store.clone())
+                .with_columns(schema.fields().cloned())
+                .with_save_mode(SaveMode::Ignore)
+                .await
+                .unwrap();
+            assert_eq!(table.snapshot().unwrap().metadata().id(), first_id);
+
+            // Check table is overwritten
+            let table = CreateBuilder::new()
+                .with_log_store(log_store)
+                .with_columns(schema.fields().cloned())
+                .with_save_mode(SaveMode::Overwrite)
+                .await
+                .unwrap();
+            assert_ne!(table.snapshot().unwrap().metadata().id(), first_id)
+        }
 
-        // Fail to create table with unknown Delta key
-        let table = CreateBuilder::new()
-            .with_location("memory:///")
-            .with_columns(schema.fields().cloned())
-            .with_configuration(config.clone())
-            .await;
-        assert!(table.is_err());
+        #[tokio::test]
+        async fn test_create_or_replace_existing_table() {
+            let batch = get_record_batch(None, false);
+            let schema = get_delta_schema();
+            let table = DeltaOps::new_in_memory()
+                .write(vec![batch.clone()])
+                .with_save_mode(SaveMode::ErrorIfExists)
+                .await
+                .unwrap();
+            let state = table.snapshot().unwrap();
+            assert_eq!(state.version(), 0);
+            assert_eq!(state.log_data().num_files(), 1);
+
+            let mut table = DeltaOps(table)
+                .create()
+                .with_columns(schema.fields().cloned())
+                .with_save_mode(SaveMode::Overwrite)
+                .await
+                .unwrap();
+            table.load().await.unwrap();
+            let state = table.snapshot().unwrap();
+            assert_eq!(state.version(), 1);
+            // Checks if files got removed after overwrite
+            assert_eq!(state.log_data().num_files(), 0);
+        }
 
-        // Succeed in creating table with unknown Delta key since we set raise_if_key_not_exists to false
-        let table = CreateBuilder::new()
-            .with_location("memory:///")
-            .with_columns(schema.fields().cloned())
-            .with_raise_if_key_not_exists(false)
-            .with_configuration(config)
-            .await;
-        assert!(table.is_ok());
+        #[tokio::test]
+        async fn test_create_or_replace_existing_table_partitioned() {
+            let batch = get_record_batch(None, false);
+            let schema = get_delta_schema();
+            let table = DeltaOps::new_in_memory()
+                .write(vec![batch.clone()])
+                .with_save_mode(SaveMode::ErrorIfExists)
+                .await
+                .unwrap();
+            let state = table.snapshot().unwrap();
+            assert_eq!(state.version(), 0);
+            assert_eq!(state.log_data().num_files(), 1);
+
+            let mut table = DeltaOps(table)
+                .create()
+                .with_columns(schema.fields().cloned())
+                .with_save_mode(SaveMode::Overwrite)
+                .with_partition_columns(vec!["id"])
+                .await
+                .unwrap();
+            table.load().await.unwrap();
+            let state = table.snapshot().unwrap();
+            assert_eq!(state.version(), 1);
+            // Checks if files got removed after overwrite
+            assert_eq!(state.log_data().num_files(), 0);
+        }
 
-        // Ensure the non-Delta key was set correctly
-        let value = table
-            .unwrap()
-            .snapshot()
-            .unwrap()
-            .metadata()
-            .configuration()
-            .get("key")
-            .unwrap()
-            .clone();
-        assert_eq!(String::from("value"), value);
+        #[tokio::test]
+        async fn test_create_table_metadata_raise_if_key_not_exists() {
+            let schema = get_delta_schema();
+            let config: HashMap<String, Option<String>> =
+                vec![("key".to_string(), Some("value".to_string()))]
+                    .into_iter()
+                    .collect();
+
+            // Fail to create table with unknown Delta key
+            let table = CreateBuilder::new()
+                .with_location("memory:///")
+                .with_columns(schema.fields().cloned())
+                .with_configuration(config.clone())
+                .await;
+            assert!(table.is_err());
+
+            // Succeed in creating table with unknown Delta key since we set raise_if_key_not_exists to false
+            let table = CreateBuilder::new()
+                .with_location("memory:///")
+                .with_columns(schema.fields().cloned())
+                .with_raise_if_key_not_exists(false)
+                .with_configuration(config)
+                .await;
+            assert!(table.is_ok());
+
+            // Ensure the non-Delta key was set correctly
+            let value = table
+                .unwrap()
+                .snapshot()
+                .unwrap()
+                .metadata()
+                .configuration()
+                .get("key")
+                .unwrap()
+                .clone();
+            assert_eq!(String::from("value"), value);
+        }
     }
 }

crates/core/src/operations/generate.rs

@@ -7,7 +7,7 @@
 //! enon-partitioned tables this will generate a `_symlink_format_manifest/manifest` file next to
 //! the `_delta_log`, for example:
 //!
-//! ```
+//! ```ignore
 //! COVID-19_NYT
 //! ├── _delta_log
 //! │   ├── 00000000000000000000.crc
@@ -27,7 +27,7 @@
 //! For partitioned tables, a `manifest` file will be generated inside a hive-style partitioned
 //! tree structure, e.g.:
 //!
-//! ```
+//! ```ignore
 //! delta-0.8.0-partitioned
 //! ├── _delta_log
 //! │   └── 00000000000000000000.json
@@ -74,12 +74,10 @@ use futures::future::BoxFuture;
 use std::collections::HashMap;
 use std::sync::Arc;
 
-use itertools::Itertools;
 use object_store::path::{Path, PathPart};
 use tracing::log::*;
 
 use super::{CustomExecuteHandler, Operation};
-use crate::kernel::scalars::ScalarExt;
 use crate::kernel::{resolve_snapshot, EagerSnapshot};
 use crate::logstore::object_store::PutPayload;
 use crate::logstore::LogStoreRef;

crates/core/src/operations/restore.rs

@@ -375,6 +375,7 @@ impl std::future::IntoFuture for RestoreBuilder {
 }
 
 #[cfg(test)]
+#[cfg(feature = "datafusion")]
 mod tests {
 
     use crate::writer::test_utils::{create_bare_table, get_record_batch};
@@ -383,7 +384,6 @@ mod tests {
     /// Verify that restore respects constraints that were added/removed in previous version_to_restore
     /// <https://github.com/delta-io/delta-rs/issues/3352>
     #[tokio::test]
-    #[cfg(feature = "datafusion")]
     async fn test_simple_restore_constraints() -> DeltaResult<()> {
         use std::collections::HashMap;