WIP: Enable needles-pass-by-value lint

Author: AdamGS

Cargo.toml

@@ -290,56 +290,56 @@ package = "getrandom"
 version = "0.3.1"
 
 [workspace.lints.rust]
-let_underscore_drop = "deny"
-macro_use_extern_crate = "deny"
-redundant_lifetimes = "deny"
-unsafe_op_in_unsafe_fn = "deny"
-unused_lifetimes = "deny"
-unused_qualifications = "deny"
-unexpected_cfgs = { level = "deny", check-cfg = [
+let_underscore_drop = "warn"
+macro_use_extern_crate = "warn"
+redundant_lifetimes = "warn"
+unsafe_op_in_unsafe_fn = "warn"
+unused_lifetimes = "warn"
+unused_qualifications = "warn"
+unexpected_cfgs = { level = "warn", check-cfg = [
     "cfg(codspeed)",
     "cfg(disable_loom)",
     "cfg(vortex_nightly)",
 ] }
 warnings = "warn"
 
 [workspace.lints.clippy]
-all = { level = "deny", priority = -1 }
-as_ptr_cast_mut = "deny"
-borrow_as_ptr = "deny"
-cargo = { level = "deny", priority = -1 }
-cast_possible_truncation = "deny"
-cognitive_complexity = "deny"
-collection_is_never_read = "deny"
-dbg_macro = "deny"
-debug_assert_with_mut_call = "deny"
-derive_partial_eq_without_eq = "deny"
-equatable_if_let = "deny"
-exit = "deny"
-expect_fun_call = "deny"
-expect_used = "deny"
-fallible_impl_from = "deny"
-get_unwrap = "deny"
-host_endian_bytes = "deny"
-if_then_some_else_none = "deny"
-inconsistent_struct_constructor = "deny"
-manual_assert = "deny"
-manual_is_variant_and = "deny"
-many_single_char_names = "deny"
-mem_forget = "deny"
+large_futures = "deny"
+all = { level = "warn", priority = -1 }
+as_ptr_cast_mut = "warn"
+borrow_as_ptr = "warn"
+cargo = { level = "warn", priority = -1 }
+cast_possible_truncation = "warn"
+cognitive_complexity = "warn"
+collection_is_never_read = "warn"
+dbg_macro = "warn"
+debug_assert_with_mut_call = "warn"
+derive_partial_eq_without_eq = "warn"
+equatable_if_let = "warn"
+exit = "warn"
+expect_fun_call = "warn"
+expect_used = "warn"
+fallible_impl_from = "warn"
+get_unwrap = "warn"
+host_endian_bytes = "warn"
+if_then_some_else_none = "warn"
+inconsistent_struct_constructor = "warn"
+manual_assert = "warn"
+manual_is_variant_and = "warn"
+many_single_char_names = "warn"
+mem_forget = "warn"
 multiple_crate_versions = "allow"
 needless_range_loop = "allow"
-or_fun_call = "deny"
-panic = "deny"
-# panic_in_result_fn = "deny" -- we cannot disable this for tests to use assertions
-redundant_clone = "deny"
-same_name_method = "deny"
-tests_outside_test_module = "deny"
-# todo = "deny"
-# unimplemented = "deny"
-unwrap_in_result = "deny"
-unwrap_used = "deny"
-use_debug = "deny"
+needless_pass_by_value = "warn"
+or_fun_call = "warn"
+panic = "warn"
+redundant_clone = "warn"
+same_name_method = "warn"
+tests_outside_test_module = "warn"
+unwrap_in_result = "warn"
+unwrap_used = "warn"
+use_debug = "warn"
+assigning_clones = "deny"
 
 [profile.release]
 codegen-units = 1

benchmarks/compress-bench/src/lib.rs

@@ -1,33 +1,8 @@
 // SPDX-License-Identifier: Apache-2.0
 // SPDX-FileCopyrightText: Copyright the Vortex contributors
 
-use std::sync::Arc;
-
-use ::vortex::array::arrays::ChunkedArray;
-use ::vortex::array::arrays::recursive_list_from_list_view;
-use arrow_array::RecordBatch;
-use arrow_schema::Schema;
 #[cfg(feature = "lance")]
 pub use lance_bench::compress::LanceCompressor;
+
 pub mod parquet;
 pub mod vortex;
-
-pub fn chunked_to_vec_record_batch(
-    chunked: ChunkedArray,
-) -> anyhow::Result<(Vec<RecordBatch>, Arc<Schema>)> {
-    let chunks_vec = chunked.chunks();
-    assert!(!chunks_vec.is_empty(), "empty chunks");
-
-    let batches = chunks_vec
-        .iter()
-        .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(array.clone())?;
-            Ok(RecordBatch::try_from(converted_array.as_ref())?)
-        })
-        .collect::<anyhow::Result<Vec<_>>>()?;
-
-    let schema = batches[0].schema();
-    Ok((batches, schema))
-}

benchmarks/compress-bench/src/vortex.rs

@@ -3,7 +3,6 @@
 
 use std::io::Cursor;
 use std::path::Path;
-use std::sync::Arc;
 use std::time::Duration;
 use std::time::Instant;
 
