wip

Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>

Author: joseph-isaacs

Cargo.toml

@@ -350,7 +350,7 @@ needless_range_loop = "allow"
 or_fun_call = "deny"
 panic = "deny"
 # panic_in_result_fn = "deny" -- we cannot disable this for tests to use assertions
-clone_on_ref_ptr = "warn"
+clone_on_ref_ptr = "deny"
 redundant_clone = "deny"
 same_name_method = "deny"
 tests_outside_test_module = "deny"

benchmarks/compress-bench/src/lib.rs

@@ -22,7 +22,7 @@ pub fn chunked_to_vec_record_batch(
         .map(|array| {
             // TODO(connor)[ListView]: The rust Parquet implementation does not support writing
             // `ListView` to Parquet files yet.
-            let converted_array = recursive_list_from_list_view(Arc::clone(&array))?;
+            let converted_array = recursive_list_from_list_view(Arc::clone(array))?;
             Ok(RecordBatch::try_from(converted_array.as_ref())?)
         })
         .collect::<anyhow::Result<Vec<_>>>()?;

benchmarks/compress-bench/src/parquet.rs

@@ -53,7 +53,7 @@ impl Compressor for ParquetCompressor {
         // Read the input parquet file
         let file = File::open(parquet_path)?;
         let builder = ParquetRecordBatchReaderBuilder::try_new(file)?;
-        let schema = Arc::clone(&builder.schema());
+        let schema = Arc::clone(builder.schema());
         let reader = builder.build()?;
         let batches: Vec<RecordBatch> = reader.collect::<Result<Vec<_>, _>>()?;
 
@@ -69,7 +69,7 @@ impl Compressor for ParquetCompressor {
         // First compress to get the bytes we'll decompress
         let file = File::open(parquet_path)?;
         let builder = ParquetRecordBatchReaderBuilder::try_new(file)?;
-        let schema = Arc::clone(&builder.schema());
+        let schema = Arc::clone(builder.schema());
         let reader = builder.build()?;
         let batches: Vec<RecordBatch> = reader.collect::<Result<Vec<_>, _>>()?;
 

benchmarks/datafusion-bench/src/lib.rs

@@ -81,8 +81,10 @@ pub fn make_object_store(
                     .with_bucket_name(bucket_name)
                     .build()?,
             );
-            session
-                .register_object_store(&Url::parse(&format!("s3://{bucket_name}/"))?, Arc::clone(&s3));
+            session.register_object_store(
+                &Url::parse(&format!("s3://{bucket_name}/"))?,
+                Arc::clone(&s3) as _,
+            );
             Ok(s3)
         }
         "gs" => {
@@ -92,13 +94,15 @@ pub fn make_object_store(
                     .with_bucket_name(bucket_name)
                     .build()?,
             );
-            session
-                .register_object_store(&Url::parse(&format!("gs://{bucket_name}/"))?, Arc::clone(&gcs));
+            session.register_object_store(
+                &Url::parse(&format!("gs://{bucket_name}/"))?,
+                Arc::clone(&gcs) as _,
+            );
             Ok(gcs)
         }
         _ => {
             let fs = Arc::new(LocalFileSystem::default());
-            session.register_object_store(&Url::parse("file:/")?, Arc::clone(&fs));
+            session.register_object_store(&Url::parse("file:/")?, Arc::clone(&fs) as _);
             Ok(fs)
         }
     }

benchmarks/datafusion-bench/src/main.rs

@@ -304,8 +304,10 @@ async fn register_v2_tables<B: Benchmark + ?Sized>(
             .runtime_env()
             .object_store(table_url.object_store())?;
 
-        let fs: vortex::io::filesystem::FileSystemRef =
-            Arc::new(ObjectStoreFileSystem::new(Arc::clone(&store), SESSION.handle()));
+        let fs: vortex::io::filesystem::FileSystemRef = Arc::new(ObjectStoreFileSystem::new(
+            Arc::clone(&store),
+            SESSION.handle(),
+        ));
         let base_prefix = benchmark_base.path().trim_start_matches('/').to_string();
         let fs = fs.with_prefix(base_prefix);
 

benchmarks/lance-bench/src/compress.rs

@@ -4,6 +4,7 @@
 use std::fs;
 use std::fs::File;
 use std::path::Path;
+use std::sync::Arc;
 use std::time::Duration;
 use std::time::Instant;
 
@@ -22,7 +23,6 @@ use vortex_bench::compress::Compressor;
 
 use crate::convert::convert_utf8view_batch;
 use crate::convert::convert_utf8view_schema;
-use std::sync::Arc;
 
 /// Read a Lance dataset and decompress it back into RecordBatches.
 pub async fn lance_decompress_read(path: &str) -> anyhow::Result<usize> {
@@ -93,7 +93,7 @@ impl Compressor for LanceCompressor {
         // Read the input parquet file
         let file = File::open(parquet_path)?;
         let builder = ParquetRecordBatchReaderBuilder::try_new(file)?;
-        let schema = Arc::clone(&builder.schema());
+        let schema = Arc::clone(builder.schema());
         let reader = builder.build()?;
         let batches: Vec<RecordBatch> = reader.collect::<Result<Vec<_>, _>>()?;
 
@@ -132,7 +132,7 @@ impl Compressor for LanceCompressor {
         // First compress to get the Lance dataset
         let file = File::open(parquet_path)?;
         let builder = ParquetRecordBatchReaderBuilder::try_new(file)?;
-        let schema = Arc::clone(&builder.schema());
+        let schema = Arc::clone(builder.schema());
         let reader = builder.build()?;
         let batches: Vec<RecordBatch> = reader.collect::<Result<Vec<_>, _>>()?;
 

benchmarks/lance-bench/src/convert.rs

@@ -161,7 +161,7 @@ pub async fn convert_parquet_to_lance<'p>(
             // Get schema from the first Parquet file
             let first_file = File::open(&parquet_files[0])?;
             let first_builder = ParquetRecordBatchReaderBuilder::try_new(first_file)?;
-            let schema = Arc::clone(&first_builder.schema());
+            let schema = Arc::clone(first_builder.schema());
 
             // Create a streaming iterator that reads from all Parquet files
             let batch_iter = ParquetFilesIterator::new(parquet_files, schema)?;
@@ -237,7 +237,7 @@ pub fn convert_utf8view_batch(batch: RecordBatch) -> anyhow::Result<RecordBatch>
             // Cast Utf8View to Utf8.
             cast(column, &DataType::Utf8)?
         } else {
-            Arc::clone(&column)
+            Arc::clone(column)
         };
         new_columns.push(new_column);
     }

encodings/alp/src/alp/array.rs

@@ -459,14 +459,14 @@ impl ALPArray {
     fn make_slots(encoded: &ArrayRef, patches: &Option<Patches>) -> Vec<Option<ArrayRef>> {
         let (patch_indices, patch_values, patch_chunk_offsets) = match patches {
             Some(p) => (
-                Some(Arc::clone(&p.indices())),
-                Some(Arc::clone(&p.values())),
+                Some(Arc::clone(p.indices())),
+                Some(Arc::clone(p.values())),
                 p.chunk_offsets().clone(),
             ),
             None => (None, None, None),
         };
         vec![
-            Some(Arc::clone(&encoded)),
+            Some(Arc::clone(encoded)),
             patch_indices,
             patch_values,
             patch_chunk_offsets,
@@ -501,8 +501,8 @@ impl ALPArray {
                     Patches::new_unchecked(
                         self.encoded().len(),
                         patch_offset,
-                        Arc::clone(&indices),
-                        Arc::clone(&values),
+                        Arc::clone(indices),
+                        Arc::clone(values),
                         self.slots[PATCH_CHUNK_OFFSETS_SLOT].clone(),
                         self.patch_offset_within_chunk,
                     )
@@ -836,15 +836,15 @@ mod tests {
         let patches_without_chunk_offsets = Patches::new(
             original_patches.array_len(),
             original_patches.offset(),
-            Arc::clone(&original_patches.indices()),
-            Arc::clone(&original_patches.values()),
+            Arc::clone(original_patches.indices()),
+            Arc::clone(original_patches.values()),
             None, // NO chunk_offsets - this triggers the bug!
         )
         .unwrap();
 
         // Build a new ALPArray with the same encoded data but patches without chunk_offsets.
         let alp_without_chunk_offsets = ALPArray::new(
-            Arc::clone(&normally_encoded.encoded()),
+            Arc::clone(normally_encoded.encoded()),
             normally_encoded.exponents(),
             Some(patches_without_chunk_offsets),
         );

encodings/alp/src/alp/compute/between.rs

@@ -2,6 +2,7 @@
 // SPDX-FileCopyrightText: Copyright the Vortex contributors
 
 use std::fmt::Debug;
+use std::sync::Arc;
 
 use vortex_array::ArrayRef;
 use vortex_array::IntoArray;
@@ -20,7 +21,6 @@ use crate::ALP;
 use crate::ALPArray;
 use crate::ALPFloat;
 use crate::match_each_alp_float_ptype;
-use std::sync::Arc;
 
 impl BetweenReduce for ALP {
     fn between(
@@ -85,7 +85,7 @@ where
         upper_strict,
     };
 
-    Arc::clone(&array.encoded()).between(
+    Arc::clone(array.encoded()).between(
         ConstantArray::new(Scalar::primitive(lower_enc, nullability), array.len()).into_array(),
         ConstantArray::new(Scalar::primitive(upper_enc, nullability), array.len()).into_array(),
         options,

encodings/alp/src/alp/compute/cast.rs

@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: Apache-2.0
 // SPDX-FileCopyrightText: Copyright the Vortex contributors
 
+use std::sync::Arc;
+
 use vortex_array::ArrayRef;
 use vortex_array::IntoArray;
 use vortex_array::builtins::ArrayBuiltins;
@@ -11,7 +13,6 @@ use vortex_error::VortexResult;
 
 use crate::alp::ALP;
 use crate::alp::ALPArray;
-use std::sync::Arc;
 
 impl CastReduce for ALP {
     fn cast(array: &ALPArray, dtype: &DType) -> VortexResult<Option<ArrayRef>> {
@@ -35,7 +36,7 @@ impl CastReduce for ALP {
                         Patches::new(
                             p.array_len(),
                             p.offset(),
-                            Arc::clone(&p.indices()),
+                            Arc::clone(p.indices()),
                             p.values().cast(dtype.clone())?,
                             p.chunk_offsets().clone(),
                         )