@@ -58,9 +57,9 @@ impl Compressor for VortexCompressor {
         let start = Instant::now();
         let data = Bytes::from(buf);
         let scan = SESSION.open_options().open_buffer(data)?.scan()?;
-        let schema = Arc::new(scan.dtype()?.to_arrow_schema()?);
+        let schema = scan.dtype()?.to_arrow_schema()?;
 
-        let stream = scan.into_record_batch_stream(schema)?;
+        let stream = scan.into_record_batch_stream(&schema)?;
         pin_mut!(stream);
 
         while let Some(batch) = stream.next().await {

benchmarks/datafusion-bench/src/main.rs

@@ -26,6 +26,7 @@ use datafusion_physical_plan::collect;
 use futures::StreamExt;
 use parking_lot::Mutex;
 use tokio::fs::File;
+use vortex::io::filesystem::FileSystemRef;
 use vortex::scan::api::DataSourceRef;
 use vortex_bench::Benchmark;
 use vortex_bench::BenchmarkArg;
@@ -304,9 +305,9 @@ 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(store.clone(), SESSION.handle()));
-        let base_prefix = benchmark_base.path().trim_start_matches('/').to_string();
+        let fs =
+            Arc::new(ObjectStoreFileSystem::new(store.clone(), SESSION.handle())) as FileSystemRef;
+        let base_prefix = benchmark_base.path().trim_start_matches('/');
         let fs = fs.with_prefix(base_prefix);
 
         let glob_pattern = match &pattern {

benchmarks/duckdb-bench/src/lib.rs

@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: Apache-2.0
 // SPDX-FileCopyrightText: Copyright the Vortex contributors
 
+#![allow(clippy::needless_pass_by_value)]
+
 //! DuckDB context for benchmarks.
 
 use std::ops::Deref;

benchmarks/duckdb-bench/src/main.rs

@@ -1,8 +1,6 @@
 // SPDX-License-Identifier: Apache-2.0
 // SPDX-FileCopyrightText: Copyright the Vortex contributors
 
-mod validation;
-
 use std::path::PathBuf;
 
 use clap::Parser;

benchmarks/duckdb-bench/src/validation.rs

@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: Apache-2.0
 // SPDX-FileCopyrightText: Copyright the Vortex contributors
 
+#![allow(clippy::needless_pass_by_value)]
+
 pub mod compress;
 pub mod convert;
 pub mod random_access;

benchmarks/lance-bench/src/lib.rs

@@ -48,15 +48,15 @@ pub fn decompress_into_array(
             &patches_values,
             &patches_chunk_offsets,
             patches,
-            dtype,
+            &dtype,
         ))
     } else {
         let encoded_prim = encoded.to_primitive();
         // We need to drop ALPArray here in case converting encoded buffer into
         // primitive didn't create a copy. In that case both alp_encoded and array
         // will hold a reference to the buffer we want to mutate.
         drop(encoded);
-        decompress_unchunked_core(encoded_prim, exponents, patches, dtype, ctx)
+        decompress_unchunked_core(encoded_prim, exponents, patches, &dtype, ctx)
     }
 }
 
@@ -70,6 +70,7 @@ pub fn decompress_into_array(
 /// A `PrimitiveArray` containing the decompressed floating-point values with all patches applied.
 pub fn execute_decompress(array: ALPArray, ctx: &mut ExecutionCtx) -> VortexResult<PrimitiveArray> {
     let (encoded, exponents, patches, dtype) = array.into_parts();
+
     if let Some(ref patches) = patches
         && let Some(chunk_offsets) = patches.chunk_offsets()
     {
@@ -85,11 +86,11 @@ pub fn execute_decompress(array: ALPArray, ctx: &mut ExecutionCtx) -> VortexResu
             &patches_values,
             &patches_chunk_offsets,
             patches,
-            dtype,
+            &dtype,
         ))
     } else {
         let encoded = encoded.execute::<PrimitiveArray>(ctx)?;
-        decompress_unchunked_core(encoded, exponents, patches, dtype, ctx)
+        decompress_unchunked_core(encoded, exponents, patches, &dtype, ctx)
     }
 }
 
@@ -108,7 +109,7 @@ fn decompress_chunked_core(
     patches_values: &PrimitiveArray,
     patches_chunk_offsets: &PrimitiveArray,
     patches: &Patches,
-    dtype: DType,
+    dtype: &DType,
 ) -> PrimitiveArray {
     let validity = encoded.validity().clone();
     let ptype = dtype.as_ptype();
@@ -157,7 +158,7 @@ fn decompress_unchunked_core(
     encoded: PrimitiveArray,
     exponents: Exponents,
     patches: Option<Patches>,
-    dtype: DType,
+    dtype: &DType,
     ctx: &mut ExecutionCtx,
 ) -> VortexResult<PrimitiveArray> {
     let validity = encoded.validity().clone();

encodings/alp/src/alp/decompress.rs

@@ -311,7 +311,7 @@ impl VTable for ALPRDVTable {
         let decoded_array = if array.is_f32() {
             PrimitiveArray::new(
                 alp_rd_decode::<f32>(
-                    left_parts.into_buffer::<u16>(),
+                    left_parts.as_slice::<u16>(),
                     left_parts_dict,
                     array.right_bit_width,
                     right_parts.into_buffer_mut::<u32>(),
@@ -323,7 +323,7 @@ impl VTable for ALPRDVTable {
         } else {
             PrimitiveArray::new(
                 alp_rd_decode::<f64>(
-                    left_parts.into_buffer::<u16>(),
+                    left_parts.as_slice::<u16>(),
                     left_parts_dict,
                     array.right_bit_width,
                     right_parts.into_buffer_mut::<u64>(),