diff --git a/Cargo.toml b/Cargo.toml index 5bbc2ebab02..4438736d01c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -291,13 +291,13 @@ 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)", @@ -305,42 +305,42 @@ unexpected_cfgs = { level = "deny", check-cfg = [ 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 diff --git a/benchmarks/compress-bench/src/lib.rs b/benchmarks/compress-bench/src/lib.rs index c429e24e84e..0441e5e7fe6 100644 --- a/benchmarks/compress-bench/src/lib.rs +++ b/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, Arc)> { - 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::>>()?; - - let schema = batches[0].schema(); - Ok((batches, schema)) -} diff --git a/benchmarks/compress-bench/src/vortex.rs b/benchmarks/compress-bench/src/vortex.rs index b2eb1a5d490..ec03fb2ae65 100644 --- a/benchmarks/compress-bench/src/vortex.rs +++ b/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 { diff --git a/benchmarks/datafusion-bench/src/main.rs b/benchmarks/datafusion-bench/src/main.rs index 1a02be09ff0..71cfd8c5eb2 100644 --- a/benchmarks/datafusion-bench/src/main.rs +++ b/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( .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 { diff --git a/benchmarks/duckdb-bench/src/lib.rs b/benchmarks/duckdb-bench/src/lib.rs index fed9f82b004..a0538c1998b 100644 --- a/benchmarks/duckdb-bench/src/lib.rs +++ b/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; diff --git a/benchmarks/duckdb-bench/src/main.rs b/benchmarks/duckdb-bench/src/main.rs index 7ab8f1ac7ab..fa860dd290e 100644 --- a/benchmarks/duckdb-bench/src/main.rs +++ b/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; diff --git a/benchmarks/duckdb-bench/src/validation.rs b/benchmarks/duckdb-bench/src/validation.rs deleted file mode 100644 index 04fcde8e43c..00000000000 --- a/benchmarks/duckdb-bench/src/validation.rs +++ /dev/null @@ -1,110 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -// SPDX-FileCopyrightText: Copyright the Vortex contributors - -//! TPC-H result validation against reference data. - -use std::env; -use std::fs; -use std::path::Path; -use std::path::PathBuf; - -use duckdb_bench::DuckClient; -use similar::ChangeTag; -use similar::TextDiff; -use vortex_bench::Format; -use vortex_bench::tpch::benchmark::TpcHBenchmark; - -/// Verify DuckDB TPC-H results against reference data. -/// -/// This function runs TPC-H queries via DuckDB on Vortex files and compares -/// the results against known-correct reference outputs. -/// -/// Only runs for scale factor 1.0 since reference data is only available for SF=1. -#[expect(dead_code)] -pub fn verify_duckdb_tpch_results( - benchmark: &TpcHBenchmark, - queries: Vec, -) -> anyhow::Result<()> { - // omit validation for sf != 1. - if benchmark.scale_factor != "1.0" { - return Ok(()); - } - - let query_dir = Path::new(env!("CARGO_MANIFEST_DIR")) - .join("../../vortex-duckdb/duckdb/extension/tpch/dbgen/queries"); - - let tmp_dir = format!( - "{}/spiral-tpch", - // $RUNNER_TEMP is defined by GitHub Actions. - env::var("TMPDIR").or_else(|_| env::var("RUNNER_TEMP"))? - ); - - if PathBuf::from(&tmp_dir).exists() { - fs::remove_dir_all(&tmp_dir)?; - } - fs::create_dir(&tmp_dir)?; - - let duckdb = DuckClient::new_in_memory()?; - duckdb.register_tables(benchmark, Format::OnDiskVortex)?; - - let mut query_files = fs::read_dir(query_dir)? - .filter_map(Result::ok) - .filter(|entry| entry.path().extension().is_some_and(|ext| ext == "sql")) - .collect::>(); - query_files.sort_by_key(|entry| entry.file_name()); - - let mut is_matching_ref_result = true; - - for query_file in query_files - .iter() - .enumerate() - .filter(|entry| queries.contains(&(entry.0 + 1))) - .map(|query_file| query_file.1) - { - let query_file_path = query_file.path(); - let query_name = query_file_path - .file_stem() - .and_then(|stem| stem.to_str()) - .ok_or_else(|| anyhow::anyhow!("Invalid query filename"))?; - - let create_table = format!( - "CREATE OR REPLACE TABLE {query_name}_result AS {};", - fs::read_to_string(&query_file_path)? - ); - - let csv_actual = format!("{tmp_dir}/{query_name}.csv"); - let write_csv = - format!("COPY {query_name}_result TO '{csv_actual}' (HEADER, DELIMITER '|');",); - - duckdb.execute_query(&create_table)?; - duckdb.execute_query(&write_csv)?; - - let csv_expected = Path::new(env!("CARGO_MANIFEST_DIR")).join(format!( - "../../vortex-bench/tpch/results/duckdb/{query_name}.csv" - )); - let expected = fs::read_to_string(csv_expected)?; - let actual = fs::read_to_string(csv_actual)?; - - if expected != actual { - let diff = TextDiff::from_lines(&expected, &actual); - - for change in diff.iter_all_changes() { - let sign = match change.tag() { - ChangeTag::Delete => "-", - ChangeTag::Insert => "+", - ChangeTag::Equal => " ", - }; - print!("{sign}{change}"); - } - - eprintln!("query output does not match the reference for {query_name}"); - is_matching_ref_result = false; - } - } - - if !is_matching_ref_result { - return Err(anyhow::anyhow!("not all queries matched the reference")); - } - - Ok(()) -} diff --git a/benchmarks/lance-bench/src/lib.rs b/benchmarks/lance-bench/src/lib.rs index a646bb4e680..61814f88ca6 100644 --- a/benchmarks/lance-bench/src/lib.rs +++ b/benchmarks/lance-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)] + pub mod compress; pub mod convert; pub mod random_access; diff --git a/encodings/alp/public-api.lock b/encodings/alp/public-api.lock index e1a0f5f0498..4aca75c2ce6 100644 --- a/encodings/alp/public-api.lock +++ b/encodings/alp/public-api.lock @@ -548,6 +548,6 @@ pub fn f64::to_u16(bits: Self::UINT) -> u16 pub fn vortex_alp::alp_encode(parray: &vortex_array::arrays::primitive::array::PrimitiveArray, exponents: core::option::Option) -> vortex_error::VortexResult -pub fn vortex_alp::alp_rd_decode(left_parts: vortex_buffer::buffer::Buffer, left_parts_dict: &[u16], right_bit_width: u8, right_parts: vortex_buffer::buffer_mut::BufferMut<::UINT>, left_parts_patches: core::option::Option<&vortex_array::patches::Patches>, ctx: &mut vortex_array::executor::ExecutionCtx) -> vortex_error::VortexResult> +pub fn vortex_alp::alp_rd_decode(left_parts: &[u16], left_parts_dict: &[u16], right_bit_width: u8, right_parts: vortex_buffer::buffer_mut::BufferMut<::UINT>, left_parts_patches: core::option::Option<&vortex_array::patches::Patches>, ctx: &mut vortex_array::executor::ExecutionCtx) -> vortex_error::VortexResult> pub fn vortex_alp::decompress_into_array(array: vortex_alp::ALPArray, ctx: &mut vortex_array::executor::ExecutionCtx) -> vortex_error::VortexResult diff --git a/encodings/alp/src/alp/compute/mod.rs b/encodings/alp/src/alp/compute/mod.rs index 58a09ee4358..5dc6e8ee403 100644 --- a/encodings/alp/src/alp/compute/mod.rs +++ b/encodings/alp/src/alp/compute/mod.rs @@ -45,6 +45,6 @@ mod tests { #[case::f32_large(alp_encode(&PrimitiveArray::from_iter((0..100).map(|i| i as f32 * 1.5)), None).unwrap())] #[case::f64_large(alp_encode(&PrimitiveArray::from_iter((0..100).map(|i| i as f64 * 2.5)), None).unwrap())] fn test_alp_binary_numeric(#[case] array: ALPArray) { - test_binary_numeric_array(array.into_array()); + test_binary_numeric_array(&array.into_array()); } } diff --git a/encodings/alp/src/alp/decompress.rs b/encodings/alp/src/alp/decompress.rs index a979fea8f80..fd85389252e 100644 --- a/encodings/alp/src/alp/decompress.rs +++ b/encodings/alp/src/alp/decompress.rs @@ -6,12 +6,14 @@ use std::mem::transmute; use vortex_array::ExecutionCtx; use vortex_array::ToCanonical; use vortex_array::arrays::PrimitiveArray; +use vortex_array::arrays::PrimitiveArrayParts; use vortex_array::arrays::chunk_range; use vortex_array::arrays::patch_chunk; use vortex_array::dtype::DType; use vortex_array::match_each_unsigned_integer_ptype; use vortex_array::patches::Patches; use vortex_array::vtable::ValidityHelper; +use vortex_buffer::Buffer; use vortex_buffer::BufferMut; use vortex_error::VortexResult; @@ -48,7 +50,7 @@ pub fn decompress_into_array( &patches_values, &patches_chunk_offsets, patches, - dtype, + &dtype, )) } else { let encoded_prim = encoded.to_primitive(); @@ -56,7 +58,7 @@ pub fn decompress_into_array( // 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 +72,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 { let (encoded, exponents, patches, dtype) = array.into_parts(); + if let Some(ref patches) = patches && let Some(chunk_offsets) = patches.chunk_offsets() { @@ -85,11 +88,11 @@ pub fn execute_decompress(array: ALPArray, ctx: &mut ExecutionCtx) -> VortexResu &patches_values, &patches_chunk_offsets, patches, - dtype, + &dtype, )) } else { let encoded = encoded.execute::(ctx)?; - decompress_unchunked_core(encoded, exponents, patches, dtype, ctx) + decompress_unchunked_core(encoded, exponents, patches, &dtype, ctx) } } @@ -108,16 +111,27 @@ 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(); let array_len = encoded.len(); + + let PrimitiveArrayParts { + ptype: _, + buffer, + validity, + } = encoded.into_parts(); + let offset_within_chunk = patches.offset_within_chunk().unwrap_or(0); - match_each_alp_float_ptype!(ptype, |T| { + match_each_alp_float_ptype!(dtype.as_ptype(), |T| { let patches_values = patches_values.as_slice::(); - let mut alp_buffer = encoded.into_buffer_mut(); + let mut alp_buffer = { + let buffer = Buffer::from_byte_buffer(buffer.into_host_sync()); + buffer + .try_into_mut() + .unwrap_or_else(|buffer| BufferMut::copy_from(&buffer)) + }; + match_each_unsigned_integer_ptype!(patches_chunk_offsets.ptype(), |C| { let patches_chunk_offsets = patches_chunk_offsets.as_slice::(); @@ -157,7 +171,7 @@ fn decompress_unchunked_core( encoded: PrimitiveArray, exponents: Exponents, patches: Option, - dtype: DType, + dtype: &DType, ctx: &mut ExecutionCtx, ) -> VortexResult { let validity = encoded.validity().clone(); diff --git a/encodings/alp/src/alp_rd/array.rs b/encodings/alp/src/alp_rd/array.rs index e015937ae78..a7b9ff19af8 100644 --- a/encodings/alp/src/alp_rd/array.rs +++ b/encodings/alp/src/alp_rd/array.rs @@ -311,7 +311,7 @@ impl VTable for ALPRDVTable { let decoded_array = if array.is_f32() { PrimitiveArray::new( alp_rd_decode::( - left_parts.into_buffer::(), + left_parts.as_slice::(), left_parts_dict, array.right_bit_width, right_parts.into_buffer_mut::(), @@ -323,7 +323,7 @@ impl VTable for ALPRDVTable { } else { PrimitiveArray::new( alp_rd_decode::( - left_parts.into_buffer::(), + left_parts.as_slice::(), left_parts_dict, array.right_bit_width, right_parts.into_buffer_mut::(), diff --git a/encodings/alp/src/alp_rd/compute/mod.rs b/encodings/alp/src/alp_rd/compute/mod.rs index 1a4c240667b..83058d74de8 100644 --- a/encodings/alp/src/alp_rd/compute/mod.rs +++ b/encodings/alp/src/alp_rd/compute/mod.rs @@ -95,6 +95,6 @@ mod tests { encoder.encode(&arr) })] fn test_alp_rd_binary_numeric(#[case] array: ALPRDArray) { - test_binary_numeric_array(array.into_array()); + test_binary_numeric_array(&array.into_array()); } } diff --git a/encodings/alp/src/alp_rd/mod.rs b/encodings/alp/src/alp_rd/mod.rs index 58188f7ab9f..0ba03c1df84 100644 --- a/encodings/alp/src/alp_rd/mod.rs +++ b/encodings/alp/src/alp_rd/mod.rs @@ -291,7 +291,7 @@ impl RDEncoder { /// /// The function panics if the provided `left_parts` and `right_parts` differ in length. pub fn alp_rd_decode( - left_parts: Buffer, + left_parts: &[u16], left_parts_dict: &[u16], right_bit_width: u8, right_parts: BufferMut, @@ -321,7 +321,7 @@ pub fn alp_rd_decode( left_parts_dict, right_bit_width, right_parts, - values, + values.as_ref(), )) } @@ -348,7 +348,7 @@ fn alp_rd_decode_core( _left_parts_dict: &[u16], right_bit_width: u8, right_parts: BufferMut, - values: BufferMut, + values: &[u16], ) -> Buffer { // Shift the left-parts and add in the right-parts. let mut index = 0; diff --git a/encodings/datetime-parts/src/compress.rs b/encodings/datetime-parts/src/compress.rs index 8ef0c760546..b277ce748e8 100644 --- a/encodings/datetime-parts/src/compress.rs +++ b/encodings/datetime-parts/src/compress.rs @@ -27,6 +27,7 @@ pub struct TemporalParts { /// /// Splitting the components by granularity creates more small values, which enables better /// cascading compression. +#[allow(clippy::needless_pass_by_value)] pub fn split_temporal(array: TemporalArray) -> VortexResult { let temporal_values = array.temporal_values().to_primitive(); diff --git a/encodings/datetime-parts/src/ops.rs b/encodings/datetime-parts/src/ops.rs index f6226298e2f..44c3ee74ea6 100644 --- a/encodings/datetime-parts/src/ops.rs +++ b/encodings/datetime-parts/src/ops.rs @@ -52,7 +52,7 @@ impl OperationsVTable for DateTimePartsVTable { .vortex_expect("subseconds fits in i64"); let ts = timestamp::combine( - TimestampParts { + &TimestampParts { days, seconds, subseconds, diff --git a/encodings/datetime-parts/src/timestamp.rs b/encodings/datetime-parts/src/timestamp.rs index 6cafeb530d5..7011e36bd5a 100644 --- a/encodings/datetime-parts/src/timestamp.rs +++ b/encodings/datetime-parts/src/timestamp.rs @@ -57,7 +57,7 @@ pub fn split(timestamp: i64, time_unit: TimeUnit) -> VortexResult i64 { +pub fn combine(ts_parts: &TimestampParts, time_unit: TimeUnit) -> i64 { let divisor = match time_unit { TimeUnit::Nanoseconds => 1_000_000_000, TimeUnit::Microseconds => 1_000_000, @@ -142,7 +142,7 @@ mod tests { seconds: 3723, // 1*3600 + 2*60 + 3 subseconds: 0, }; - let ts = combine(parts, TimeUnit::Seconds); + let ts = combine(&parts, TimeUnit::Seconds); assert_eq!(ts, SECONDS_PER_DAY + 3723); } @@ -154,7 +154,7 @@ mod tests { seconds: 3723, subseconds: 456, }; - let ts = combine(parts, TimeUnit::Milliseconds); + let ts = combine(&parts, TimeUnit::Milliseconds); assert_eq!(ts, (SECONDS_PER_DAY + 3723) * 1000 + 456); } @@ -166,7 +166,7 @@ mod tests { seconds: 3723, subseconds: 456789, }; - let ts = combine(parts, TimeUnit::Microseconds); + let ts = combine(&parts, TimeUnit::Microseconds); assert_eq!(ts, (SECONDS_PER_DAY + 3723) * 1_000_000 + 456789); } @@ -178,7 +178,7 @@ mod tests { seconds: 3723, subseconds: 456789123, }; - let ts = combine(parts, TimeUnit::Nanoseconds); + let ts = combine(&parts, TimeUnit::Nanoseconds); assert_eq!(ts, (SECONDS_PER_DAY + 3723) * 1_000_000_000 + 456789123); } @@ -190,6 +190,6 @@ mod tests { seconds: 0, subseconds: 0, }; - combine(parts, TimeUnit::Days); + combine(&parts, TimeUnit::Days); } } diff --git a/encodings/decimal-byte-parts/src/decimal_byte_parts/compute/compare.rs b/encodings/decimal-byte-parts/src/decimal_byte_parts/compute/compare.rs index b1a5f0987d7..a8cd540f746 100644 --- a/encodings/decimal-byte-parts/src/decimal_byte_parts/compute/compare.rs +++ b/encodings/decimal-byte-parts/src/decimal_byte_parts/compute/compare.rs @@ -79,7 +79,7 @@ impl CompareKernel for DecimalBytePartsVTable { // Used to represent the overflow direction when trying to // convert into the scalar type. -#[derive(Debug)] +#[derive(Debug, Copy, Clone)] enum Sign { Positive, Negative, diff --git a/encodings/fastlanes/src/bitpacking/array/bitpack_compress.rs b/encodings/fastlanes/src/bitpacking/array/bitpack_compress.rs index ab807dd7699..d39c216fba8 100644 --- a/encodings/fastlanes/src/bitpacking/array/bitpack_compress.rs +++ b/encodings/fastlanes/src/bitpacking/array/bitpack_compress.rs @@ -6,7 +6,9 @@ use itertools::Itertools; use num_traits::PrimInt; use vortex_array::IntoArray; use vortex_array::arrays::PrimitiveArray; +use vortex_array::arrays::PrimitiveArrayParts; use vortex_array::buffer::BufferHandle; +use vortex_array::dtype::DType; use vortex_array::dtype::IntegerPType; use vortex_array::dtype::NativePType; use vortex_array::dtype::PType; @@ -105,22 +107,34 @@ pub unsafe fn bitpack_encode_unchecked( // SAFETY: non-negativity of input checked by caller. let packed = unsafe { bitpack_unchecked(&array, bit_width)? }; + let nullability = array.dtype().nullability(); + + let len = array.len(); + let stats = array.statistics().to_owned(); + + let PrimitiveArrayParts { + ptype, + buffer: _, + validity, + } = array.into_parts(); + // SAFETY: checked by bitpack_unchecked let bitpacked = unsafe { BitPackedArray::new_unchecked( BufferHandle::new_host(packed), - array.dtype().clone(), - array.validity().clone(), + DType::from(ptype).with_nullability(nullability), + validity, None, bit_width, - array.len(), + len, 0, ) }; - bitpacked - .stats_set - .to_ref(bitpacked.as_ref()) - .inherit_from(array.statistics()); + + for (stat, value) in stats.into_iter() { + bitpacked.stats_set.set(stat, value); + } + Ok(bitpacked) } @@ -220,7 +234,7 @@ pub fn gather_patches( bit_width, num_exceptions_hint, patch_validity, - validity_mask, + &validity_mask, )? }) } else if array_len < u16::MAX as usize { @@ -230,7 +244,7 @@ pub fn gather_patches( bit_width, num_exceptions_hint, patch_validity, - validity_mask, + &validity_mask, )? }) } else if array_len < u32::MAX as usize { @@ -240,7 +254,7 @@ pub fn gather_patches( bit_width, num_exceptions_hint, patch_validity, - validity_mask, + &validity_mask, )? }) } else { @@ -250,7 +264,7 @@ pub fn gather_patches( bit_width, num_exceptions_hint, patch_validity, - validity_mask, + &validity_mask, )? }) }; @@ -263,7 +277,7 @@ fn gather_patches_impl( bit_width: u8, num_exceptions_hint: usize, patch_validity: Validity, - validity_mask: Mask, + validity_mask: &Mask, ) -> VortexResult> where T: PrimInt + NativePType, diff --git a/encodings/fastlanes/src/bitpacking/array/bitpack_decompress.rs b/encodings/fastlanes/src/bitpacking/array/bitpack_decompress.rs index c7e399983cb..22f89333edf 100644 --- a/encodings/fastlanes/src/bitpacking/array/bitpack_decompress.rs +++ b/encodings/fastlanes/src/bitpacking/array/bitpack_decompress.rs @@ -165,7 +165,7 @@ pub fn apply_patches_to_uninit_range_fn T>( dst, indices.as_slice::

(), values, - validity, + &validity, patches.offset(), f, ) @@ -181,7 +181,7 @@ fn insert_values_and_validity_at_indices_to_uninit_range< dst: &mut UninitRange, indices: &[IndexT], values: &[T], - values_validity: Mask, + values_validity: &Mask, indices_offset: usize, f: F, ) { diff --git a/encodings/fastlanes/src/bitpacking/compute/mod.rs b/encodings/fastlanes/src/bitpacking/compute/mod.rs index 999a9cbb793..56b9756fbd2 100644 --- a/encodings/fastlanes/src/bitpacking/compute/mod.rs +++ b/encodings/fastlanes/src/bitpacking/compute/mod.rs @@ -93,6 +93,6 @@ mod tests { #[case::i32_basic(bitpack_encode(&PrimitiveArray::from_iter([10i32, 20, 30, 40, 50]), 7, None).unwrap())] #[case::large_u32(bitpack_encode(&PrimitiveArray::from_iter((0..100).map(|i| i as u32)), 7, None).unwrap())] fn test_bitpacked_binary_numeric(#[case] array: BitPackedArray) { - test_binary_numeric_array(array.into_array()); + test_binary_numeric_array(&array.into_array()); } } diff --git a/encodings/fastlanes/src/delta/array/delta_compress.rs b/encodings/fastlanes/src/delta/array/delta_compress.rs index f15c3ad8d58..c6df6ad9cbe 100644 --- a/encodings/fastlanes/src/delta/array/delta_compress.rs +++ b/encodings/fastlanes/src/delta/array/delta_compress.rs @@ -109,23 +109,23 @@ mod tests { #[test] fn test_compress() -> VortexResult<()> { - do_roundtrip_test((0u32..10_000).collect()) + do_roundtrip_test(&(0u32..10_000).collect()) } #[test] fn test_compress_nullable() -> VortexResult<()> { - do_roundtrip_test(PrimitiveArray::from_option_iter( + do_roundtrip_test(&PrimitiveArray::from_option_iter( (0u32..10_000).map(|i| (i % 2 == 0).then_some(i)), )) } #[test] fn test_compress_overflow() -> VortexResult<()> { - do_roundtrip_test((0..10_000).map(|i| (i % (u8::MAX as i32)) as u8).collect()) + do_roundtrip_test(&(0..10_000).map(|i| (i % (u8::MAX as i32)) as u8).collect()) } - fn do_roundtrip_test(input: PrimitiveArray) -> VortexResult<()> { - let delta = DeltaArray::try_from_primitive_array(&input)?; + fn do_roundtrip_test(input: &PrimitiveArray) -> VortexResult<()> { + let delta = DeltaArray::try_from_primitive_array(input)?; assert_eq!(delta.len(), input.len()); let decompressed = delta_decompress(&delta, &mut SESSION.create_execution_ctx())?; assert_arrays_eq!(decompressed, input); diff --git a/encodings/fastlanes/src/delta/vtable/operations.rs b/encodings/fastlanes/src/delta/vtable/operations.rs index 619c65712b1..481088cf55f 100644 --- a/encodings/fastlanes/src/delta/vtable/operations.rs +++ b/encodings/fastlanes/src/delta/vtable/operations.rs @@ -238,6 +238,6 @@ mod tests { #[case::delta_u64_basic(DeltaArray::try_from_vec(vec![1u64, 1, 1, 1, 1]).unwrap())] #[case::delta_u32_large(DeltaArray::try_from_vec(vec![1u32; 100]).unwrap())] fn test_delta_binary_numeric(#[case] array: DeltaArray) { - test_binary_numeric_array(array.into_array()); + test_binary_numeric_array(&array.into_array()); } } diff --git a/encodings/fastlanes/src/for/array/mod.rs b/encodings/fastlanes/src/for/array/mod.rs index 18f4e6ee415..c4deac555d9 100644 --- a/encodings/fastlanes/src/for/array/mod.rs +++ b/encodings/fastlanes/src/for/array/mod.rs @@ -23,6 +23,7 @@ pub struct FoRArray { } impl FoRArray { + #[allow(clippy::needless_pass_by_value)] pub fn try_new(encoded: ArrayRef, reference: Scalar) -> VortexResult { if reference.is_null() { vortex_bail!("Reference value cannot be null"); diff --git a/encodings/fastlanes/src/for/compute/mod.rs b/encodings/fastlanes/src/for/compute/mod.rs index 1374d826cb3..1dc643068f9 100644 --- a/encodings/fastlanes/src/for/compute/mod.rs +++ b/encodings/fastlanes/src/for/compute/mod.rs @@ -182,6 +182,6 @@ mod tests { Scalar::from(2000i32) ).unwrap())] fn test_for_binary_numeric(#[case] array: FoRArray) { - test_binary_numeric_array(array.into_array()); + test_binary_numeric_array(&array.into_array()); } } diff --git a/encodings/fsst/src/compress.rs b/encodings/fsst/src/compress.rs index 9a1e6828eaf..9f896a441f4 100644 --- a/encodings/fsst/src/compress.rs +++ b/encodings/fsst/src/compress.rs @@ -17,6 +17,7 @@ use vortex_error::VortexExpect; use crate::FSSTArray; /// Compress a string array using FSST. +#[allow(clippy::needless_pass_by_value)] pub fn fsst_compress + AsRef>( strings: A, compressor: &Compressor, diff --git a/encodings/pco/public-api.lock b/encodings/pco/public-api.lock index a3b019777f7..1a3042bca0a 100644 --- a/encodings/pco/public-api.lock +++ b/encodings/pco/public-api.lock @@ -6,7 +6,7 @@ impl vortex_pco::PcoArray pub fn vortex_pco::PcoArray::decompress(&self) -> vortex_error::VortexResult -pub fn vortex_pco::PcoArray::from_array(array: vortex_array::array::ArrayRef, level: usize, nums_per_page: usize) -> vortex_error::VortexResult +pub fn vortex_pco::PcoArray::from_array(array: &vortex_array::array::ArrayRef, level: usize, nums_per_page: usize) -> vortex_error::VortexResult pub fn vortex_pco::PcoArray::from_primitive(parray: &vortex_array::arrays::primitive::array::PrimitiveArray, level: usize, values_per_page: usize) -> vortex_error::VortexResult diff --git a/encodings/pco/src/array.rs b/encodings/pco/src/array.rs index ac4d8fbf86f..8cec3f14196 100644 --- a/encodings/pco/src/array.rs +++ b/encodings/pco/src/array.rs @@ -428,7 +428,7 @@ impl PcoArray { )) } - pub fn from_array(array: ArrayRef, level: usize, nums_per_page: usize) -> VortexResult { + pub fn from_array(array: &ArrayRef, level: usize, nums_per_page: usize) -> VortexResult { if let Some(parray) = array.as_opt::() { Self::from_primitive(parray, level, nums_per_page) } else { diff --git a/encodings/runend/src/array.rs b/encodings/runend/src/array.rs index 0f4b2949300..9f723bcaa38 100644 --- a/encodings/runend/src/array.rs +++ b/encodings/runend/src/array.rs @@ -34,6 +34,7 @@ use vortex_error::VortexExpect as _; use vortex_error::VortexResult; use vortex_error::vortex_bail; use vortex_error::vortex_ensure; +use vortex_error::vortex_err; use vortex_error::vortex_panic; use vortex_session::VortexSession; @@ -435,19 +436,19 @@ impl RunEndArray { /// Run the array through run-end encoding. pub fn encode(array: ArrayRef) -> VortexResult { - if let Some(parray) = array.as_opt::() { - let (ends, values) = runend_encode(parray); - // SAFETY: runend_encode handles this - unsafe { - Ok(Self::new_unchecked( - ends.into_array(), - values, - 0, - array.len(), - )) - } - } else { - vortex_bail!("REE can only encode primitive arrays") + let parray = array + .try_into::() + .map_err(|_| vortex_err!("REE can only encode primitive arrays"))?; + + let (ends, values) = runend_encode(&parray); + // SAFETY: runend_encode handles this + unsafe { + Ok(Self::new_unchecked( + ends.into_array(), + values, + 0, + parray.len(), + )) } } diff --git a/encodings/runend/src/compress.rs b/encodings/runend/src/compress.rs index 94f42281f48..12b0f62146e 100644 --- a/encodings/runend/src/compress.rs +++ b/encodings/runend/src/compress.rs @@ -6,6 +6,7 @@ use vortex_array::ArrayRef; use vortex_array::IntoArray; use vortex_array::ToCanonical; use vortex_array::arrays::BoolArray; +use vortex_array::arrays::BoolArrayParts; use vortex_array::arrays::ConstantArray; use vortex_array::arrays::PrimitiveArray; use vortex_array::dtype::NativePType; @@ -59,7 +60,7 @@ pub fn runend_encode(array: &PrimitiveArray) -> (PrimitiveArray, ArrayRef) { Some(validity) => { match_each_native_ptype!(array.ptype(), |P| { let (ends, values) = - runend_encode_nullable_primitive(array.as_slice::

(), validity); + runend_encode_nullable_primitive(array.as_slice::

(), &validity); ( PrimitiveArray::new(ends, Validity::NonNullable), values.into_array(), @@ -106,7 +107,7 @@ fn runend_encode_primitive(elements: &[T]) -> (Buffer, Buff fn runend_encode_nullable_primitive( elements: &[T], - element_validity: BitBuffer, + element_validity: &BitBuffer, ) -> (Buffer, PrimitiveArray) { let mut ends = BufferMut::empty(); let mut values = BufferMut::empty(); @@ -172,14 +173,15 @@ pub fn runend_decode_primitive( offset: usize, length: usize, ) -> VortexResult { + let nullability = values.dtype().nullability(); let validity_mask = values.validity_mask()?; Ok(match_each_native_ptype!(values.ptype(), |P| { match_each_unsigned_integer_ptype!(ends.ptype(), |E| { runend_decode_typed_primitive( - trimmed_ends_iter(ends.as_slice::(), offset, length), - values.as_slice::

(), + trimmed_ends_iter(&ends.into_buffer::(), offset, length), + &values.into_buffer::

(), validity_mask, - values.dtype().nullability(), + nullability, length, ) }) @@ -192,13 +194,24 @@ pub fn runend_decode_bools( offset: usize, length: usize, ) -> VortexResult { - let validity_mask = values.validity_mask()?; + let nullability = values.dtype().nullability(); + let BoolArrayParts { + bits, + offset: bool_offsets, + len, + validity, + } = values.into_parts(); + + let buffer = bits.as_host().clone(); + let bit_buffer = BitBuffer::new_with_offset(buffer, len, bool_offsets); + + let validity_mask = validity.to_mask(len); Ok(match_each_unsigned_integer_ptype!(ends.ptype(), |E| { runend_decode_typed_bool( - trimmed_ends_iter(ends.as_slice::(), offset, length), - &values.to_bit_buffer(), + trimmed_ends_iter(&ends.into_buffer::(), offset, length), + &bit_buffer, validity_mask, - values.dtype().nullability(), + nullability, length, ) })) diff --git a/encodings/sequence/src/compute/take.rs b/encodings/sequence/src/compute/take.rs index 437afa8eeae..75a3f7cee11 100644 --- a/encodings/sequence/src/compute/take.rs +++ b/encodings/sequence/src/compute/take.rs @@ -32,7 +32,7 @@ fn take_inner( mul: S, base: S, indices: &[T], - indices_mask: Mask, + indices_mask: &Mask, result_nullability: Nullability, len: usize, ) -> ArrayRef { @@ -92,7 +92,7 @@ impl TakeExecute for SequenceVTable { mul, base, indices, - mask, + &mask, result_nullability, array.len(), ))) diff --git a/encodings/sparse/src/canonical.rs b/encodings/sparse/src/canonical.rs index 7711719c3b0..d4c3febf46d 100644 --- a/encodings/sparse/src/canonical.rs +++ b/encodings/sparse/src/canonical.rs @@ -80,7 +80,7 @@ pub(super) fn execute_sparse( } DType::Struct(struct_fields, ..) => execute_sparse_struct( struct_fields, - array.fill_scalar().as_struct(), + &array.fill_scalar().as_struct(), array.dtype(), array.patches(), array.len(), @@ -104,11 +104,11 @@ pub(super) fn execute_sparse( dtype @ DType::Utf8(..) => { let fill_value = array.fill_scalar().as_utf8().value().cloned(); let fill_value = fill_value.map(BufferString::into_inner); - execute_varbin(array, dtype.clone(), fill_value, ctx)? + execute_varbin(array, dtype.clone(), &fill_value, ctx)? } dtype @ DType::Binary(..) => { let fill_value = array.fill_scalar().as_binary().value().cloned(); - execute_varbin(array, dtype.clone(), fill_value, ctx)? + execute_varbin(array, dtype.clone(), &fill_value, ctx)? } DType::List(values_dtype, nullability) => { execute_sparse_lists(array, values_dtype.clone(), *nullability, ctx)? @@ -151,12 +151,12 @@ fn execute_sparse_lists( match_smallest_offset_type!(total_canonical_values, |O| { execute_sparse_lists_inner::( indices.as_slice(), - values, - fill_value, + &values, + &fill_value, values_dtype, array.len(), total_canonical_values, - validity, + &validity, ) }) })) @@ -164,12 +164,12 @@ fn execute_sparse_lists( fn execute_sparse_lists_inner( patch_indices: &[I], - patch_values: ListViewArray, - fill_value: ListScalar, + patch_values: &ListViewArray, + fill_value: &ListScalar, values_dtype: Arc, len: usize, total_canonical_values: usize, - validity: Validity, + validity: &Validity, ) -> ArrayRef { // Create the builder with appropriate types. It is easy to just use the same type for both // `offsets` and `sizes` since we have no other constraints. @@ -235,8 +235,8 @@ fn execute_sparse_fixed_size_list( Ok(match_each_integer_ptype!(indices.ptype(), |I| { execute_sparse_fixed_size_list_inner::( indices.as_slice(), - values, - fill_value, + &values, + &fill_value, array.len(), validity, ) @@ -252,8 +252,8 @@ fn execute_sparse_fixed_size_list( /// elements without tracking offsets. fn execute_sparse_fixed_size_list_inner( indices: &[I], - values: FixedSizeListArray, - fill_value: ListScalar, + values: &FixedSizeListArray, + fill_value: &ListScalar, array_len: usize, validity: Validity, ) -> FixedSizeListArray { @@ -387,7 +387,7 @@ fn execute_sparse_primitives TryFrom<&'a Scalar, Error fn execute_sparse_struct( struct_fields: &StructFields, - fill_struct: StructScalar, + fill_struct: &StructScalar, dtype: &DType, // Resolution is unnecessary b/c we're just pushing the patches into the fields. unresolved_patches: &Patches, @@ -481,7 +481,7 @@ fn execute_sparse_decimal( fn execute_varbin( array: &SparseArray, dtype: DType, - fill_value: Option, + fill_value: &Option, ctx: &mut ExecutionCtx, ) -> VortexResult { let patches = array.resolved_patches()?; @@ -492,14 +492,14 @@ fn execute_varbin( Ok(match_each_integer_ptype!(indices.ptype(), |I| { let indices = indices.to_buffer::(); - execute_varbin_inner::(fill_value, indices, values, dtype, validity, len).into_array() + execute_varbin_inner::(fill_value, indices, &values, dtype, validity, len).into_array() })) } fn execute_varbin_inner( - fill_value: Option, + fill_value: &Option, indices: Buffer, - values: VarBinViewArray, + values: &VarBinViewArray, dtype: DType, validity: Validity, len: usize, @@ -509,7 +509,7 @@ fn execute_varbin_inner( let n_patch_buffers = values.buffers().len(); let mut buffers = values.buffers().to_vec(); - let fill = if let Some(buffer) = &fill_value { + let fill = if let Some(buffer) = fill_value { buffers.push(BufferHandle::new_host(buffer.clone())); BinaryView::make_view( buffer.as_ref(), diff --git a/encodings/sparse/src/compute/mod.rs b/encodings/sparse/src/compute/mod.rs index 85ef2f86eed..1546b71b0ff 100644 --- a/encodings/sparse/src/compute/mod.rs +++ b/encodings/sparse/src/compute/mod.rs @@ -90,7 +90,7 @@ mod test { #[rstest] fn test_sparse_binary_numeric(array: ArrayRef) { - test_binary_numeric_array(array) + test_binary_numeric_array(&array) } #[test] @@ -243,6 +243,6 @@ mod tests { Scalar::from(0i32) ).unwrap())] fn test_sparse_binary_numeric(#[case] array: SparseArray) { - test_binary_numeric_array(array.into_array()); + test_binary_numeric_array(&array.into_array()); } } diff --git a/encodings/zigzag/src/compute/mod.rs b/encodings/zigzag/src/compute/mod.rs index 06c8a416374..867c0a3c15e 100644 --- a/encodings/zigzag/src/compute/mod.rs +++ b/encodings/zigzag/src/compute/mod.rs @@ -229,6 +229,6 @@ mod tests { #[case::zigzag_i32_large(zigzag_encode(PrimitiveArray::from_iter((-50..50).map(|i| i * 10))).unwrap() )] fn test_zigzag_binary_numeric(#[case] array: ZigZagArray) { - test_binary_numeric_array(array.into_array()); + test_binary_numeric_array(&array.into_array()); } } diff --git a/encodings/zstd/benches/listview_rebuild.rs b/encodings/zstd/benches/listview_rebuild.rs index a15012e02ef..732dfba68f3 100644 --- a/encodings/zstd/benches/listview_rebuild.rs +++ b/encodings/zstd/benches/listview_rebuild.rs @@ -16,7 +16,7 @@ use vortex_zstd::ZstdArray; fn rebuild_naive(bencher: Bencher) { let dudes = VarBinViewArray::from_iter_str(["Washington", "Adams", "Jefferson", "Madison"]) .into_array(); - let dudes = ZstdArray::from_array(dudes, 9, 1024).unwrap().into_array(); + let dudes = ZstdArray::from_array(&dudes, 9, 1024).unwrap().into_array(); let offsets = std::iter::repeat_n(0u32, 1024) .collect::>() @@ -32,7 +32,7 @@ fn rebuild_naive(bencher: Bencher) { bencher .with_inputs(|| &list_view) - .bench_refs(|list_view| list_view.rebuild(ListViewRebuildMode::MakeZeroCopyToList)) + .bench_refs(|list_view| list_view.rebuild(&ListViewRebuildMode::MakeZeroCopyToList)) } fn main() { diff --git a/encodings/zstd/public-api.lock b/encodings/zstd/public-api.lock index 05a3767339d..af6bce59e17 100644 --- a/encodings/zstd/public-api.lock +++ b/encodings/zstd/public-api.lock @@ -6,7 +6,7 @@ impl vortex_zstd::ZstdArray pub fn vortex_zstd::ZstdArray::decompress(&self) -> vortex_error::VortexResult -pub fn vortex_zstd::ZstdArray::from_array(array: vortex_array::array::ArrayRef, level: i32, values_per_frame: usize) -> vortex_error::VortexResult +pub fn vortex_zstd::ZstdArray::from_array(array: &vortex_array::array::ArrayRef, level: i32, values_per_frame: usize) -> vortex_error::VortexResult pub fn vortex_zstd::ZstdArray::from_canonical(canonical: &vortex_array::canonical::Canonical, level: i32, values_per_frame: usize) -> vortex_error::VortexResult> diff --git a/encodings/zstd/src/array.rs b/encodings/zstd/src/array.rs index 64c606142f1..229782f4301 100644 --- a/encodings/zstd/src/array.rs +++ b/encodings/zstd/src/array.rs @@ -685,7 +685,7 @@ impl ZstdArray { } } - pub fn from_array(array: ArrayRef, level: i32, values_per_frame: usize) -> VortexResult { + pub fn from_array(array: &ArrayRef, level: i32, values_per_frame: usize) -> VortexResult { Self::from_canonical(&array.to_canonical()?, level, values_per_frame)? .ok_or_else(|| vortex_err!("Zstd can only encode Primitive and VarBinView arrays")) } diff --git a/fuzz/src/array/take.rs b/fuzz/src/array/take.rs index 8c963c2c066..e383cdaba7f 100644 --- a/fuzz/src/array/take.rs +++ b/fuzz/src/array/take.rs @@ -73,7 +73,7 @@ pub fn take_canonical_array(array: &ArrayRef, indices: &[Option]) -> Vort let primitive_array = array.to_primitive(); match_each_native_ptype!(p, |P| { Ok(take_primitive::

( - primitive_array, + &primitive_array, validity, indices_slice_non_opt, )) @@ -145,15 +145,16 @@ pub fn take_canonical_array(array: &ArrayRef, indices: &[Option]) -> Vort } fn take_primitive( - primitive_array: PrimitiveArray, + primitive_array: &PrimitiveArray, validity: Validity, indices: &[usize], ) -> ArrayRef { - let vec_values = primitive_array.as_slice::().to_vec(); + let vec_values = primitive_array.as_slice::(); PrimitiveArray::new( indices .iter() - .map(|i| vec_values[*i]) + .copied() + .map(|i| vec_values[i]) .collect::>(), validity, ) diff --git a/fuzz/src/lib.rs b/fuzz/src/lib.rs index 1d117e6d113..f7d48fa04b3 100644 --- a/fuzz/src/lib.rs +++ b/fuzz/src/lib.rs @@ -2,6 +2,7 @@ // SPDX-FileCopyrightText: Copyright the Vortex contributors #![allow(clippy::use_debug)] +#![allow(clippy::needless_pass_by_value)] mod array; pub mod compress; diff --git a/vortex-array/benches/listview_rebuild.rs b/vortex-array/benches/listview_rebuild.rs index f9e25e6b0d3..26eb6619e19 100644 --- a/vortex-array/benches/listview_rebuild.rs +++ b/vortex-array/benches/listview_rebuild.rs @@ -107,7 +107,9 @@ fn make_nested_list_lv( fn i32_small(bencher: Bencher) { let lv = make_primitive_lv(50, 32, 32); bencher.with_inputs(|| &lv).bench_refs(|lv| { - let rebuilt = lv.rebuild(ListViewRebuildMode::MakeZeroCopyToList).unwrap(); + let rebuilt = lv + .rebuild(&ListViewRebuildMode::MakeZeroCopyToList) + .unwrap(); rebuilt.elements().to_canonical().unwrap() }); } @@ -116,7 +118,9 @@ fn i32_small(bencher: Bencher) { fn i32_small_overlapping(bencher: Bencher) { let lv = make_primitive_lv(50, 8, 1); bencher.with_inputs(|| &lv).bench_refs(|lv| { - let rebuilt = lv.rebuild(ListViewRebuildMode::MakeZeroCopyToList).unwrap(); + let rebuilt = lv + .rebuild(&ListViewRebuildMode::MakeZeroCopyToList) + .unwrap(); rebuilt.elements().to_canonical().unwrap() }); } @@ -125,7 +129,9 @@ fn i32_small_overlapping(bencher: Bencher) { fn varbinview_small(bencher: Bencher) { let lv = make_varbinview_lv(50, 32, 32); bencher.with_inputs(|| &lv).bench_refs(|lv| { - let rebuilt = lv.rebuild(ListViewRebuildMode::MakeZeroCopyToList).unwrap(); + let rebuilt = lv + .rebuild(&ListViewRebuildMode::MakeZeroCopyToList) + .unwrap(); rebuilt.elements().to_canonical().unwrap() }); } @@ -134,7 +140,9 @@ fn varbinview_small(bencher: Bencher) { fn struct_small(bencher: Bencher) { let lv = make_struct_lv(50, 32, 32); bencher.with_inputs(|| &lv).bench_refs(|lv| { - let rebuilt = lv.rebuild(ListViewRebuildMode::MakeZeroCopyToList).unwrap(); + let rebuilt = lv + .rebuild(&ListViewRebuildMode::MakeZeroCopyToList) + .unwrap(); rebuilt.elements().to_canonical().unwrap() }); } @@ -143,7 +151,9 @@ fn struct_small(bencher: Bencher) { fn i32_large(bencher: Bencher) { let lv = make_primitive_lv(50, 1_024, 1_024); bencher.with_inputs(|| &lv).bench_refs(|lv| { - let rebuilt = lv.rebuild(ListViewRebuildMode::MakeZeroCopyToList).unwrap(); + let rebuilt = lv + .rebuild(&ListViewRebuildMode::MakeZeroCopyToList) + .unwrap(); rebuilt.elements().to_canonical().unwrap() }); } @@ -152,7 +162,9 @@ fn i32_large(bencher: Bencher) { fn varbinview_large(bencher: Bencher) { let lv = make_varbinview_lv(5, 1_024, 1_024); bencher.with_inputs(|| &lv).bench_refs(|lv| { - let rebuilt = lv.rebuild(ListViewRebuildMode::MakeZeroCopyToList).unwrap(); + let rebuilt = lv + .rebuild(&ListViewRebuildMode::MakeZeroCopyToList) + .unwrap(); rebuilt.elements().to_canonical().unwrap() }); } @@ -161,7 +173,9 @@ fn varbinview_large(bencher: Bencher) { fn struct_large(bencher: Bencher) { let lv = make_struct_lv(25, 1_024, 1_024); bencher.with_inputs(|| &lv).bench_refs(|lv| { - let rebuilt = lv.rebuild(ListViewRebuildMode::MakeZeroCopyToList).unwrap(); + let rebuilt = lv + .rebuild(&ListViewRebuildMode::MakeZeroCopyToList) + .unwrap(); rebuilt.elements().to_canonical().unwrap() }); } @@ -183,7 +197,9 @@ fn fsl_large(bencher: Bencher) { Validity::NonNullable, ); bencher.with_inputs(|| &lv).bench_refs(|lv| { - let rebuilt = lv.rebuild(ListViewRebuildMode::MakeZeroCopyToList).unwrap(); + let rebuilt = lv + .rebuild(&ListViewRebuildMode::MakeZeroCopyToList) + .unwrap(); rebuilt.elements().to_canonical().unwrap() }); } @@ -192,7 +208,9 @@ fn fsl_large(bencher: Bencher) { fn list_i32_large(bencher: Bencher) { let lv = make_nested_list_lv(2, 512, 2); bencher.with_inputs(|| &lv).bench_refs(|lv| { - let rebuilt = lv.rebuild(ListViewRebuildMode::MakeZeroCopyToList).unwrap(); + let rebuilt = lv + .rebuild(&ListViewRebuildMode::MakeZeroCopyToList) + .unwrap(); rebuilt.elements().to_canonical().unwrap() }); } diff --git a/vortex-array/benches/take_patches.rs b/vortex-array/benches/take_patches.rs index 31084c9a0a2..fee90e1bd23 100644 --- a/vortex-array/benches/take_patches.rs +++ b/vortex-array/benches/take_patches.rs @@ -49,7 +49,7 @@ fn take_search(bencher: Bencher, (patches_sparsity, index_multiple): (f64, f64)) bencher .with_inputs(|| (&patches, &indices, LEGACY_SESSION.create_execution_ctx())) .bench_refs(|(patches, indices, ctx)| { - patches.take_search(indices.to_primitive(), false, ctx) + patches.take_search(&indices.to_primitive(), false, ctx) }); } @@ -66,7 +66,7 @@ fn take_search_chunked(bencher: Bencher, (patches_sparsity, index_multiple): (f6 bencher .with_inputs(|| (&patches, &indices, LEGACY_SESSION.create_execution_ctx())) .bench_refs(|(patches, indices, ctx)| { - patches.take_search(indices.to_primitive(), false, ctx) + patches.take_search(&indices.to_primitive(), false, ctx) }); } @@ -82,7 +82,9 @@ fn take_map(bencher: Bencher, (patches_sparsity, index_multiple): (f64, f64)) { bencher .with_inputs(|| (&patches, &indices, LEGACY_SESSION.create_execution_ctx())) - .bench_refs(|(patches, indices, ctx)| patches.take_map(indices.to_primitive(), false, ctx)); + .bench_refs(|(patches, indices, ctx)| { + patches.take_map(&indices.to_primitive(), false, ctx) + }); } fn fixture(len: usize, sparsity: f64, rng: &mut StdRng) -> Patches { diff --git a/vortex-array/public-api.lock b/vortex-array/public-api.lock index 4e1817ab468..a050b38b662 100644 --- a/vortex-array/public-api.lock +++ b/vortex-array/public-api.lock @@ -2012,7 +2012,7 @@ pub unsafe fn vortex_array::arrays::ListViewArray::with_zero_copy_to_list(self, impl vortex_array::arrays::ListViewArray -pub fn vortex_array::arrays::ListViewArray::rebuild(&self, mode: vortex_array::arrays::ListViewRebuildMode) -> vortex_error::VortexResult +pub fn vortex_array::arrays::ListViewArray::rebuild(&self, mode: &vortex_array::arrays::ListViewRebuildMode) -> vortex_error::VortexResult impl core::clone::Clone for vortex_array::arrays::ListViewArray @@ -4096,9 +4096,9 @@ pub fn vortex_array::arrays::chunk_range(chunk_idx: usize, offset: usize, array_ pub fn vortex_array::arrays::compute_is_constant(values: &[T]) -> bool -pub fn vortex_array::arrays::list_from_list_view(list_view: vortex_array::arrays::ListViewArray) -> vortex_error::VortexResult +pub fn vortex_array::arrays::list_from_list_view(list_view: &vortex_array::arrays::ListViewArray) -> vortex_error::VortexResult -pub fn vortex_array::arrays::list_view_from_list(list: vortex_array::arrays::ListArray, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult +pub fn vortex_array::arrays::list_view_from_list(list: &vortex_array::arrays::ListArray, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult pub fn vortex_array::arrays::mask_validity_canonical(canonical: vortex_array::Canonical, validity_mask: &vortex_mask::Mask, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult @@ -5948,7 +5948,7 @@ pub vortex_array::compute::MinMaxResult::min: vortex_array::scalar::Scalar impl vortex_array::compute::MinMaxResult -pub fn vortex_array::compute::MinMaxResult::from_scalar(scalar: vortex_array::scalar::Scalar) -> vortex_error::VortexResult> +pub fn vortex_array::compute::MinMaxResult::from_scalar(scalar: &vortex_array::scalar::Scalar) -> vortex_error::VortexResult> impl core::clone::Clone for vortex_array::compute::MinMaxResult @@ -7980,6 +7980,10 @@ impl num_traits::cast::AsPrimitive for vortex_array::dtype::i256 pub fn vortex_array::dtype::i256::as_(self) -> i8 +impl num_traits::cast::AsPrimitive for bool + +pub fn bool::as_(self) -> vortex_array::dtype::i256 + impl num_traits::cast::AsPrimitive for i128 pub fn i128::as_(self) -> vortex_array::dtype::i256 @@ -11028,9 +11032,9 @@ pub fn vortex_array::patches::Patches::slice(&self, range: core::ops::range::Ran pub fn vortex_array::patches::Patches::take(&self, take_indices: &vortex_array::ArrayRef, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult> -pub fn vortex_array::patches::Patches::take_map(&self, take_indices: vortex_array::arrays::PrimitiveArray, include_nulls: bool, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult> +pub fn vortex_array::patches::Patches::take_map(&self, take_indices: &vortex_array::arrays::PrimitiveArray, include_nulls: bool, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult> -pub fn vortex_array::patches::Patches::take_search(&self, take_indices: vortex_array::arrays::PrimitiveArray, include_nulls: bool, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult> +pub fn vortex_array::patches::Patches::take_search(&self, take_indices: &vortex_array::arrays::PrimitiveArray, include_nulls: bool, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult> pub fn vortex_array::patches::Patches::take_with_nulls(&self, take_indices: &vortex_array::ArrayRef, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult> @@ -12012,6 +12016,8 @@ impl<'a> core::fmt::Debug for vortex_array::scalar::ExtScalar<'a> pub fn vortex_array::scalar::ExtScalar<'a>::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result +impl<'a> core::marker::Copy for vortex_array::scalar::ExtScalar<'a> + pub struct vortex_array::scalar::ListScalar<'a> impl<'a> vortex_array::scalar::ListScalar<'a> @@ -16212,6 +16218,12 @@ pub type vortex_array::stats::StatsSetRef<'_>::Target<'t> = vortex_flatbuffers:: pub fn vortex_array::stats::StatsSetRef<'_>::write_flatbuffer<'fb>(&self, fbb: &mut flatbuffers::builder::FlatBufferBuilder<'fb>) -> vortex_error::VortexResult> +impl<'a> core::clone::Clone for vortex_array::stats::StatsSetRef<'a> + +pub fn vortex_array::stats::StatsSetRef<'a>::clone(&self) -> vortex_array::stats::StatsSetRef<'a> + +impl<'a> core::marker::Copy for vortex_array::stats::StatsSetRef<'a> + pub struct vortex_array::stats::TypedStatsSetRef<'a, 'b> pub vortex_array::stats::TypedStatsSetRef::dtype: &'b vortex_array::dtype::DType @@ -17970,6 +17982,8 @@ impl<'a> core::fmt::Debug for vortex_array::CanonicalView<'a> pub fn vortex_array::CanonicalView<'a>::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result +impl<'a> core::marker::Copy for vortex_array::CanonicalView<'a> + pub enum vortex_array::Columnar pub vortex_array::Columnar::Canonical(vortex_array::Canonical) diff --git a/vortex-array/src/arrays/bool/compute/take.rs b/vortex-array/src/arrays/bool/compute/take.rs index 7d688ff75eb..9248df04750 100644 --- a/vortex-array/src/arrays/bool/compute/take.rs +++ b/vortex-array/src/arrays/bool/compute/take.rs @@ -56,13 +56,13 @@ fn take_valid_indices>(bools: &BitBuffer, indices: &[I]) - // the overhead to convert to a Vec. if bools.len() <= 4096 { let bools = bools.iter().collect_vec(); - take_byte_bool(bools, indices) + take_byte_bool(&bools, indices) } else { take_bool_impl(bools, indices) } } -fn take_byte_bool>(bools: Vec, indices: &[I]) -> BitBuffer { +fn take_byte_bool>(bools: &[bool], indices: &[I]) -> BitBuffer { BitBuffer::collect_bool(indices.len(), |idx| { bools[unsafe { indices.get_unchecked(idx).as_() }] }) diff --git a/vortex-array/src/arrays/chunked/compute/mod.rs b/vortex-array/src/arrays/chunked/compute/mod.rs index 7bb48a0262b..3175a4afb7a 100644 --- a/vortex-array/src/arrays/chunked/compute/mod.rs +++ b/vortex-array/src/arrays/chunked/compute/mod.rs @@ -157,6 +157,6 @@ mod tests { DType::Primitive(PType::I32, Nullability::NonNullable), ).unwrap())] fn test_chunked_binary_numeric(#[case] array: ChunkedArray) { - test_binary_numeric_array(array.into_array()) + test_binary_numeric_array(&array.into_array()) } } diff --git a/vortex-array/src/arrays/chunked/vtable/canonical.rs b/vortex-array/src/arrays/chunked/vtable/canonical.rs index a3884b05fde..5c6b3c6dedf 100644 --- a/vortex-array/src/arrays/chunked/vtable/canonical.rs +++ b/vortex-array/src/arrays/chunked/vtable/canonical.rs @@ -137,7 +137,7 @@ fn swizzle_list_chunks( let chunk_array = chunk.clone().execute::(ctx)?; // By rebuilding as zero-copy to `List` and trimming all elements (to prevent gaps), we make // the final output `ListView` also zero-copyable to `List`. - let chunk_array = chunk_array.rebuild(ListViewRebuildMode::MakeExact)?; + let chunk_array = chunk_array.rebuild(&ListViewRebuildMode::MakeExact)?; // Add the `elements` of the current array as a new chunk. list_elements_chunks.push(chunk_array.elements().clone()); diff --git a/vortex-array/src/arrays/constant/vtable/canonical.rs b/vortex-array/src/arrays/constant/vtable/canonical.rs index af8c5ee8e4f..c102a5b2d49 100644 --- a/vortex-array/src/arrays/constant/vtable/canonical.rs +++ b/vortex-array/src/arrays/constant/vtable/canonical.rs @@ -396,7 +396,7 @@ mod tests { ); let const_array = ConstantArray::new(list_scalar, 2).into_array(); let canonical_const = const_array.to_listview(); - let list_array = canonical_const.rebuild(ListViewRebuildMode::MakeZeroCopyToList)?; + let list_array = canonical_const.rebuild(&ListViewRebuildMode::MakeZeroCopyToList)?; assert_arrays_eq!( list_array.elements().to_primitive(), PrimitiveArray::from_iter([1u64, 2, 1, 2]) diff --git a/vortex-array/src/arrays/decimal/compute/between.rs b/vortex-array/src/arrays/decimal/compute/between.rs index 01127a99acf..f114fcbd966 100644 --- a/vortex-array/src/arrays/decimal/compute/between.rs +++ b/vortex-array/src/arrays/decimal/compute/between.rs @@ -40,15 +40,15 @@ impl BetweenKernel for DecimalVTable { arr.dtype.nullability() | lower.dtype().nullability() | upper.dtype().nullability(); match_each_decimal_value_type!(arr.values_type(), |D| { - between_unpack::(arr, lower, upper, nullability, options) + between_unpack::(arr, &lower, &upper, nullability, options) }) } } fn between_unpack( arr: &DecimalArray, - lower: Scalar, - upper: Scalar, + lower: &Scalar, + upper: &Scalar, nullability: Nullability, options: &BetweenOptions, ) -> VortexResult> { diff --git a/vortex-array/src/arrays/decimal/compute/cast.rs b/vortex-array/src/arrays/decimal/compute/cast.rs index cc0304ba8e3..a60e04f1088 100644 --- a/vortex-array/src/arrays/decimal/compute/cast.rs +++ b/vortex-array/src/arrays/decimal/compute/cast.rs @@ -125,7 +125,7 @@ pub fn upcast_decimal_values( match_each_decimal_value_type!(from_values_type, |F| { let from_buffer = array.buffer::(); match_each_decimal_value_type!(to_values_type, |T| { - let to_buffer = upcast_decimal_buffer::(from_buffer); + let to_buffer = upcast_decimal_buffer::(&from_buffer); Ok(DecimalArray::new(to_buffer, decimal_dtype, validity)) }) }) @@ -133,7 +133,9 @@ pub fn upcast_decimal_values( /// Upcast a buffer of decimal values from type F to type T. /// Since T is wider than F, this conversion never fails. -fn upcast_decimal_buffer(from: Buffer) -> Buffer { +fn upcast_decimal_buffer( + from: &Buffer, +) -> Buffer { from.iter() .map(|&v| T::from(v).vortex_expect("upcast should never fail")) .collect() diff --git a/vortex-array/src/arrays/decimal/compute/sum.rs b/vortex-array/src/arrays/decimal/compute/sum.rs index 94ee581dd60..a15cd40b7da 100644 --- a/vortex-array/src/arrays/decimal/compute/sum.rs +++ b/vortex-array/src/arrays/decimal/compute/sum.rs @@ -57,7 +57,7 @@ impl SumKernel for DecimalVTable { .vortex_expect("cannot fail to cast initial value"); Ok(sum_to_scalar( - array.buffer::(), + &array.buffer::(), validity, initial_val, return_decimal_dtype, @@ -73,7 +73,7 @@ impl SumKernel for DecimalVTable { /// Returns a null scalar if the sum overflows the underlying integer type or if the result /// exceeds the declared decimal precision. fn sum_to_scalar( - values: Buffer, + values: &Buffer, validity: Option<&BitBuffer>, initial: O, return_decimal_dtype: DecimalDType, @@ -98,7 +98,7 @@ where } fn sum_decimal, I: Copy + CheckedAdd + 'static>( - values: Buffer, + values: &[T], initial: I, ) -> Option { let mut sum = initial; @@ -109,11 +109,15 @@ fn sum_decimal, I: Copy + CheckedAdd + 'static>( Some(sum) } -fn sum_decimal_with_validity, I: Copy + CheckedAdd + 'static>( - values: Buffer, +fn sum_decimal_with_validity( + values: &Buffer, validity: &BitBuffer, initial: I, -) -> Option { +) -> Option +where + T: AsPrimitive, + I: Copy + CheckedAdd + 'static, +{ let mut sum = initial; for (v, valid) in values.iter().zip_eq(validity) { if valid { diff --git a/vortex-array/src/arrays/filter/execute/bool.rs b/vortex-array/src/arrays/filter/execute/bool.rs index 46dc2500905..dc33cfb34f6 100644 --- a/vortex-array/src/arrays/filter/execute/bool.rs +++ b/vortex-array/src/arrays/filter/execute/bool.rs @@ -12,7 +12,7 @@ use crate::arrays::filter::execute::filter_validity; pub fn filter_bool(array: &BoolArray, mask: &Arc) -> BoolArray { let validity = array.validity().vortex_expect("missing BoolArray validity"); - let filtered_validity = filter_validity(validity, mask); + let filtered_validity = filter_validity(&validity, mask); let bit_buffer = array.to_bit_buffer(); let filtered_buffer = bitbuffer::filter_bit_buffer(&bit_buffer, mask.as_ref()); diff --git a/vortex-array/src/arrays/filter/execute/decimal.rs b/vortex-array/src/arrays/filter/execute/decimal.rs index 46530123336..c3bdc2e091c 100644 --- a/vortex-array/src/arrays/filter/execute/decimal.rs +++ b/vortex-array/src/arrays/filter/execute/decimal.rs @@ -12,7 +12,7 @@ use crate::match_each_decimal_value_type; use crate::vtable::ValidityHelper; pub fn filter_decimal(array: &DecimalArray, mask: &Arc) -> DecimalArray { - let filtered_validity = filter_validity(array.validity().clone(), mask); + let filtered_validity = filter_validity(array.validity(), mask); match_each_decimal_value_type!(array.values_type(), |T| { let filtered_buffer = buffer::filter_buffer(array.buffer::(), mask.as_ref()); diff --git a/vortex-array/src/arrays/filter/execute/fixed_size_list.rs b/vortex-array/src/arrays/filter/execute/fixed_size_list.rs index 126cbc69fd7..e7037281140 100644 --- a/vortex-array/src/arrays/filter/execute/fixed_size_list.rs +++ b/vortex-array/src/arrays/filter/execute/fixed_size_list.rs @@ -25,7 +25,7 @@ pub fn filter_fixed_size_list( array: &FixedSizeListArray, selection_mask: &Arc, ) -> FixedSizeListArray { - let filtered_validity = filter_validity(array.validity().clone(), selection_mask); + let filtered_validity = filter_validity(array.validity(), selection_mask); let elements = array.elements(); let new_len = selection_mask.true_count(); diff --git a/vortex-array/src/arrays/filter/execute/listview.rs b/vortex-array/src/arrays/filter/execute/listview.rs index 88fa205822c..97a2d67ac3e 100644 --- a/vortex-array/src/arrays/filter/execute/listview.rs +++ b/vortex-array/src/arrays/filter/execute/listview.rs @@ -41,7 +41,7 @@ pub fn filter_listview(array: &ListViewArray, selection_mask: &Arc) let offsets = array.offsets(); let sizes = array.sizes(); - let new_validity = filter_validity(array.validity().clone(), selection_mask); + let new_validity = filter_validity(array.validity(), selection_mask); debug_assert!( new_validity .maybe_len() @@ -70,7 +70,7 @@ pub fn filter_listview(array: &ListViewArray, selection_mask: &Arc) // right now, so we will just rebuild every time (similar to `ListArray`). new_array - .rebuild(ListViewRebuildMode::MakeZeroCopyToList) + .rebuild(&ListViewRebuildMode::MakeZeroCopyToList) .vortex_expect("ListViewArray rebuild to zero-copy List should always succeed") } diff --git a/vortex-array/src/arrays/filter/execute/mod.rs b/vortex-array/src/arrays/filter/execute/mod.rs index 0863e6f0cc7..228d39862d1 100644 --- a/vortex-array/src/arrays/filter/execute/mod.rs +++ b/vortex-array/src/arrays/filter/execute/mod.rs @@ -40,7 +40,7 @@ fn values_to_mask(values: &Arc) -> Mask { } /// A helper function that lazily filters a [`Validity`] with selection mask values. -fn filter_validity(validity: Validity, mask: &Arc) -> Validity { +fn filter_validity(validity: &Validity, mask: &Arc) -> Validity { validity .filter(&values_to_mask(mask)) .vortex_expect("Somehow unable to wrap filter around a validity array") diff --git a/vortex-array/src/arrays/filter/execute/primitive.rs b/vortex-array/src/arrays/filter/execute/primitive.rs index 32ad8ec5c9a..b14f86c8dbf 100644 --- a/vortex-array/src/arrays/filter/execute/primitive.rs +++ b/vortex-array/src/arrays/filter/execute/primitive.rs @@ -15,7 +15,7 @@ pub fn filter_primitive(array: &PrimitiveArray, mask: &Arc) -> Primi let validity = array .validity() .vortex_expect("missing PrimitiveArray validity"); - let filtered_validity = filter_validity(validity, mask); + let filtered_validity = filter_validity(&validity, mask); match_each_native_ptype!(array.ptype(), |T| { let filtered_buffer = buffer::filter_buffer(array.to_buffer::(), mask.as_ref()); diff --git a/vortex-array/src/arrays/filter/execute/struct_.rs b/vortex-array/src/arrays/filter/execute/struct_.rs index b0cd7701703..2124b788ce1 100644 --- a/vortex-array/src/arrays/filter/execute/struct_.rs +++ b/vortex-array/src/arrays/filter/execute/struct_.rs @@ -13,7 +13,7 @@ use crate::arrays::filter::execute::values_to_mask; use crate::vtable::ValidityHelper; pub fn filter_struct(array: &StructArray, mask: &Arc) -> StructArray { - let filtered_validity = filter_validity(array.validity().clone(), mask); + let filtered_validity = filter_validity(array.validity(), mask); let mask_for_filter = values_to_mask(mask); let fields: Vec = array diff --git a/vortex-array/src/arrays/list/test_harness.rs b/vortex-array/src/arrays/list/test_harness.rs index 7ced1eb878f..cfa34d3f51c 100644 --- a/vortex-array/src/arrays/list/test_harness.rs +++ b/vortex-array/src/arrays/list/test_harness.rs @@ -20,7 +20,7 @@ impl ListArray { /// appended to the array. pub fn from_iter_slow( iter: I, - dtype: Arc, + dtype: &Arc, ) -> VortexResult where I::Item: IntoIterator, @@ -47,7 +47,7 @@ impl ListArray { pub fn from_iter_opt_slow>, T>( iter: I, - dtype: Arc, + dtype: &Arc, ) -> VortexResult where T: IntoIterator, diff --git a/vortex-array/src/arrays/list/tests.rs b/vortex-array/src/arrays/list/tests.rs index 9d63372ba6b..99e12729528 100644 --- a/vortex-array/src/arrays/list/tests.rs +++ b/vortex-array/src/arrays/list/tests.rs @@ -76,7 +76,7 @@ fn test_simple_list_array_from_iter() { let list = ListArray::try_new(elements, offsets, validity).unwrap(); let list_from_iter = - ListArray::from_iter_slow::(vec![vec![1i32, 2], vec![3]], Arc::new(I32.into())) + ListArray::from_iter_slow::(vec![vec![1i32, 2], vec![3]], &Arc::new(I32.into())) .unwrap(); assert_eq!(list.len(), list_from_iter.len()); @@ -444,6 +444,7 @@ type OptVec = Vec>; // Helper function to create a list of lists from a 3D vector with Option types. #[allow(clippy::cast_possible_truncation)] +#[allow(clippy::needless_pass_by_value)] fn create_list_of_lists_nullable(data: OptVec>>) -> ListArray { // Flatten all elements and track offsets and validity. let mut all_elements = Vec::new(); diff --git a/vortex-array/src/arrays/list/vtable/mod.rs b/vortex-array/src/arrays/list/vtable/mod.rs index 99c052565b2..596ffd654b6 100644 --- a/vortex-array/src/arrays/list/vtable/mod.rs +++ b/vortex-array/src/arrays/list/vtable/mod.rs @@ -211,7 +211,7 @@ impl VTable for ListVTable { } fn execute(array: &Self::Array, ctx: &mut ExecutionCtx) -> VortexResult { - Ok(list_view_from_list(array.clone(), ctx)?.into_array()) + Ok(list_view_from_list(array, ctx)?.into_array()) } fn execute_parent( diff --git a/vortex-array/src/arrays/listview/array.rs b/vortex-array/src/arrays/listview/array.rs index 2d43320bd95..7a4ec8c7172 100644 --- a/vortex-array/src/arrays/listview/array.rs +++ b/vortex-array/src/arrays/listview/array.rs @@ -306,8 +306,8 @@ impl ListViewArray { if cfg!(debug_assertions) && is_zctl { validate_zctl( &self.elements, - self.offsets.to_primitive(), - self.sizes.to_primitive(), + &self.offsets.to_primitive(), + &self.sizes.to_primitive(), ) .vortex_expect("Failed to validate zero-copy to list flag"); } @@ -335,8 +335,8 @@ impl ListViewArray { pub fn verify_is_zero_copy_to_list(&self) -> bool { validate_zctl( &self.elements, - self.offsets.to_primitive(), - self.sizes.to_primitive(), + &self.offsets.to_primitive(), + &self.sizes.to_primitive(), ) .is_ok() } @@ -495,8 +495,8 @@ where /// [`ListArray`](crate::arrays::ListArray). fn validate_zctl( elements: &ArrayRef, - offsets_primitive: PrimitiveArray, - sizes_primitive: PrimitiveArray, + offsets_primitive: &PrimitiveArray, + sizes_primitive: &PrimitiveArray, ) -> VortexResult<()> { // Offsets must be sorted (but not strictly sorted, zero-length lists are allowed), even // if there are null views. @@ -559,8 +559,8 @@ fn validate_zctl( fn count_references( element_references: &mut [u8], - offsets_primitive: PrimitiveArray, - sizes_primitive: PrimitiveArray, + offsets_primitive: &PrimitiveArray, + sizes_primitive: &PrimitiveArray, ) { let offsets_slice = offsets_primitive.as_slice::(); let sizes_slice = sizes_primitive.as_slice::(); diff --git a/vortex-array/src/arrays/listview/compute/take.rs b/vortex-array/src/arrays/listview/compute/take.rs index af1eac44e55..5aa376a6d20 100644 --- a/vortex-array/src/arrays/listview/compute/take.rs +++ b/vortex-array/src/arrays/listview/compute/take.rs @@ -81,7 +81,7 @@ impl TakeReduce for ListViewVTable { Ok(Some( new_array - .rebuild(ListViewRebuildMode::MakeZeroCopyToList)? + .rebuild(&ListViewRebuildMode::MakeZeroCopyToList)? .into_array(), )) } diff --git a/vortex-array/src/arrays/listview/conversion.rs b/vortex-array/src/arrays/listview/conversion.rs index fb8a45161cd..654326e2fbc 100644 --- a/vortex-array/src/arrays/listview/conversion.rs +++ b/vortex-array/src/arrays/listview/conversion.rs @@ -29,7 +29,10 @@ use crate::vtable::ValidityHelper; /// /// The output [`ListViewArray`] will be zero-copyable back to a [`ListArray`], and additionally it /// will not have any leading or trailing garbage data. -pub fn list_view_from_list(list: ListArray, ctx: &mut ExecutionCtx) -> VortexResult { +pub fn list_view_from_list( + list: &ListArray, + ctx: &mut ExecutionCtx, +) -> VortexResult { // If the list is empty, create an empty `ListViewArray` with the same offset `DType` as the // input. if list.is_empty() { @@ -106,9 +109,9 @@ fn build_sizes_from_offsets( /// /// Otherwise, this function fall back to the (very) expensive path and will rebuild the /// [`ListArray`] from scratch. -pub fn list_from_list_view(list_view: ListViewArray) -> VortexResult { +pub fn list_from_list_view(list_view: &ListViewArray) -> VortexResult { // Rebuild as zero-copyable to list array and also trim all leading and trailing elements. - let zctl_array = list_view.rebuild(ListViewRebuildMode::MakeExact)?; + let zctl_array = list_view.rebuild(&ListViewRebuildMode::MakeExact)?; debug_assert!(zctl_array.is_zero_copy_to_list()); let list_offsets = match_each_integer_ptype!(zctl_array.offsets().dtype().as_ptype(), |O| { @@ -212,7 +215,7 @@ pub fn recursive_list_from_list_view(array: ArrayRef) -> VortexResult }; // Make the conversion to `ListArray`. - let list_array = list_from_list_view(listview_with_converted_elements)?; + let list_array = list_from_list_view(&listview_with_converted_elements)?; list_array.into_array() } Canonical::FixedSizeList(fixed_size_list) => { @@ -311,7 +314,7 @@ mod tests { ListArray::try_new(elements.clone(), offsets.clone(), Validity::NonNullable).unwrap(); let mut ctx = LEGACY_SESSION.create_execution_ctx(); - let list_view = list_view_from_list(list_array.clone(), &mut ctx)?; + let list_view = list_view_from_list(&list_array, &mut ctx)?; // Verify structure. assert_eq!(list_view.len(), 4); @@ -333,7 +336,7 @@ mod tests { #[test] fn test_listview_to_list_zero_copy() -> VortexResult<()> { let list_view = create_basic_listview(); - let list_array = list_from_list_view(list_view.clone())?; + let list_array = list_from_list_view(&list_view)?; // Should have same elements. assert_arrays_eq!(list_view.elements().clone(), list_array.elements().clone()); @@ -360,11 +363,11 @@ mod tests { // This conversion will create an empty ListViewArray. // Note: list_view_from_list handles the empty case specially. let mut ctx = LEGACY_SESSION.create_execution_ctx(); - let empty_list_view = list_view_from_list(empty_list.clone(), &mut ctx)?; + let empty_list_view = list_view_from_list(&empty_list, &mut ctx)?; assert_eq!(empty_list_view.len(), 0); // Convert back. - let converted_back = list_from_list_view(empty_list_view)?; + let converted_back = list_from_list_view(&empty_list_view)?; assert_eq!(converted_back.len(), 0); // For empty arrays, we can't use assert_arrays_eq directly since the offsets might differ. // Just check that it's empty. @@ -382,14 +385,14 @@ mod tests { ListArray::try_new(elements.clone(), offsets.clone(), validity.clone()).unwrap(); let mut ctx = LEGACY_SESSION.create_execution_ctx(); - let nullable_list_view = list_view_from_list(nullable_list.clone(), &mut ctx)?; + let nullable_list_view = list_view_from_list(&nullable_list, &mut ctx)?; // Verify validity is preserved. assert_eq!(nullable_list_view.validity(), &validity); assert_eq!(nullable_list_view.len(), 3); // Round-trip conversion. - let converted_back = list_from_list_view(nullable_list_view)?; + let converted_back = list_from_list_view(&nullable_list_view)?; assert_arrays_eq!(nullable_list, converted_back); Ok(()) } @@ -398,7 +401,7 @@ mod tests { fn test_non_zero_copy_listview_to_list() -> VortexResult<()> { // Create ListViewArray with overlapping lists (not zero-copyable). let list_view = create_overlapping_listview(); - let list_array = list_from_list_view(list_view.clone())?; + let list_array = list_from_list_view(&list_view)?; // The resulting ListArray should have monotonic offsets. for i in 0..list_array.len() { @@ -417,7 +420,7 @@ mod tests { let empty_lists_view = create_empty_lists_listview(); // Convert to ListArray. - let list_array = list_from_list_view(empty_lists_view.clone())?; + let list_array = list_from_list_view(&empty_lists_view)?; assert_eq!(list_array.len(), 4); // All sublists should be empty. @@ -427,7 +430,7 @@ mod tests { // Round-trip. let mut ctx = LEGACY_SESSION.create_execution_ctx(); - let converted_back = list_view_from_list(list_array, &mut ctx)?; + let converted_back = list_view_from_list(&list_array, &mut ctx)?; assert_arrays_eq!(empty_lists_view, converted_back); Ok(()) } @@ -442,7 +445,7 @@ mod tests { .unwrap(); let mut ctx = LEGACY_SESSION.create_execution_ctx(); - let list_view_i32 = list_view_from_list(list_i32.clone(), &mut ctx)?; + let list_view_i32 = list_view_from_list(&list_i32, &mut ctx)?; assert_eq!(list_view_i32.offsets().dtype(), i32_offsets.dtype()); assert_eq!(list_view_i32.sizes().dtype(), i32_offsets.dtype()); @@ -452,7 +455,7 @@ mod tests { ListArray::try_new(elements.clone(), i64_offsets.clone(), Validity::NonNullable) .unwrap(); - let list_view_i64 = list_view_from_list(list_i64.clone(), &mut ctx)?; + let list_view_i64 = list_view_from_list(&list_i64, &mut ctx)?; assert_eq!(list_view_i64.offsets().dtype(), i64_offsets.dtype()); assert_eq!(list_view_i64.sizes().dtype(), i64_offsets.dtype()); @@ -468,21 +471,21 @@ mod tests { // Test 1: Basic round-trip. let original = create_basic_listview(); - let to_list = list_from_list_view(original.clone())?; - let back_to_view = list_view_from_list(to_list, &mut ctx)?; + let to_list = list_from_list_view(&original)?; + let back_to_view = list_view_from_list(&to_list, &mut ctx)?; assert_arrays_eq!(original, back_to_view); // Test 2: Nullable round-trip. let nullable = create_nullable_listview(); - let nullable_to_list = list_from_list_view(nullable.clone())?; - let nullable_back = list_view_from_list(nullable_to_list, &mut ctx)?; + let nullable_to_list = list_from_list_view(&nullable)?; + let nullable_back = list_view_from_list(&nullable_to_list, &mut ctx)?; assert_arrays_eq!(nullable, nullable_back); // Test 3: Non-zero-copyable round-trip. let overlapping = create_overlapping_listview(); - let overlapping_to_list = list_from_list_view(overlapping.clone())?; - let overlapping_back = list_view_from_list(overlapping_to_list, &mut ctx)?; + let overlapping_to_list = list_from_list_view(&overlapping)?; + let overlapping_back = list_view_from_list(&overlapping_to_list, &mut ctx)?; assert_arrays_eq!(overlapping, overlapping_back); Ok(()) } @@ -496,7 +499,7 @@ mod tests { ListArray::try_new(elements.clone(), offsets, Validity::NonNullable).unwrap(); let mut ctx = LEGACY_SESSION.create_execution_ctx(); - let list_view = list_view_from_list(single_elem_list.clone(), &mut ctx)?; + let list_view = list_view_from_list(&single_elem_list, &mut ctx)?; assert_eq!(list_view.len(), 3); // Verify sizes are all 1. @@ -504,7 +507,7 @@ mod tests { assert_arrays_eq!(expected_sizes, list_view.sizes().clone()); // Round-trip. - let converted_back = list_from_list_view(list_view)?; + let converted_back = list_from_list_view(&list_view)?; assert_arrays_eq!(single_elem_list, converted_back); Ok(()) } @@ -518,7 +521,7 @@ mod tests { ListArray::try_new(elements.clone(), offsets.clone(), Validity::NonNullable).unwrap(); let mut ctx = LEGACY_SESSION.create_execution_ctx(); - let list_view = list_view_from_list(mixed_list.clone(), &mut ctx)?; + let list_view = list_view_from_list(&mixed_list, &mut ctx)?; assert_eq!(list_view.len(), 5); // Verify sizes. @@ -526,7 +529,7 @@ mod tests { assert_arrays_eq!(expected_sizes, list_view.sizes().clone()); // Round-trip. - let converted_back = list_from_list_view(list_view)?; + let converted_back = list_from_list_view(&list_view)?; assert_arrays_eq!(mixed_list, converted_back); Ok(()) } @@ -693,7 +696,7 @@ mod tests { #[test] fn test_recursive_mixed_listview_and_list() -> VortexResult<()> { let listview = create_basic_listview(); - let list = list_from_list_view(listview.clone())?; + let list = list_from_list_view(&listview)?; let struct_array = StructArray::try_new( FieldNames::from(["listview_field", "list_field"]), diff --git a/vortex-array/src/arrays/listview/rebuild.rs b/vortex-array/src/arrays/listview/rebuild.rs index 957e474ef62..f590e930366 100644 --- a/vortex-array/src/arrays/listview/rebuild.rs +++ b/vortex-array/src/arrays/listview/rebuild.rs @@ -55,7 +55,7 @@ pub enum ListViewRebuildMode { impl ListViewArray { /// Rebuilds the [`ListViewArray`] according to the specified mode. - pub fn rebuild(&self, mode: ListViewRebuildMode) -> VortexResult { + pub fn rebuild(&self, mode: &ListViewRebuildMode) -> VortexResult { if self.is_empty() { return Ok(self.clone()); } @@ -393,7 +393,7 @@ mod tests { let listview = ListViewArray::new(elements, offsets, sizes, Validity::NonNullable); - let flattened = listview.rebuild(ListViewRebuildMode::MakeZeroCopyToList)?; + let flattened = listview.rebuild(&ListViewRebuildMode::MakeZeroCopyToList)?; // After flatten: elements should be [A, B, C, B, C] = [1, 2, 3, 2, 3] // Lists should be sequential with no overlaps @@ -436,7 +436,7 @@ mod tests { let listview = ListViewArray::new(elements, offsets, sizes, validity); - let flattened = listview.rebuild(ListViewRebuildMode::MakeZeroCopyToList)?; + let flattened = listview.rebuild(&ListViewRebuildMode::MakeZeroCopyToList)?; // Verify nullability is preserved assert_eq!(flattened.dtype().nullability(), Nullability::Nullable); @@ -473,7 +473,7 @@ mod tests { let listview = ListViewArray::new(elements, offsets, sizes, Validity::NonNullable); - let trimmed = listview.rebuild(ListViewRebuildMode::TrimElements)?; + let trimmed = listview.rebuild(&ListViewRebuildMode::TrimElements)?; // After trimming: elements should be [A, B, _, C, D] = [1, 2, 97, 3, 4]. assert_eq!(trimmed.elements().len(), 5); @@ -516,7 +516,7 @@ mod tests { let listview = ListViewArray::new(elements, offsets, sizes, validity); // First rebuild to make it zero-copy-to-list - let rebuilt = listview.rebuild(ListViewRebuildMode::MakeZeroCopyToList)?; + let rebuilt = listview.rebuild(&ListViewRebuildMode::MakeZeroCopyToList)?; assert!(rebuilt.is_zero_copy_to_list()); // Verify NULL items have correct offsets (should not reuse previous offsets) @@ -534,7 +534,7 @@ mod tests { // Now rebuild with MakeExact (which calls naive_rebuild then trim_elements) // This should not panic (issue #5412) - let exact = rebuilt.rebuild(ListViewRebuildMode::MakeExact)?; + let exact = rebuilt.rebuild(&ListViewRebuildMode::MakeExact)?; // Verify the result is still valid assert!(exact.is_valid(0).unwrap()); @@ -569,7 +569,7 @@ mod tests { let sizes = PrimitiveArray::from_iter(vec![2u16, 2]).into_array(); let listview = ListViewArray::new(elements, offsets, sizes, Validity::NonNullable); - let trimmed = listview.rebuild(ListViewRebuildMode::TrimElements)?; + let trimmed = listview.rebuild(&ListViewRebuildMode::TrimElements)?; assert_arrays_eq!( trimmed.list_elements_at(1).unwrap(), PrimitiveArray::from_iter([30i32, 40]) @@ -589,7 +589,7 @@ mod tests { let sizes = PrimitiveArray::from_iter(vec![70_000u32, 2]).into_array(); let listview = ListViewArray::new(elements, offsets, sizes, Validity::NonNullable); - let trimmed = listview.rebuild(ListViewRebuildMode::TrimElements)?; + let trimmed = listview.rebuild(&ListViewRebuildMode::TrimElements)?; assert_arrays_eq!( trimmed.list_elements_at(1).unwrap(), PrimitiveArray::from_iter([30i32, 40]) diff --git a/vortex-array/src/arrays/listview/tests/basic.rs b/vortex-array/src/arrays/listview/tests/basic.rs index 09dce5797bf..6d1f310d147 100644 --- a/vortex-array/src/arrays/listview/tests/basic.rs +++ b/vortex-array/src/arrays/listview/tests/basic.rs @@ -127,7 +127,7 @@ fn test_from_list_array() -> VortexResult<()> { let list_array = ListArray::try_new(elements, offsets, validity).unwrap(); let mut ctx = LEGACY_SESSION.create_execution_ctx(); - let list_view = list_view_from_list(list_array, &mut ctx)?; + let list_view = list_view_from_list(&list_array, &mut ctx)?; assert_eq!(list_view.len(), 3); diff --git a/vortex-array/src/arrays/masked/execute.rs b/vortex-array/src/arrays/masked/execute.rs index c6c0fc64348..210e4807c6f 100644 --- a/vortex-array/src/arrays/masked/execute.rs +++ b/vortex-array/src/arrays/masked/execute.rs @@ -11,17 +11,22 @@ use vortex_mask::Mask; use crate::Canonical; use crate::IntoArray; use crate::arrays::BoolArray; +use crate::arrays::BoolArrayParts; use crate::arrays::DecimalArray; +use crate::arrays::DecimalArrayParts; use crate::arrays::ExtensionArray; use crate::arrays::FixedSizeListArray; use crate::arrays::ListViewArray; +use crate::arrays::ListViewArrayParts; use crate::arrays::NullArray; use crate::arrays::PrimitiveArray; +use crate::arrays::PrimitiveArrayParts; use crate::arrays::StructArray; +use crate::arrays::StructArrayParts; use crate::arrays::VarBinViewArray; +use crate::arrays::VarBinViewArrayParts; use crate::dtype::Nullability; use crate::executor::ExecutionCtx; -use crate::match_each_decimal_value_type; use crate::validity::Validity; use crate::vtable::ValidityHelper; @@ -49,7 +54,7 @@ pub fn mask_validity_canonical( } Canonical::Struct(a) => Canonical::Struct(mask_validity_struct(a, validity_mask)), Canonical::Extension(a) => { - Canonical::Extension(mask_validity_extension(a, validity_mask, ctx)?) + Canonical::Extension(mask_validity_extension(&a, validity_mask, ctx)?) } }) } @@ -65,88 +70,90 @@ fn mask_validity_null(array: NullArray, _mask: &Mask) -> NullArray { } fn mask_validity_bool(array: BoolArray, mask: &Mask) -> BoolArray { - let len = array.len(); - let new_validity = combine_validity(array.validity(), mask, len); - BoolArray::new(array.to_bit_buffer(), new_validity) + let new_validity = combine_validity(array.validity(), mask, array.len()); + let BoolArrayParts { + bits, + offset, + len, + validity: _, + } = array.into_parts(); + BoolArray::new_handle(bits, offset, len, new_validity) } fn mask_validity_primitive(array: PrimitiveArray, mask: &Mask) -> PrimitiveArray { - let len = array.len(); - let ptype = array.ptype(); - let new_validity = combine_validity(array.validity(), mask, len); + let new_validity = combine_validity(array.validity(), mask, array.len()); + let PrimitiveArrayParts { + ptype, + buffer, + validity: _, + } = array.into_parts(); // SAFETY: validity has same length as values - unsafe { - PrimitiveArray::new_unchecked_from_handle( - array.buffer_handle().clone(), - ptype, - new_validity, - ) - } + unsafe { PrimitiveArray::new_unchecked_from_handle(buffer, ptype, new_validity) } } fn mask_validity_decimal(array: DecimalArray, mask: &Mask) -> DecimalArray { - let len = array.len(); - let dec_dtype = array.decimal_dtype(); - let values_type = array.values_type(); - let new_validity = combine_validity(array.validity(), mask, len); + let new_validity = combine_validity(array.validity(), mask, array.len()); + let DecimalArrayParts { + decimal_dtype, + values, + values_type, + validity: _, + } = array.into_parts(); // SAFETY: We're only changing validity, not the data structure - match_each_decimal_value_type!(values_type, |T| { - let buffer = array.buffer::(); - unsafe { DecimalArray::new_unchecked(buffer, dec_dtype, new_validity) } - }) + unsafe { DecimalArray::new_unchecked_handle(values, values_type, decimal_dtype, new_validity) } } /// Mask validity for VarBinViewArray. fn mask_validity_varbinview(array: VarBinViewArray, mask: &Mask) -> VarBinViewArray { - let len = array.len(); - let dtype = array.dtype().as_nullable(); - let new_validity = combine_validity(array.validity(), mask, len); + let new_validity = combine_validity(array.validity(), mask, array.len()); + let VarBinViewArrayParts { + dtype, + buffers, + views, + validity: _, + } = array.into_parts(); // SAFETY: We're only changing validity, not the data structure unsafe { - VarBinViewArray::new_handle_unchecked( - array.views_handle().clone(), - array.buffers().clone(), - dtype, - new_validity, - ) + VarBinViewArray::new_handle_unchecked(views, buffers, dtype.as_nullable(), new_validity) } } fn mask_validity_listview(array: ListViewArray, mask: &Mask) -> ListViewArray { - let len = array.len(); - let new_validity = combine_validity(array.validity(), mask, len); + let new_validity = combine_validity(array.validity(), mask, array.len()); + let ListViewArrayParts { + elements_dtype: _, + elements, + offsets, + sizes, + validity: _, + } = array.into_parts(); // SAFETY: We're only changing validity, not the data structure - unsafe { - ListViewArray::new_unchecked( - array.elements().clone(), - array.offsets().clone(), - array.sizes().clone(), - new_validity, - ) - } + unsafe { ListViewArray::new_unchecked(elements, offsets, sizes, new_validity) } } fn mask_validity_fixed_size_list(array: FixedSizeListArray, mask: &Mask) -> FixedSizeListArray { let len = array.len(); let list_size = array.list_size(); let new_validity = combine_validity(array.validity(), mask, len); + let (elements, ..) = array.into_parts(); // SAFETY: We're only changing validity, not the data structure - unsafe { - FixedSizeListArray::new_unchecked(array.elements().clone(), list_size, new_validity, len) - } + unsafe { FixedSizeListArray::new_unchecked(elements, list_size, new_validity, len) } } fn mask_validity_struct(array: StructArray, mask: &Mask) -> StructArray { let len = array.len(); let new_validity = combine_validity(array.validity(), mask, len); - let fields = array.unmasked_fields().clone(); - let struct_fields = array.struct_fields().clone(); + let StructArrayParts { + struct_fields, + fields, + validity: _, + } = array.into_parts(); // SAFETY: We're only changing validity, not the data structure unsafe { StructArray::new_unchecked(fields, struct_fields, len, new_validity) } } fn mask_validity_extension( - array: ExtensionArray, + array: &ExtensionArray, mask: &Mask, ctx: &mut ExecutionCtx, ) -> VortexResult { diff --git a/vortex-array/src/arrays/primitive/array/patch.rs b/vortex-array/src/arrays/primitive/array/patch.rs index 5c43c095830..544b014a9f9 100644 --- a/vortex-array/src/arrays/primitive/array/patch.rs +++ b/vortex-array/src/arrays/primitive/array/patch.rs @@ -32,9 +32,9 @@ impl PrimitiveArray { Ok(match_each_integer_ptype!(patch_indices.ptype(), |I| { match_each_native_ptype!(self.ptype(), |T| { self.patch_typed::( - patch_indices, + &patch_indices, patches.offset(), - patch_values, + &patch_values, patched_validity, ) }) @@ -43,9 +43,9 @@ impl PrimitiveArray { fn patch_typed( self, - patch_indices: PrimitiveArray, + patch_indices: &PrimitiveArray, patch_indices_offset: usize, - patch_values: PrimitiveArray, + patch_values: &PrimitiveArray, patched_validity: Validity, ) -> Self where diff --git a/vortex-array/src/arrays/primitive/array/top_value.rs b/vortex-array/src/arrays/primitive/array/top_value.rs index d85975f0325..fded7008d03 100644 --- a/vortex-array/src/arrays/primitive/array/top_value.rs +++ b/vortex-array/src/arrays/primitive/array/top_value.rs @@ -28,13 +28,13 @@ impl PrimitiveArray { } match_each_native_ptype!(self.ptype(), |P| { - let (top, count) = typed_top_value(self.as_slice::

(), self.validity_mask()?); + let (top, count) = typed_top_value(self.as_slice::

(), &self.validity_mask()?); Ok(Some((top.into(), count))) }) } } -fn typed_top_value(values: &[T], mask: Mask) -> (T, usize) +fn typed_top_value(values: &[T], mask: &Mask) -> (T, usize) where T: NativePType, NativeValue: Eq + Hash, diff --git a/vortex-array/src/arrays/primitive/compute/cast.rs b/vortex-array/src/arrays/primitive/compute/cast.rs index 1ee863d112f..fc88c5731f1 100644 --- a/vortex-array/src/arrays/primitive/compute/cast.rs +++ b/vortex-array/src/arrays/primitive/compute/cast.rs @@ -54,14 +54,14 @@ impl CastKernel for PrimitiveVTable { // Otherwise, we need to cast the values one-by-one Ok(Some(match_each_native_ptype!(new_ptype, |T| { match_each_native_ptype!(array.ptype(), |F| { - PrimitiveArray::new(cast::(array.as_slice(), mask)?, new_validity) + PrimitiveArray::new(cast::(array.as_slice(), &mask)?, new_validity) .into_array() }) }))) } } -fn cast(array: &[F], mask: Mask) -> VortexResult> { +fn cast(array: &[F], mask: &Mask) -> VortexResult> { match mask.bit_buffer() { AllOr::All => { let mut buffer = BufferMut::with_capacity(array.len()); diff --git a/vortex-array/src/arrays/primitive/compute/mod.rs b/vortex-array/src/arrays/primitive/compute/mod.rs index 7465adb1db9..e6b03f5cbd5 100644 --- a/vortex-array/src/arrays/primitive/compute/mod.rs +++ b/vortex-array/src/arrays/primitive/compute/mod.rs @@ -53,6 +53,6 @@ mod tests { #[case::f64(PrimitiveArray::from_iter([10.1f64, 20.2, 30.3, 40.4, 50.5]))] fn test_primitive_binary_numeric(#[case] array: PrimitiveArray) { use crate::compute::conformance::binary_numeric::test_binary_numeric_array; - test_binary_numeric_array(array.into_array()); + test_binary_numeric_array(&array.into_array()); } } diff --git a/vortex-array/src/arrays/varbin/compute/compare.rs b/vortex-array/src/arrays/varbin/compute/compare.rs index b05b97023f0..0d877b43649 100644 --- a/vortex-array/src/arrays/varbin/compute/compare.rs +++ b/vortex-array/src/arrays/varbin/compute/compare.rs @@ -61,13 +61,13 @@ impl CompareKernel for VarBinVTable { CompareOperator::Eq | CompareOperator::Lte => { let lhs_offsets = lhs.offsets().clone().execute::(ctx)?; match_each_integer_ptype!(lhs_offsets.ptype(), |P| { - compare_offsets_to_empty::

(lhs_offsets, true) + compare_offsets_to_empty::

(&lhs_offsets, true) }) } CompareOperator::NotEq | CompareOperator::Gt => { let lhs_offsets = lhs.offsets().clone().execute::(ctx)?; match_each_integer_ptype!(lhs_offsets.ptype(), |P| { - compare_offsets_to_empty::

(lhs_offsets, false) + compare_offsets_to_empty::

(&lhs_offsets, false) }) } }; @@ -131,7 +131,7 @@ impl CompareKernel for VarBinVTable { } } -fn compare_offsets_to_empty(offsets: PrimitiveArray, eq: bool) -> BitBuffer { +fn compare_offsets_to_empty(offsets: &PrimitiveArray, eq: bool) -> BitBuffer { let fn_ = if eq { P::eq } else { P::ne }; let offsets = offsets.as_slice::

(); BitBuffer::collect_bool(offsets.len() - 1, |idx| { diff --git a/vortex-array/src/arrays/varbin/compute/filter.rs b/vortex-array/src/arrays/varbin/compute/filter.rs index 648f3c32666..1647c6825c6 100644 --- a/vortex-array/src/arrays/varbin/compute/filter.rs +++ b/vortex-array/src/arrays/varbin/compute/filter.rs @@ -67,7 +67,7 @@ fn filter_select_var_bin_by_slice( offsets.as_slice::(), values.bytes().as_slice(), mask_slices, - values.validity_mask()?, + &values.validity_mask()?, selection_count, ) }) @@ -78,7 +78,7 @@ fn filter_select_var_bin_by_slice_primitive_offset( offsets: &[O], data: &[u8], mask_slices: &[(usize, usize)], - logical_validity: Mask, + logical_validity: &Mask, selection_count: usize, ) -> VortexResult where @@ -166,7 +166,7 @@ fn filter_select_var_bin_by_index( offsets.as_slice::(), values.bytes().as_slice(), mask_indices, - values.validity().clone(), + values.validity(), selection_count, ) }) @@ -178,7 +178,7 @@ fn filter_select_var_bin_by_index_primitive_offset( data: &[u8], mask_indices: &[usize], // TODO(ngates): pass LogicalValidity instead - validity: Validity, + validity: &Validity, selection_count: usize, ) -> VortexResult { let mut builder = VarBinBuilder::::with_capacity(selection_count); diff --git a/vortex-array/src/arrays/varbin/compute/take.rs b/vortex-array/src/arrays/varbin/compute/take.rs index 851b4a4ce54..b8cc3a492c4 100644 --- a/vortex-array/src/arrays/varbin/compute/take.rs +++ b/vortex-array/src/arrays/varbin/compute/take.rs @@ -47,64 +47,64 @@ impl TakeExecute for VarBinVTable { offsets.as_slice::(), data.as_slice(), indices.as_slice::(), - array_validity, - indices_validity, + &array_validity, + &indices_validity, ), PType::U16 => take::( dtype, offsets.as_slice::(), data.as_slice(), indices.as_slice::(), - array_validity, - indices_validity, + &array_validity, + &indices_validity, ), PType::U32 => take::( dtype, offsets.as_slice::(), data.as_slice(), indices.as_slice::(), - array_validity, - indices_validity, + &array_validity, + &indices_validity, ), PType::U64 => take::( dtype, offsets.as_slice::(), data.as_slice(), indices.as_slice::(), - array_validity, - indices_validity, + &array_validity, + &indices_validity, ), PType::I8 => take::( dtype, offsets.as_slice::(), data.as_slice(), indices.as_slice::(), - array_validity, - indices_validity, + &array_validity, + &indices_validity, ), PType::I16 => take::( dtype, offsets.as_slice::(), data.as_slice(), indices.as_slice::(), - array_validity, - indices_validity, + &array_validity, + &indices_validity, ), PType::I32 => take::( dtype, offsets.as_slice::(), data.as_slice(), indices.as_slice::(), - array_validity, - indices_validity, + &array_validity, + &indices_validity, ), PType::I64 => take::( dtype, offsets.as_slice::(), data.as_slice(), indices.as_slice::(), - array_validity, - indices_validity, + &array_validity, + &indices_validity, ), _ => unreachable!("invalid PType for offsets"), } @@ -119,8 +119,8 @@ fn take( offsets: &[Offset], data: &[u8], indices: &[Index], - validity_mask: Mask, - indices_validity_mask: Mask, + validity_mask: &Mask, + indices_validity_mask: &Mask, ) -> VortexResult { if !validity_mask.all_true() || !indices_validity_mask.all_true() { return Ok(take_nullable::( @@ -182,8 +182,8 @@ fn take_nullable VarBinArray { let mut new_offsets = BufferMut::::with_capacity(indices.len() + 1); new_offsets.push(NewOffset::zero()); diff --git a/vortex-array/src/arrays/varbinview/array.rs b/vortex-array/src/arrays/varbinview/array.rs index 97377fa972e..c7323009157 100644 --- a/vortex-array/src/arrays/varbinview/array.rs +++ b/vortex-array/src/arrays/varbinview/array.rs @@ -212,6 +212,7 @@ impl VarBinViewArray { /// /// - The validity must have the same nullability as the dtype. /// - If validity is an array, its length must match `views.len()`. + #[allow(clippy::needless_pass_by_value)] pub unsafe fn new_unchecked( views: Buffer, buffers: Arc<[ByteBuffer]>, diff --git a/vortex-array/src/arrow/executor/byte_view.rs b/vortex-array/src/arrow/executor/byte_view.rs index 0e6b4923325..7384c4cfe5a 100644 --- a/vortex-array/src/arrow/executor/byte_view.rs +++ b/vortex-array/src/arrow/executor/byte_view.rs @@ -59,7 +59,7 @@ pub fn execute_varbinview_to_arrow( } pub(super) fn to_arrow_byte_view( - array: ArrayRef, + array: &ArrayRef, ctx: &mut ExecutionCtx, ) -> VortexResult { // First we cast the array into the desired ByteView type. diff --git a/vortex-array/src/arrow/executor/decimal.rs b/vortex-array/src/arrow/executor/decimal.rs index b99d245e269..da35b603adc 100644 --- a/vortex-array/src/arrow/executor/decimal.rs +++ b/vortex-array/src/arrow/executor/decimal.rs @@ -32,15 +32,15 @@ pub(super) fn to_arrow_decimal( let decimal_array = array.execute::(ctx)?; match data_type { - DataType::Decimal32(..) => to_arrow_decimal32(decimal_array), - DataType::Decimal64(..) => to_arrow_decimal64(decimal_array), - DataType::Decimal128(..) => to_arrow_decimal128(decimal_array), - DataType::Decimal256(..) => to_arrow_decimal256(decimal_array), + DataType::Decimal32(..) => to_arrow_decimal32(&decimal_array), + DataType::Decimal64(..) => to_arrow_decimal64(&decimal_array), + DataType::Decimal128(..) => to_arrow_decimal128(&decimal_array), + DataType::Decimal256(..) => to_arrow_decimal256(&decimal_array), _ => unreachable!("to_arrow_decimal called with non-decimal type"), } } -fn to_arrow_decimal32(array: DecimalArray) -> VortexResult { +fn to_arrow_decimal32(array: &DecimalArray) -> VortexResult { let null_buffer = to_null_buffer(array.validity_mask()?); let buffer: Buffer = match array.values_type() { DecimalType::I8 => { @@ -84,7 +84,7 @@ fn to_arrow_decimal32(array: DecimalArray) -> VortexResult { )) } -fn to_arrow_decimal64(array: DecimalArray) -> VortexResult { +fn to_arrow_decimal64(array: &DecimalArray) -> VortexResult { let null_buffer = to_null_buffer(array.validity_mask()?); let buffer: Buffer = match array.values_type() { DecimalType::I8 => { @@ -123,7 +123,7 @@ fn to_arrow_decimal64(array: DecimalArray) -> VortexResult { )) } -fn to_arrow_decimal128(array: DecimalArray) -> VortexResult { +fn to_arrow_decimal128(array: &DecimalArray) -> VortexResult { let null_buffer = to_null_buffer(array.validity_mask()?); let buffer: Buffer = match array.values_type() { DecimalType::I8 => { @@ -157,7 +157,7 @@ fn to_arrow_decimal128(array: DecimalArray) -> VortexResult { )) } -fn to_arrow_decimal256(array: DecimalArray) -> VortexResult { +fn to_arrow_decimal256(array: &DecimalArray) -> VortexResult { let null_buffer = to_null_buffer(array.validity_mask()?); let buffer: Buffer = match array.values_type() { DecimalType::I8 => { diff --git a/vortex-array/src/arrow/executor/dictionary.rs b/vortex-array/src/arrow/executor/dictionary.rs index 78e7f981824..9055e4124ab 100644 --- a/vortex-array/src/arrow/executor/dictionary.rs +++ b/vortex-array/src/arrow/executor/dictionary.rs @@ -35,7 +35,7 @@ pub(super) fn to_arrow_dictionary( Err(array) => array, }; let array = match array.try_into::() { - Ok(constant) => return constant_to_dict(constant, codes_type, values_type, ctx), + Ok(constant) => return constant_to_dict(&constant, codes_type, values_type, ctx), Err(array) => array, }; @@ -51,7 +51,7 @@ pub(super) fn to_arrow_dictionary( /// Convert a constant array to a dictionary with a single entry. fn constant_to_dict( - array: ConstantArray, + array: &ConstantArray, codes_type: &DataType, values_type: &DataType, ctx: &mut ExecutionCtx, @@ -68,7 +68,7 @@ fn constant_to_dict( .into_array() .execute_arrow(Some(values_type), ctx)?; let codes = zeroed_codes_array(codes_type, len)?; - make_dict_array(codes_type, codes, values) + make_dict_array(codes_type, &codes, &values) } /// Convert a Vortex dictionary array to an Arrow dictionary array. @@ -81,7 +81,7 @@ fn dict_to_dict( let DictArrayParts { codes, values, .. } = array.into_parts(); let codes = codes.execute_arrow(Some(codes_type), ctx)?; let values = values.execute_arrow(Some(values_type), ctx)?; - make_dict_array(codes_type, codes, values) + make_dict_array(codes_type, &codes, &values) } /// Construct a zeroed Arrow primitive array directly. @@ -102,33 +102,54 @@ fn zeroed_codes_array(codes_type: &DataType, len: usize) -> VortexResult VortexResult { Ok(match codes_type { DataType::Int8 => Arc::new(unsafe { - DictionaryArray::new_unchecked(codes.as_primitive::().clone(), values) + DictionaryArray::new_unchecked(codes.as_primitive::().clone(), values.clone()) }), DataType::Int16 => Arc::new(unsafe { - DictionaryArray::new_unchecked(codes.as_primitive::().clone(), values) + DictionaryArray::new_unchecked( + codes.as_primitive::().clone(), + values.clone(), + ) }), DataType::Int32 => Arc::new(unsafe { - DictionaryArray::new_unchecked(codes.as_primitive::().clone(), values) + DictionaryArray::new_unchecked( + codes.as_primitive::().clone(), + values.clone(), + ) }), DataType::Int64 => Arc::new(unsafe { - DictionaryArray::new_unchecked(codes.as_primitive::().clone(), values) + DictionaryArray::new_unchecked( + codes.as_primitive::().clone(), + values.clone(), + ) }), DataType::UInt8 => Arc::new(unsafe { - DictionaryArray::new_unchecked(codes.as_primitive::().clone(), values) + DictionaryArray::new_unchecked( + codes.as_primitive::().clone(), + values.clone(), + ) }), DataType::UInt16 => Arc::new(unsafe { - DictionaryArray::new_unchecked(codes.as_primitive::().clone(), values) + DictionaryArray::new_unchecked( + codes.as_primitive::().clone(), + values.clone(), + ) }), DataType::UInt32 => Arc::new(unsafe { - DictionaryArray::new_unchecked(codes.as_primitive::().clone(), values) + DictionaryArray::new_unchecked( + codes.as_primitive::().clone(), + values.clone(), + ) }), DataType::UInt64 => Arc::new(unsafe { - DictionaryArray::new_unchecked(codes.as_primitive::().clone(), values) + DictionaryArray::new_unchecked( + codes.as_primitive::().clone(), + values.clone(), + ) }), _ => vortex_bail!("Unsupported dictionary codes type: {:?}", codes_type), }) diff --git a/vortex-array/src/arrow/executor/list.rs b/vortex-array/src/arrow/executor/list.rs index 944ab4d3e19..3fea331c89b 100644 --- a/vortex-array/src/arrow/executor/list.rs +++ b/vortex-array/src/arrow/executor/list.rs @@ -48,7 +48,7 @@ pub(super) fn to_arrow_list( let zctl = if array.is_zero_copy_to_list() { array } else { - array.rebuild(ListViewRebuildMode::MakeZeroCopyToList)? + array.rebuild(&ListViewRebuildMode::MakeZeroCopyToList)? }; return list_view_zctl::(zctl, elements_field, ctx); } @@ -63,7 +63,7 @@ pub(super) fn to_arrow_list( let zctl = if list_view.is_zero_copy_to_list() { list_view } else { - list_view.rebuild(ListViewRebuildMode::MakeZeroCopyToList)? + list_view.rebuild(&ListViewRebuildMode::MakeZeroCopyToList)? }; list_view_zctl::(zctl, elements_field, ctx) } diff --git a/vortex-array/src/arrow/executor/mod.rs b/vortex-array/src/arrow/executor/mod.rs index abb55efb97f..9f4db871a70 100644 --- a/vortex-array/src/arrow/executor/mod.rs +++ b/vortex-array/src/arrow/executor/mod.rs @@ -98,17 +98,17 @@ impl ArrowArrayExecutor for ArrayRef { let arrow = match &resolved_type { DataType::Null => to_arrow_null(self, ctx), DataType::Boolean => to_arrow_bool(self, ctx), - DataType::Int8 => to_arrow_primitive::(self, ctx), - DataType::Int16 => to_arrow_primitive::(self, ctx), - DataType::Int32 => to_arrow_primitive::(self, ctx), - DataType::Int64 => to_arrow_primitive::(self, ctx), - DataType::UInt8 => to_arrow_primitive::(self, ctx), - DataType::UInt16 => to_arrow_primitive::(self, ctx), - DataType::UInt32 => to_arrow_primitive::(self, ctx), - DataType::UInt64 => to_arrow_primitive::(self, ctx), - DataType::Float16 => to_arrow_primitive::(self, ctx), - DataType::Float32 => to_arrow_primitive::(self, ctx), - DataType::Float64 => to_arrow_primitive::(self, ctx), + DataType::Int8 => to_arrow_primitive::(&self, ctx), + DataType::Int16 => to_arrow_primitive::(&self, ctx), + DataType::Int32 => to_arrow_primitive::(&self, ctx), + DataType::Int64 => to_arrow_primitive::(&self, ctx), + DataType::UInt8 => to_arrow_primitive::(&self, ctx), + DataType::UInt16 => to_arrow_primitive::(&self, ctx), + DataType::UInt32 => to_arrow_primitive::(&self, ctx), + DataType::UInt64 => to_arrow_primitive::(&self, ctx), + DataType::Float16 => to_arrow_primitive::(&self, ctx), + DataType::Float32 => to_arrow_primitive::(&self, ctx), + DataType::Float64 => to_arrow_primitive::(&self, ctx), DataType::Timestamp(..) | DataType::Date32 | DataType::Date64 @@ -118,8 +118,8 @@ impl ArrowArrayExecutor for ArrayRef { DataType::LargeBinary => to_arrow_byte_array::(self, ctx), DataType::Utf8 => to_arrow_byte_array::(self, ctx), DataType::LargeUtf8 => to_arrow_byte_array::(self, ctx), - DataType::BinaryView => to_arrow_byte_view::(self, ctx), - DataType::Utf8View => to_arrow_byte_view::(self, ctx), + DataType::BinaryView => to_arrow_byte_view::(&self, ctx), + DataType::Utf8View => to_arrow_byte_view::(&self, ctx), // TODO(joe): pass down preferred DataType::List(elements_field) => to_arrow_list::(self, elements_field, ctx), // TODO(joe): pass down preferred diff --git a/vortex-array/src/arrow/executor/primitive.rs b/vortex-array/src/arrow/executor/primitive.rs index 7910ed5fa31..9901b0ea9e4 100644 --- a/vortex-array/src/arrow/executor/primitive.rs +++ b/vortex-array/src/arrow/executor/primitive.rs @@ -31,7 +31,7 @@ where } pub(super) fn to_arrow_primitive( - array: ArrayRef, + array: &ArrayRef, ctx: &mut ExecutionCtx, ) -> VortexResult where diff --git a/vortex-array/src/arrow/executor/run_end.rs b/vortex-array/src/arrow/executor/run_end.rs index 2cc853b475f..4c03f31ea03 100644 --- a/vortex-array/src/arrow/executor/run_end.rs +++ b/vortex-array/src/arrow/executor/run_end.rs @@ -50,7 +50,7 @@ pub(super) fn to_arrow_run_end( ) -> VortexResult { let array = match array.try_into::() { Ok(constant) => { - return constant_to_run_end(constant, ends_type, values_type, ctx); + return constant_to_run_end(&constant, ends_type, values_type, ctx); } Err(array) => array, }; @@ -62,7 +62,7 @@ pub(super) fn to_arrow_run_end( // NOTE(ngates): while this module still lives in vortex-array, we cannot depend on the // vortex-runend crate. Therefore, we match on the encoding ID string and extract the children // and metadata directly. - return run_end_to_arrow(array, ends_type, values_type, ctx); + return run_end_to_arrow(&array, ends_type, values_type, ctx); } // Fallback: canonicalize to flat Arrow, then cast to REE. @@ -75,7 +75,7 @@ pub(super) fn to_arrow_run_end( } fn run_end_to_arrow( - array: ArrayRef, + array: &ArrayRef, ends_type: &DataType, values_type: &Field, ctx: &mut ExecutionCtx, @@ -138,7 +138,7 @@ where /// Convert a constant array to a run-end encoded array with a single run. fn constant_to_run_end( - array: ConstantArray, + array: &ConstantArray, ends_type: &DataType, values_type: &Field, ctx: &mut ExecutionCtx, diff --git a/vortex-array/src/builders/fixed_size_list.rs b/vortex-array/src/builders/fixed_size_list.rs index 3ce90ad339e..deae399c584 100644 --- a/vortex-array/src/builders/fixed_size_list.rs +++ b/vortex-array/src/builders/fixed_size_list.rs @@ -107,6 +107,7 @@ impl FixedSizeListBuilder { /// fixed-size list arrays without accompanying metadata). /// /// [`ListArray`]: crate::arrays::ListArray + #[allow(clippy::needless_pass_by_value)] pub fn append_value(&mut self, value: ListScalar) -> VortexResult<()> { let Some(elements) = value.elements() else { // If `elements` is `None`, then the `value` is a null value. diff --git a/vortex-array/src/builders/list.rs b/vortex-array/src/builders/list.rs index 643402b6555..c2ba0f5b588 100644 --- a/vortex-array/src/builders/list.rs +++ b/vortex-array/src/builders/list.rs @@ -110,6 +110,7 @@ impl ListBuilder { } /// Appends a list `value` to the builder. + #[allow(clippy::needless_pass_by_value)] pub fn append_value(&mut self, value: ListScalar) -> VortexResult<()> { match value.elements() { None => { @@ -426,7 +427,7 @@ mod tests { fn test_extend_builder_gen() { let list = ListArray::from_iter_opt_slow::( [Some(vec![0, 1, 2]), None, Some(vec![4, 5])], - Arc::new(I32.into()), + &Arc::new(I32.into()), ) .unwrap(); assert_eq!(list.len(), 3); @@ -449,7 +450,7 @@ mod tests { None, Some(vec![4, 5]), ], - Arc::new(DType::Primitive(I32, NonNullable)), + &Arc::new(DType::Primitive(I32, NonNullable)), ) .unwrap() .to_listview(); diff --git a/vortex-array/src/builders/listview.rs b/vortex-array/src/builders/listview.rs index dc3509c55d9..a7b708492e8 100644 --- a/vortex-array/src/builders/listview.rs +++ b/vortex-array/src/builders/listview.rs @@ -151,6 +151,7 @@ impl ListViewBuilder { /// /// This method extends the value builder with the provided values and records /// the offset and size of the new list. + #[allow(clippy::needless_pass_by_value)] pub fn append_value(&mut self, value: ListScalar) -> VortexResult<()> { let Some(elements) = value.elements() else { // If `elements` is `None`, then the `value` is a null value. @@ -314,7 +315,7 @@ impl ArrayBuilder for ListViewBuilder { // Otherwise, after removing any leading and trailing elements, we can simply bulk append // the entire array. let listview = listview - .rebuild(ListViewRebuildMode::MakeExact) + .rebuild(&ListViewRebuildMode::MakeExact) .vortex_expect("ListViewArray::rebuild(MakeExact) failed in extend_from_array"); debug_assert!(listview.is_zero_copy_to_list()); @@ -357,7 +358,7 @@ impl ArrayBuilder for ListViewBuilder { match_each_integer_ptype!(new_offsets.ptype(), |A| { adjust_and_extend_offsets::( uninit_range, - new_offsets, + &new_offsets, old_elements_len, new_elements_len, ); @@ -389,7 +390,7 @@ impl ArrayBuilder for ListViewBuilder { /// offset. fn adjust_and_extend_offsets<'a, O: IntegerPType, A: IntegerPType>( mut uninit_range: UninitRange<'a, O>, - new_offsets: PrimitiveArray, + new_offsets: &PrimitiveArray, old_elements_len: usize, new_elements_len: usize, ) { @@ -615,7 +616,7 @@ mod tests { // Create a source ListArray. let source = ListArray::from_iter_opt_slow::>( [Some(vec![1, 2, 3]), None, Some(vec![4, 5])], - Arc::new(I32.into()), + &Arc::new(I32.into()), ) .unwrap(); @@ -632,7 +633,7 @@ mod tests { // Extend from empty array (should be no-op). let empty_source = ListArray::from_iter_opt_slow::>( std::iter::empty::>>(), - Arc::new(I32.into()), + &Arc::new(I32.into()), ) .unwrap(); builder.extend_from_array(&empty_source.into_array()); diff --git a/vortex-array/src/builders/struct_.rs b/vortex-array/src/builders/struct_.rs index 30353dbf8ae..3af8c38f022 100644 --- a/vortex-array/src/builders/struct_.rs +++ b/vortex-array/src/builders/struct_.rs @@ -58,6 +58,7 @@ impl StructBuilder { } /// Appends a struct `value` to the builder. + #[allow(clippy::needless_pass_by_value)] pub fn append_value(&mut self, struct_scalar: StructScalar) -> VortexResult<()> { if !self.dtype.is_nullable() && struct_scalar.is_null() { vortex_bail!("Tried to append a null `StructScalar` to a non-nullable struct builder",); diff --git a/vortex-array/src/canonical.rs b/vortex-array/src/canonical.rs index 169891f9748..5dd1bc1670f 100644 --- a/vortex-array/src/canonical.rs +++ b/vortex-array/src/canonical.rs @@ -171,7 +171,7 @@ impl Canonical { match self { Canonical::VarBinView(array) => Ok(Canonical::VarBinView(array.compact_buffers()?)), Canonical::List(array) => Ok(Canonical::List( - array.rebuild(ListViewRebuildMode::TrimElements)?, + array.rebuild(&ListViewRebuildMode::TrimElements)?, )), _ => Ok(self.clone()), } @@ -839,7 +839,7 @@ impl Executable for StructArray { } /// A view into a canonical array type. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Copy)] pub enum CanonicalView<'a> { Null(&'a NullArray), Bool(&'a BoolArray), diff --git a/vortex-array/src/compute/conformance/binary_numeric.rs b/vortex-array/src/compute/conformance/binary_numeric.rs index 1f4061f5cd9..b7c32ada095 100644 --- a/vortex-array/src/compute/conformance/binary_numeric.rs +++ b/vortex-array/src/compute/conformance/binary_numeric.rs @@ -78,18 +78,18 @@ fn to_vec_of_scalar(array: &ArrayRef) -> Vec { /// Panics if: /// - The array cannot be converted to primitive form /// - Results don't match expected values (for operations that don't overflow) -fn test_binary_numeric_conformance(array: ArrayRef) +fn test_binary_numeric_conformance(array: &ArrayRef) where Scalar: From, { // First test with the standard scalar value of 1 - test_standard_binary_numeric::(array.clone()); + test_standard_binary_numeric::(array); // Then test edge cases test_binary_numeric_edge_cases(array); } -fn test_standard_binary_numeric(array: ArrayRef) +fn test_standard_binary_numeric(array: &ArrayRef) where Scalar: From, { @@ -214,7 +214,7 @@ where /// test_binary_numeric_array(array.into_array()); /// } /// ``` -pub fn test_binary_numeric_array(array: ArrayRef) { +pub fn test_binary_numeric_array(array: &ArrayRef) { match array.dtype() { DType::Primitive(ptype, _) => match ptype { PType::I8 => test_binary_numeric_conformance::(array), @@ -245,22 +245,22 @@ pub fn test_binary_numeric_array(array: ArrayRef) { /// - Negative one (tests signed arithmetic) /// - Maximum value (tests overflow behavior) /// - Minimum value (tests underflow behavior) -fn test_binary_numeric_edge_cases(array: ArrayRef) { +fn test_binary_numeric_edge_cases(array: &ArrayRef) { match array.dtype() { DType::Primitive(ptype, _) => match ptype { - PType::I8 => test_binary_numeric_edge_cases_signed::(array), - PType::I16 => test_binary_numeric_edge_cases_signed::(array), - PType::I32 => test_binary_numeric_edge_cases_signed::(array), - PType::I64 => test_binary_numeric_edge_cases_signed::(array), - PType::U8 => test_binary_numeric_edge_cases_unsigned::(array), - PType::U16 => test_binary_numeric_edge_cases_unsigned::(array), - PType::U32 => test_binary_numeric_edge_cases_unsigned::(array), - PType::U64 => test_binary_numeric_edge_cases_unsigned::(array), + PType::I8 => test_binary_numeric_edge_cases_signed::(array.clone()), + PType::I16 => test_binary_numeric_edge_cases_signed::(array.clone()), + PType::I32 => test_binary_numeric_edge_cases_signed::(array.clone()), + PType::I64 => test_binary_numeric_edge_cases_signed::(array.clone()), + PType::U8 => test_binary_numeric_edge_cases_unsigned::(array.clone()), + PType::U16 => test_binary_numeric_edge_cases_unsigned::(array.clone()), + PType::U32 => test_binary_numeric_edge_cases_unsigned::(array.clone()), + PType::U64 => test_binary_numeric_edge_cases_unsigned::(array.clone()), PType::F16 => { eprintln!("Skipping f16 edge case tests (not supported)"); } - PType::F32 => test_binary_numeric_edge_cases_float::(array), - PType::F64 => test_binary_numeric_edge_cases_float::(array), + PType::F32 => test_binary_numeric_edge_cases_float::(array.clone()), + PType::F64 => test_binary_numeric_edge_cases_float::(array.clone()), }, dtype => vortex_panic!( "Binary numeric edge case tests are only supported for primitive numeric types, got {dtype}" @@ -327,6 +327,7 @@ where test_binary_numeric_with_scalar(array, T::neg_infinity()); } +#[allow(clippy::needless_pass_by_value)] fn test_binary_numeric_with_scalar(array: ArrayRef, scalar_value: T) where T: NativePType + Num + Copy + std::fmt::Debug, diff --git a/vortex-array/src/compute/min_max.rs b/vortex-array/src/compute/min_max.rs index d789834707c..42c1eba8696 100644 --- a/vortex-array/src/compute/min_max.rs +++ b/vortex-array/src/compute/min_max.rs @@ -54,7 +54,7 @@ pub fn min_max(array: &ArrayRef) -> VortexResult> { options: &(), })? .unwrap_scalar()?; - MinMaxResult::from_scalar(scalar) + MinMaxResult::from_scalar(&scalar) } #[derive(Debug, Clone, PartialEq, Eq)] @@ -64,7 +64,7 @@ pub struct MinMaxResult { } impl MinMaxResult { - pub fn from_scalar(scalar: Scalar) -> VortexResult> { + pub fn from_scalar(scalar: &Scalar) -> VortexResult> { if scalar.is_null() { Ok(None) } else { @@ -184,7 +184,7 @@ fn min_max_impl( }; for kernel in kernels { if let Some(output) = kernel.invoke(&args)? { - return MinMaxResult::from_scalar(output.unwrap_scalar()?); + return MinMaxResult::from_scalar(&output.unwrap_scalar()?); } } diff --git a/vortex-array/src/display/tree.rs b/vortex-array/src/display/tree.rs index b2fd2c569da..a10d06eb40e 100644 --- a/vortex-array/src/display/tree.rs +++ b/vortex-array/src/display/tree.rs @@ -140,7 +140,7 @@ impl fmt::Display for TreeDisplayWrapper { metadata, stats, }; - array_fmt.format("root", array) + array_fmt.format("root", &array) } } @@ -154,7 +154,7 @@ pub struct TreeFormatter<'a, 'b: 'a> { } impl<'a, 'b: 'a> TreeFormatter<'a, 'b> { - fn format(&mut self, name: &str, array: ArrayRef) -> fmt::Result { + fn format(&mut self, name: &str, array: &ArrayRef) -> fmt::Result { if self.stats { let nbytes = array.nbytes(); let total_size = self @@ -253,7 +253,7 @@ impl<'a, 'b: 'a> TreeFormatter<'a, 'b> { .into_iter() .zip(array.children().into_iter()) { - i.format(&name, child)?; + i.format(&name, &child)?; } Ok(()) })?; diff --git a/vortex-array/src/dtype/bigint/bigcast.rs b/vortex-array/src/dtype/bigint/bigcast.rs index 0abc55efefb..c4316768d16 100644 --- a/vortex-array/src/dtype/bigint/bigcast.rs +++ b/vortex-array/src/dtype/bigint/bigcast.rs @@ -1,6 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: Copyright the Vortex contributors +use num_traits::AsPrimitive; use num_traits::ToPrimitive; use crate::dtype::i256; @@ -51,6 +52,12 @@ impl ToI256 for i256 { } } +impl AsPrimitive for bool { + fn as_(self) -> i256 { + if self { i256::ONE } else { i256::ZERO } + } +} + /// Checked numeric casts up to and including i256 support. /// /// This is meant as a more inclusive version of `NumCast` from the `num-traits` crate. diff --git a/vortex-array/src/dtype/serde/flatbuffers.rs b/vortex-array/src/dtype/serde/flatbuffers.rs index 2cd12f5dc77..ecd25476948 100644 --- a/vortex-array/src/dtype/serde/flatbuffers.rs +++ b/vortex-array/src/dtype/serde/flatbuffers.rs @@ -61,6 +61,7 @@ impl ViewedDType { impl StructFields { /// Creates a new instance from a flatbuffer-defined object and its underlying buffer. + #[allow(clippy::needless_pass_by_value)] fn from_fb( fb_struct: fbd::Struct_<'_>, buffer: FlatBuffer, @@ -429,6 +430,7 @@ mod test { use crate::dtype::serde::flatbuffers::ViewedDType; use crate::dtype::test::SESSION; + #[allow(clippy::needless_pass_by_value)] fn roundtrip_dtype(dtype: DType) { let bytes = dtype.write_flatbuffer_bytes().unwrap(); let root_fb = root::(&bytes).unwrap(); diff --git a/vortex-array/src/expr/exprs.rs b/vortex-array/src/expr/exprs.rs index bc30ba86ec4..c0433236425 100644 --- a/vortex-array/src/expr/exprs.rs +++ b/vortex-array/src/expr/exprs.rs @@ -148,15 +148,16 @@ pub fn nested_case_when( let has_else = else_value.is_some(); let mut children = Vec::with_capacity(when_then_pairs.len() * 2 + usize::from(has_else)); - for (condition, then_value) in &when_then_pairs { - children.push(condition.clone()); - children.push(then_value.clone()); + let when_then_len = when_then_pairs.len(); + for (condition, then_value) in when_then_pairs.into_iter() { + children.push(condition); + children.push(then_value); } if let Some(else_expr) = else_value { children.push(else_expr); } - let Ok(num_when_then_pairs) = u32::try_from(when_then_pairs.len()) else { + let Ok(num_when_then_pairs) = u32::try_from(when_then_len) else { vortex_panic!("nested_case_when has too many when/then pairs"); }; let options = CaseWhenOptions { diff --git a/vortex-array/src/expr/transform/match_between.rs b/vortex-array/src/expr/transform/match_between.rs index 56f03dbac4d..0e3d7d36117 100644 --- a/vortex-array/src/expr/transform/match_between.rs +++ b/vortex-array/src/expr/transform/match_between.rs @@ -16,6 +16,7 @@ use crate::scalar_fn::fns::operators::Operator; /// This pass looks for expression of the form /// `x >= a && x < b` and converts them into x between a and b` +#[allow(clippy::needless_pass_by_value)] pub fn find_between(expr: Expression) -> Expression { // We search all pairs of cnfs to find any pair of expressions can be converted into a between // expression. diff --git a/vortex-array/src/expr/transform/partition.rs b/vortex-array/src/expr/transform/partition.rs index ef852a6cd7f..5a7b1704b9a 100644 --- a/vortex-array/src/expr/transform/partition.rs +++ b/vortex-array/src/expr/transform/partition.rs @@ -38,6 +38,7 @@ use crate::expr::traversal::TraversalOrder; /// expression for computing the validity, or to include that expression as part of the root. /// /// See . +#[allow(clippy::needless_pass_by_value)] pub fn partition( expr: Expression, scope: &DType, diff --git a/vortex-array/src/expr/transform/replace.rs b/vortex-array/src/expr/transform/replace.rs index ca9509d2b00..bf6d648f0ff 100644 --- a/vortex-array/src/expr/transform/replace.rs +++ b/vortex-array/src/expr/transform/replace.rs @@ -14,6 +14,7 @@ use crate::expr::traversal::Transformed; use crate::expr::traversal::TraversalOrder; /// Replaces all occurrences of `needle` in the expression `expr` with `replacement`. +#[allow(clippy::needless_pass_by_value)] pub fn replace(expr: Expression, needle: &Expression, replacement: Expression) -> Expression { expr.transform_down(|node| { if &node == needle { diff --git a/vortex-array/src/patches.rs b/vortex-array/src/patches.rs index 33dfa7591ba..73c410d68ea 100644 --- a/vortex-array/src/patches.rs +++ b/vortex-array/src/patches.rs @@ -707,9 +707,9 @@ impl Patches { let take_indices = take_indices.to_array().execute::(ctx)?; if self.is_map_faster_than_search(&take_indices) { - self.take_map(take_indices, true, ctx) + self.take_map(&take_indices, true, ctx) } else { - self.take_search(take_indices, true, ctx) + self.take_search(&take_indices, true, ctx) } } @@ -727,9 +727,9 @@ impl Patches { let take_indices = take_indices.to_array().execute::(ctx)?; if self.is_map_faster_than_search(&take_indices) { - self.take_map(take_indices, false, ctx) + self.take_map(&take_indices, false, ctx) } else { - self.take_search(take_indices, false, ctx) + self.take_search(&take_indices, false, ctx) } } @@ -739,7 +739,7 @@ impl Patches { )] pub fn take_search( &self, - take_indices: PrimitiveArray, + take_indices: &PrimitiveArray, include_nulls: bool, ctx: &mut ExecutionCtx, ) -> VortexResult> { @@ -763,7 +763,7 @@ impl Patches { take_indices_with_search_fn( patch_indices_slice, take_slice, - take_indices.validity_mask()?, + &take_indices.validity_mask()?, include_nulls, |take_idx| { self.search_index_chunked_batch( @@ -778,7 +778,7 @@ impl Patches { take_indices_with_search_fn( patch_indices_slice, take_slice, - take_indices.validity_mask()?, + &take_indices.validity_mask()?, include_nulls, |take_idx| { let Some(offset) = ::from(self.offset) else { @@ -816,7 +816,7 @@ impl Patches { pub fn take_map( &self, - take_indices: PrimitiveArray, + take_indices: &PrimitiveArray, include_nulls: bool, ctx: &mut ExecutionCtx, ) -> VortexResult> { @@ -1009,7 +1009,7 @@ unsafe fn apply_patches_to_buffer_inner( fn take_map, T: NativePType>( indices: &[I], - take_indices: PrimitiveArray, + take_indices: &PrimitiveArray, indices_offset: usize, min_index: usize, max_index: usize, @@ -1179,7 +1179,7 @@ fn take_indices_with_search_fn< >( indices: &[I], take_indices: &[T], - take_validity: Mask, + take_validity: &Mask, include_nulls: bool, search_fn: F, ) -> VortexResult<(BufferMut, BufferMut)> { @@ -1295,7 +1295,7 @@ mod test { let taken = patches .take_search( - PrimitiveArray::new(buffer![9, 0], Validity::from_iter([true, false])), + &PrimitiveArray::new(buffer![9, 0], Validity::from_iter([true, false])), true, &mut LEGACY_SESSION.create_execution_ctx(), ) @@ -1329,7 +1329,7 @@ mod test { let taken = patches .take_search( - PrimitiveArray::new(buffer![500, 1200, 999], Validity::AllValid), + &PrimitiveArray::new(buffer![500, 1200, 999], Validity::AllValid), true, &mut LEGACY_SESSION.create_execution_ctx(), ) @@ -1356,7 +1356,7 @@ mod test { let taken = patches .take_search( - PrimitiveArray::new(buffer![3, 4, 5], Validity::AllValid), + &PrimitiveArray::new(buffer![3, 4, 5], Validity::AllValid), true, &mut LEGACY_SESSION.create_execution_ctx(), ) @@ -1378,7 +1378,7 @@ mod test { let taken = patches .take_search( - PrimitiveArray::new(buffer![10, 15, 20, 99], Validity::AllValid), + &PrimitiveArray::new(buffer![10, 15, 20, 99], Validity::AllValid), true, &mut LEGACY_SESSION.create_execution_ctx(), ) @@ -1405,7 +1405,7 @@ mod test { let taken = patches .take_search( - PrimitiveArray::new(BufferMut::from_iter(0..1500u64), Validity::AllValid), + &PrimitiveArray::new(BufferMut::from_iter(0..1500u64), Validity::AllValid), false, &mut LEGACY_SESSION.create_execution_ctx(), ) diff --git a/vortex-array/src/scalar/arrow.rs b/vortex-array/src/scalar/arrow.rs index bce11e27e0f..3b28fa3fc3c 100644 --- a/vortex-array/src/scalar/arrow.rs +++ b/vortex-array/src/scalar/arrow.rs @@ -70,6 +70,7 @@ impl TryFrom<&Scalar> for Arc { } /// Convert a [`BoolScalar`] to an Arrow [`Datum`]. +#[allow(clippy::needless_pass_by_value)] fn bool_to_arrow(scalar: BoolScalar<'_>) -> Result, VortexError> { value_to_arrow_scalar!(scalar.value(), BooleanArray) } @@ -108,11 +109,13 @@ fn decimal_to_arrow(scalar: DecimalScalar<'_>) -> Result, VortexE } /// Convert a [`Utf8Scalar`] to an Arrow [`Datum`]. +#[allow(clippy::needless_pass_by_value)] fn utf8_to_arrow(scalar: Utf8Scalar<'_>) -> Result, VortexError> { value_to_arrow_scalar!(scalar.value(), StringViewArray) } /// Convert a [`BinaryScalar`] to an Arrow [`Datum`]. +#[allow(clippy::needless_pass_by_value)] fn binary_to_arrow(scalar: BinaryScalar<'_>) -> Result, VortexError> { value_to_arrow_scalar!(scalar.value(), BinaryViewArray) } diff --git a/vortex-array/src/scalar/constructor.rs b/vortex-array/src/scalar/constructor.rs index dd10b4e0e90..52c75a8220b 100644 --- a/vortex-array/src/scalar/constructor.rs +++ b/vortex-array/src/scalar/constructor.rs @@ -192,6 +192,7 @@ impl Scalar { } /// A helper enum for creating a [`ListScalar`]. +#[derive(Clone, Copy)] enum ListKind { /// Variable-length list. Variable, diff --git a/vortex-array/src/scalar/proto.rs b/vortex-array/src/scalar/proto.rs index b4cdf9a76e6..238c3ed82fb 100644 --- a/vortex-array/src/scalar/proto.rs +++ b/vortex-array/src/scalar/proto.rs @@ -471,6 +471,7 @@ mod tests { VortexSession::empty() } + #[allow(clippy::needless_pass_by_value)] fn round_trip(scalar: Scalar) { assert_eq!( scalar, diff --git a/vortex-array/src/scalar/typed_view/extension/mod.rs b/vortex-array/src/scalar/typed_view/extension/mod.rs index dfbc25f936d..3508883fc92 100644 --- a/vortex-array/src/scalar/typed_view/extension/mod.rs +++ b/vortex-array/src/scalar/typed_view/extension/mod.rs @@ -19,7 +19,7 @@ use crate::scalar::ScalarValue; /// A scalar value representing an extension type. /// /// Extension types allow wrapping a storage type with custom semantics. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Copy)] pub struct ExtScalar<'a> { /// A reference to the `DType` of the extension type. This **must** be the [`DType::Extension`] /// variant. diff --git a/vortex-array/src/scalar_fn/fns/between/mod.rs b/vortex-array/src/scalar_fn/fns/between/mod.rs index 40157841531..c9560350127 100644 --- a/vortex-array/src/scalar_fn/fns/between/mod.rs +++ b/vortex-array/src/scalar_fn/fns/between/mod.rs @@ -420,7 +420,7 @@ mod tests { .unwrap() .to_bool(); - let indices = to_int_indices(matches).unwrap(); + let indices = to_int_indices(&matches).unwrap(); assert_eq!(indices, expected); } @@ -449,7 +449,7 @@ mod tests { .unwrap() .to_bool(); - let indices = to_int_indices(matches).unwrap(); + let indices = to_int_indices(&matches).unwrap(); assert!(indices.is_empty()); // upper is a fixed constant @@ -466,7 +466,7 @@ mod tests { ) .unwrap() .to_bool(); - let indices = to_int_indices(matches).unwrap(); + let indices = to_int_indices(&matches).unwrap(); assert_eq!(indices, vec![0, 1, 3]); // lower is also a constant @@ -484,7 +484,7 @@ mod tests { ) .unwrap() .to_bool(); - let indices = to_int_indices(matches).unwrap(); + let indices = to_int_indices(&matches).unwrap(); assert_eq!(indices, vec![0, 1, 2, 3, 4]); } diff --git a/vortex-array/src/scalar_fn/fns/binary/compare.rs b/vortex-array/src/scalar_fn/fns/binary/compare.rs index 3730b13cfe8..8f80c152328 100644 --- a/vortex-array/src/scalar_fn/fns/binary/compare.rs +++ b/vortex-array/src/scalar_fn/fns/binary/compare.rs @@ -273,7 +273,7 @@ mod tests { .binary(arr.to_array(), Operator::Eq) .unwrap() .to_bool(); - assert_eq!(to_int_indices(matches).unwrap(), [1u64, 2, 3, 4]); + assert_eq!(to_int_indices(&matches).unwrap(), [1u64, 2, 3, 4]); let matches = arr .to_array() @@ -281,7 +281,7 @@ mod tests { .unwrap() .to_bool(); let empty: [u64; 0] = []; - assert_eq!(to_int_indices(matches).unwrap(), empty); + assert_eq!(to_int_indices(&matches).unwrap(), empty); let other = BoolArray::new( BitBuffer::from_iter([false, false, false, true, true]), @@ -293,28 +293,28 @@ mod tests { .binary(other.to_array(), Operator::Lte) .unwrap() .to_bool(); - assert_eq!(to_int_indices(matches).unwrap(), [2u64, 3, 4]); + assert_eq!(to_int_indices(&matches).unwrap(), [2u64, 3, 4]); let matches = arr .to_array() .binary(other.to_array(), Operator::Lt) .unwrap() .to_bool(); - assert_eq!(to_int_indices(matches).unwrap(), [4u64]); + assert_eq!(to_int_indices(&matches).unwrap(), [4u64]); let matches = other .to_array() .binary(arr.to_array(), Operator::Gte) .unwrap() .to_bool(); - assert_eq!(to_int_indices(matches).unwrap(), [2u64, 3, 4]); + assert_eq!(to_int_indices(&matches).unwrap(), [2u64, 3, 4]); let matches = other .to_array() .binary(arr.to_array(), Operator::Gt) .unwrap() .to_bool(); - assert_eq!(to_int_indices(matches).unwrap(), [4u64]); + assert_eq!(to_int_indices(&matches).unwrap(), [4u64]); } #[test] diff --git a/vortex-array/src/scalar_fn/fns/cast/mod.rs b/vortex-array/src/scalar_fn/fns/cast/mod.rs index 92f31bfc013..095ec74f3ab 100644 --- a/vortex-array/src/scalar_fn/fns/cast/mod.rs +++ b/vortex-array/src/scalar_fn/fns/cast/mod.rs @@ -115,7 +115,7 @@ impl ScalarFnVTable for Cast { match columnar { ColumnarView::Canonical(canonical) => { - match cast_canonical(canonical.clone(), target_dtype, ctx)? { + match cast_canonical(canonical, target_dtype, ctx)? { Some(result) => Ok(result), None => vortex_bail!( "No CastKernel to cast canonical array {} from {} to {}", diff --git a/vortex-array/src/scalar_fn/fns/list_contains/mod.rs b/vortex-array/src/scalar_fn/fns/list_contains/mod.rs index 4bc17f9f89a..f9098f75694 100644 --- a/vortex-array/src/scalar_fn/fns/list_contains/mod.rs +++ b/vortex-array/src/scalar_fn/fns/list_contains/mod.rs @@ -335,7 +335,7 @@ fn list_contains_scalar( // Process based on the offset and size types. let list_matches = match_each_integer_ptype!(offsets.ptype(), |O| { match_each_integer_ptype!(sizes.ptype(), |S| { - process_matches::(matches, list_array.len(), offsets, sizes) + process_matches::(&matches, list_array.len(), &offsets, &sizes) }) }); @@ -349,10 +349,10 @@ fn list_contains_scalar( /// Returns a [`BitBuffer`] where each bit represents if a list contains the scalar, derived from a /// [`BoolArray`] of matches on the child elements array. fn process_matches( - matches: BoolArray, + matches: &BoolArray, list_array_len: usize, - offsets: PrimitiveArray, - sizes: PrimitiveArray, + offsets: &PrimitiveArray, + sizes: &PrimitiveArray, ) -> BitBuffer where O: IntegerPType, @@ -689,13 +689,17 @@ mod tests { // -- Tests migrated from compute/list_contains.rs -- fn nonnull_strings(values: Vec>) -> ArrayRef { - ListArray::from_iter_slow::(values, Arc::new(DType::Utf8(Nullability::NonNullable))) - .unwrap() - .as_::() - .to_listview() - .into_array() + ListArray::from_iter_slow::( + values, + &Arc::new(DType::Utf8(Nullability::NonNullable)), + ) + .unwrap() + .as_::() + .to_listview() + .into_array() } + #[allow(clippy::needless_pass_by_value)] fn null_strings(values: Vec>>) -> ArrayRef { let elements = values.iter().flatten().cloned().collect_vec(); diff --git a/vortex-array/src/stats/array.rs b/vortex-array/src/stats/array.rs index 8220a5a2deb..0caafe0ddca 100644 --- a/vortex-array/src/stats/array.rs +++ b/vortex-array/src/stats/array.rs @@ -39,6 +39,7 @@ pub struct ArrayStats { /// Reference to an array's [`StatsSet`]. Can be used to get and mutate the underlying stats. /// /// Constructed by calling [`ArrayStats::to_ref`]. +#[derive(Clone, Copy)] pub struct StatsSetRef<'a> { // We need to reference back to the array dyn_array_ref: &'a dyn DynArray, diff --git a/vortex-array/src/test_harness.rs b/vortex-array/src/test_harness.rs index 348f79829fd..1356a33f037 100644 --- a/vortex-array/src/test_harness.rs +++ b/vortex-array/src/test_harness.rs @@ -30,7 +30,7 @@ where } /// Outputs the indices of the true values in a BoolArray -pub fn to_int_indices(indices_bits: BoolArray) -> VortexResult> { +pub fn to_int_indices(indices_bits: &BoolArray) -> VortexResult> { let buffer = indices_bits.to_bit_buffer(); let mask = indices_bits.validity_mask()?; Ok(buffer diff --git a/vortex-bench/src/lib.rs b/vortex-bench/src/lib.rs index 6dad0f0f6a1..deff2f44b07 100644 --- a/vortex-bench/src/lib.rs +++ b/vortex-bench/src/lib.rs @@ -3,6 +3,7 @@ #![allow(clippy::unwrap_used)] #![allow(clippy::expect_used)] +#![allow(clippy::needless_pass_by_value)] use std::clone::Clone; use std::fmt::Display; @@ -284,7 +285,7 @@ pub fn create_benchmark(b: BenchmarkArg, opts: &Opts) -> anyhow::Result { let scale_factor = opts.get(SCALE_FACTOR_KEY).unwrap_or(DEFAULT_SCALE_FACTOR); let remote_data_dir = opts.get_as::(REMOTE_DATA_KEY); - let benchmark = TpcDsBenchmark::new(scale_factor.to_string(), remote_data_dir)?; + let benchmark = TpcDsBenchmark::new(scale_factor, remote_data_dir)?; Ok(Box::new(benchmark) as _) } BenchmarkArg::StatPopGen => { diff --git a/vortex-bench/src/tpcds/duckdb.rs b/vortex-bench/src/tpcds/duckdb.rs index 3890e223339..cd2c18b3f6a 100644 --- a/vortex-bench/src/tpcds/duckdb.rs +++ b/vortex-bench/src/tpcds/duckdb.rs @@ -9,7 +9,7 @@ use tracing::info; use crate::Format; -pub fn generate_tpcds(base_dir: PathBuf, scale_factor: String) -> Result { +pub fn generate_tpcds(base_dir: PathBuf, scale_factor: &str) -> Result { // Create output directory based on format let output_dir = base_dir.join(Format::Parquet.name()); std::fs::create_dir_all(&output_dir)?; diff --git a/vortex-bench/src/tpcds/tpcds_benchmark.rs b/vortex-bench/src/tpcds/tpcds_benchmark.rs index 3ad60876521..4767cf237a9 100644 --- a/vortex-bench/src/tpcds/tpcds_benchmark.rs +++ b/vortex-bench/src/tpcds/tpcds_benchmark.rs @@ -24,10 +24,10 @@ pub struct TpcDsBenchmark { } impl TpcDsBenchmark { - pub fn new(scale_factor: String, use_remote_data_dir: Option) -> Result { + pub fn new(scale_factor: &str, use_remote_data_dir: Option) -> Result { Ok(Self { - scale_factor: scale_factor.clone(), - data_url: Self::create_data_url(&use_remote_data_dir, &scale_factor)?, + scale_factor: scale_factor.to_string(), + data_url: Self::create_data_url(&use_remote_data_dir, scale_factor)?, }) } @@ -72,7 +72,7 @@ impl Benchmark for TpcDsBenchmark { Format::Parquet ); - generate_tpcds(base_data_dir, self.scale_factor.clone())?; + generate_tpcds(base_data_dir, self.scale_factor.as_ref())?; Ok(()) } diff --git a/vortex-bench/src/tpch/benchmark.rs b/vortex-bench/src/tpch/benchmark.rs index 71290f860e0..1981ea37018 100644 --- a/vortex-bench/src/tpch/benchmark.rs +++ b/vortex-bench/src/tpch/benchmark.rs @@ -36,8 +36,8 @@ pub struct TpcHBenchmark { impl TpcHBenchmark { pub fn new(scale_factor: String, use_remote_data_dir: Option) -> anyhow::Result { Ok(Self { - scale_factor: scale_factor.clone(), data_url: Self::create_data_url(&use_remote_data_dir, &scale_factor)?, + scale_factor, }) } diff --git a/vortex-bench/src/tpch/tpchgen.rs b/vortex-bench/src/tpch/tpchgen.rs index 75fe4db9e8f..2d7f6f905d8 100644 --- a/vortex-bench/src/tpch/tpchgen.rs +++ b/vortex-bench/src/tpch/tpchgen.rs @@ -123,7 +123,6 @@ pub async fn generate_tpch_tables(options: TpchGenOptions) -> Result<()> { table = table_name, "Generating TPC-H table", ); - let table_name = table_name.to_string(); generate_table_files(table_name, *generator, options.clone()) }) @@ -188,7 +187,7 @@ impl TableGenerator { /// Generate files for a specific table in streaming fashion fn generate_table_files( - table_name: String, + table_name: &str, generator: TableGenerator, options: TpchGenOptions, ) -> Result>>> { diff --git a/vortex-btrblocks/public-api.lock b/vortex-btrblocks/public-api.lock index c0a6c24cb9c..7b5e9be9978 100644 --- a/vortex-btrblocks/public-api.lock +++ b/vortex-btrblocks/public-api.lock @@ -252,10 +252,16 @@ pub struct vortex_btrblocks::GenerateStatsOptions pub vortex_btrblocks::GenerateStatsOptions::count_distinct_values: bool +impl core::clone::Clone for vortex_btrblocks::GenerateStatsOptions + +pub fn vortex_btrblocks::GenerateStatsOptions::clone(&self) -> vortex_btrblocks::GenerateStatsOptions + impl core::default::Default for vortex_btrblocks::GenerateStatsOptions pub fn vortex_btrblocks::GenerateStatsOptions::default() -> Self +impl core::marker::Copy for vortex_btrblocks::GenerateStatsOptions + pub struct vortex_btrblocks::IntegerStats impl core::clone::Clone for vortex_btrblocks::IntegerStats diff --git a/vortex-btrblocks/src/canonical_compressor.rs b/vortex-btrblocks/src/canonical_compressor.rs index 984078b4b80..d392877a883 100644 --- a/vortex-btrblocks/src/canonical_compressor.rs +++ b/vortex-btrblocks/src/canonical_compressor.rs @@ -13,6 +13,7 @@ use vortex_array::arrays::ExtensionArray; use vortex_array::arrays::FixedSizeListArray; use vortex_array::arrays::ListArray; use vortex_array::arrays::ListViewArray; +use vortex_array::arrays::ListViewArrayParts; use vortex_array::arrays::StructArray; use vortex_array::arrays::TemporalArray; use vortex_array::arrays::list_from_list_view; @@ -142,7 +143,7 @@ impl BtrBlocksCompressor { /// Compresses a [`ListArray`] by narrowing offsets and recursively compressing elements. fn compress_list_array( &self, - list_array: ListArray, + list_array: &ListArray, ctx: CompressorContext, ) -> VortexResult { // Reset the offsets to remove garbage data that might prevent us from narrowing our @@ -176,14 +177,22 @@ impl BtrBlocksCompressor { list_view: ListViewArray, ctx: CompressorContext, ) -> VortexResult { - let compressed_elems = self.compress(list_view.elements())?; + let ListViewArrayParts { + elements_dtype: _, + elements, + offsets, + sizes, + validity, + } = list_view.into_parts(); + + let compressed_elems = self.compress(&elements)?; let compressed_offsets = self.compress_canonical( - Canonical::Primitive(list_view.offsets().to_primitive().narrow()?), + Canonical::Primitive(offsets.to_primitive().narrow()?), ctx, Excludes::none(), )?; let compressed_sizes = self.compress_canonical( - Canonical::Primitive(list_view.sizes().to_primitive().narrow()?), + Canonical::Primitive(sizes.to_primitive().narrow()?), ctx, Excludes::none(), )?; @@ -191,7 +200,7 @@ impl BtrBlocksCompressor { compressed_elems, compressed_offsets, compressed_sizes, - list_view.validity().clone(), + validity, )? .into_array()) } @@ -240,8 +249,8 @@ impl CanonicalCompressor for BtrBlocksCompressor { if list_view_array.is_zero_copy_to_list() || list_view_array.elements().is_empty() { // Offsets are already monotonic and non-overlapping, so we // can drop the sizes array and compress as a ListArray. - let list_array = list_from_list_view(list_view_array)?; - self.compress_list_array(list_array, ctx) + let list_array = list_from_list_view(&list_view_array)?; + self.compress_list_array(&list_array, ctx) } else { self.compress_list_view_array(list_view_array, ctx) } diff --git a/vortex-btrblocks/src/stats.rs b/vortex-btrblocks/src/stats.rs index b3e25cfb8d6..add7766acd9 100644 --- a/vortex-btrblocks/src/stats.rs +++ b/vortex-btrblocks/src/stats.rs @@ -8,6 +8,7 @@ use std::fmt::Debug; use vortex_array::vtable::VTable; /// Configures how stats are generated. +#[derive(Clone, Copy)] pub struct GenerateStatsOptions { /// Should distinct values should be counted during stats generation. pub count_distinct_values: bool, diff --git a/vortex-cuda/src/dynamic_dispatch/plan_builder.rs b/vortex-cuda/src/dynamic_dispatch/plan_builder.rs index fb922d4da50..04b24e85985 100644 --- a/vortex-cuda/src/dynamic_dispatch/plan_builder.rs +++ b/vortex-cuda/src/dynamic_dispatch/plan_builder.rs @@ -151,7 +151,7 @@ impl PlanBuilderState<'_> { } else if id == RunEndVTable::ID { self.walk_runend(array) } else if id == PrimitiveVTable::ID { - self.walk_primitive(array) + self.walk_primitive(&array) } else { vortex_bail!( "Encoding {:?} not supported by dynamic dispatch plan builder", @@ -161,7 +161,7 @@ impl PlanBuilderState<'_> { } /// Canonical primitive array → LOAD source op. - fn walk_primitive(&mut self, array: ArrayRef) -> VortexResult { + fn walk_primitive(&mut self, array: &ArrayRef) -> VortexResult { let prim = array.to_canonical()?.into_primitive(); let PrimitiveArrayParts { buffer, .. } = prim.into_parts(); let device_buf = block_on(self.ctx.ensure_on_device(buffer))?; diff --git a/vortex-cuda/src/kernel/encodings/zstd.rs b/vortex-cuda/src/kernel/encodings/zstd.rs index e70047f61b3..1cbd695177d 100644 --- a/vortex-cuda/src/kernel/encodings/zstd.rs +++ b/vortex-cuda/src/kernel/encodings/zstd.rs @@ -184,7 +184,7 @@ pub async fn zstd_kernel_prepare( pub(crate) struct ZstdExecutor; impl ZstdExecutor { - fn try_specialize(array: ArrayRef) -> Option { + fn try_specialize(array: &ArrayRef) -> Option { array.as_opt::().cloned() } } @@ -197,7 +197,7 @@ impl CudaExecute for ZstdExecutor { array: ArrayRef, ctx: &mut CudaExecutionCtx, ) -> VortexResult { - let zstd = Self::try_specialize(array).ok_or_else(|| vortex_err!("Expected ZstdArray"))?; + let zstd = Self::try_specialize(&array).ok_or_else(|| vortex_err!("Expected ZstdArray"))?; match zstd.as_ref().dtype() { DType::Binary(_) | DType::Utf8(_) => decode_zstd(zstd, ctx).await, diff --git a/vortex-datafusion/src/persistent/opener.rs b/vortex-datafusion/src/persistent/opener.rs index 5986a06da49..cda037b8502 100644 --- a/vortex-datafusion/src/persistent/opener.rs +++ b/vortex-datafusion/src/persistent/opener.rs @@ -306,7 +306,7 @@ impl FileOpener for VortexOpener { if let Some(file_range) = file.range { scan_builder = apply_byte_range( - file_range, + &file_range, file.object_meta.size, vxf.row_count(), scan_builder, @@ -417,7 +417,7 @@ impl FileOpener for VortexOpener { /// If the file has a [`FileRange`], we translate it into a row range in the file for the scan. fn apply_byte_range( - file_range: FileRange, + file_range: &FileRange, total_size: u64, row_count: u64, scan_builder: ScanBuilder, diff --git a/vortex-duckdb/src/exporter/constant.rs b/vortex-duckdb/src/exporter/constant.rs index 8bb05ac1edc..66edfbae6d5 100644 --- a/vortex-duckdb/src/exporter/constant.rs +++ b/vortex-duckdb/src/exporter/constant.rs @@ -46,6 +46,7 @@ pub fn new_exporter_with_mask( new_exporter(array) } +#[allow(clippy::needless_pass_by_value)] pub(crate) fn new_exporter(array: ConstantArray) -> VortexResult> { let value = if array.scalar().is_null() { // If the scalar is null and _not_ of type Null, then we cannot assign a null DuckDB value diff --git a/vortex-duckdb/src/exporter/mod.rs b/vortex-duckdb/src/exporter/mod.rs index 10d2445f23a..12b99d812b2 100644 --- a/vortex-duckdb/src/exporter/mod.rs +++ b/vortex-duckdb/src/exporter/mod.rs @@ -168,7 +168,7 @@ fn new_array_exporter_with_flatten( Canonical::Struct(array) => struct_::new_exporter(array, cache, ctx), Canonical::Extension(ext) => { if let Ok(temporal_array) = TemporalArray::try_from(ext) { - return temporal::new_exporter(temporal_array, ctx); + return temporal::new_exporter(&temporal_array, ctx); } vortex_bail!("no non-temporal extension exporter") } diff --git a/vortex-duckdb/src/exporter/primitive.rs b/vortex-duckdb/src/exporter/primitive.rs index 274dc0a6f36..06560d40cd0 100644 --- a/vortex-duckdb/src/exporter/primitive.rs +++ b/vortex-duckdb/src/exporter/primitive.rs @@ -5,8 +5,9 @@ use std::marker::PhantomData; use vortex::array::ExecutionCtx; use vortex::array::arrays::PrimitiveArray; +use vortex::array::arrays::PrimitiveArrayParts; use vortex::array::match_each_native_ptype; -use vortex::array::vtable::ValidityHelper; +use vortex::buffer::Buffer; use vortex::dtype::NativePType; use vortex::error::VortexResult; use vortex::mask::Mask; @@ -29,18 +30,22 @@ pub fn new_exporter( array: PrimitiveArray, ctx: &mut ExecutionCtx, ) -> VortexResult> { - let validity = array - .validity() - .to_array(array.len()) - .execute::(ctx)?; + let array_len = array.len(); + let PrimitiveArrayParts { + ptype, + buffer, + validity, + } = array.into_parts(); + + let validity = validity.to_array(array_len).execute::(ctx)?; if validity.all_false() { - let ltype = LogicalType::try_from(array.ptype())?; - return Ok(all_invalid::new_exporter(array.len(), <ype)); + let ltype = LogicalType::try_from(ptype)?; + return Ok(all_invalid::new_exporter(array_len, <ype)); } - match_each_native_ptype!(array.ptype(), |T| { - let buffer = array.to_buffer::(); + match_each_native_ptype!(ptype, |T| { + let buffer = Buffer::::from_byte_buffer(buffer.to_host_sync()); let prim = Box::new(PrimitiveExporter { len: buffer.len(), start: buffer.as_ptr(), diff --git a/vortex-duckdb/src/exporter/temporal.rs b/vortex-duckdb/src/exporter/temporal.rs index c1d321d78ff..8a452510634 100644 --- a/vortex-duckdb/src/exporter/temporal.rs +++ b/vortex-duckdb/src/exporter/temporal.rs @@ -28,7 +28,7 @@ impl ColumnExporter for TemporalExporter { // TODO(joe): into_parts pub(crate) fn new_exporter( - array: TemporalArray, + array: &TemporalArray, ctx: &mut ExecutionCtx, ) -> VortexResult> { Ok(Box::new(TemporalExporter { @@ -69,7 +69,7 @@ mod tests { DataChunk::new([LogicalType::new(cpp::duckdb_type::DUCKDB_TYPE_TIMESTAMP_S)]); let mut ctx = SESSION.create_execution_ctx(); - new_exporter(arr, &mut ctx) + new_exporter(&arr, &mut ctx) .unwrap() .export(1, 5, chunk.get_vector_mut(0), &mut ctx) .unwrap(); @@ -94,7 +94,7 @@ mod tests { let mut chunk = DataChunk::new([LogicalType::new(cpp::duckdb_type::DUCKDB_TYPE_TIMESTAMP)]); let mut ctx = SESSION.create_execution_ctx(); - new_exporter(arr, &mut ctx) + new_exporter(&arr, &mut ctx) .unwrap() .export(1, 5, chunk.get_vector_mut(0), &mut ctx) .unwrap(); @@ -118,7 +118,7 @@ mod tests { let mut chunk = DataChunk::new([LogicalType::try_from(arr.dtype()).unwrap()]); let mut ctx = SESSION.create_execution_ctx(); - new_exporter(arr, &mut ctx) + new_exporter(&arr, &mut ctx) .unwrap() .export(1, 5, chunk.get_vector_mut(0), &mut ctx) .unwrap(); diff --git a/vortex-duckdb/src/lib.rs b/vortex-duckdb/src/lib.rs index 5e321fa2040..78f78b28d0d 100644 --- a/vortex-duckdb/src/lib.rs +++ b/vortex-duckdb/src/lib.rs @@ -2,6 +2,7 @@ // SPDX-FileCopyrightText: Copyright the Vortex contributors #![allow(clippy::missing_safety_doc)] +#![allow(clippy::needless_pass_by_value)] use std::ffi::CStr; use std::ffi::c_char; diff --git a/vortex-ffi/src/log.rs b/vortex-ffi/src/log.rs index cbd7bb5ae7c..1fad4aad2dc 100644 --- a/vortex-ffi/src/log.rs +++ b/vortex-ffi/src/log.rs @@ -24,6 +24,7 @@ fn to_level_filter(level: vx_log_level) -> LevelFilter { /// Log levels for the Vortex library. #[repr(C)] #[allow(non_camel_case_types)] +#[derive(Clone, Copy)] pub enum vx_log_level { /// No logging will be performed. LOG_LEVEL_OFF = 0, diff --git a/vortex-file/public-api.lock b/vortex-file/public-api.lock index fa9c5175a85..fe28d874c8d 100644 --- a/vortex-file/public-api.lock +++ b/vortex-file/public-api.lock @@ -34,7 +34,7 @@ pub struct vortex_file::segments::FileSegmentSource impl vortex_file::segments::FileSegmentSource -pub fn vortex_file::segments::FileSegmentSource::open(segments: alloc::sync::Arc<[vortex_file::SegmentSpec]>, reader: R, handle: vortex_io::runtime::handle::Handle, metrics: vortex_file::segments::RequestMetrics) -> Self +pub fn vortex_file::segments::FileSegmentSource::open(segments: alloc::sync::Arc<[vortex_file::SegmentSpec]>, reader: R, handle: &vortex_io::runtime::handle::Handle, metrics: vortex_file::segments::RequestMetrics) -> Self impl vortex_layout::segments::source::SegmentSource for vortex_file::segments::FileSegmentSource @@ -204,7 +204,7 @@ pub fn vortex_file::FooterDeserializer::buffer(&self) -> &vortex_buffer::ByteBuf pub fn vortex_file::FooterDeserializer::deserialize(&mut self) -> vortex_error::VortexResult -pub fn vortex_file::FooterDeserializer::prefix_data(&mut self, more_data: vortex_buffer::ByteBuffer) +pub fn vortex_file::FooterDeserializer::prefix_data(&mut self, more_data: &vortex_buffer::ByteBuffer) pub fn vortex_file::FooterDeserializer::with_dtype(self, dtype: vortex_array::dtype::DType) -> Self diff --git a/vortex-file/src/footer/deserializer.rs b/vortex-file/src/footer/deserializer.rs index 9b594e31fd6..1f63691fe8f 100644 --- a/vortex-file/src/footer/deserializer.rs +++ b/vortex-file/src/footer/deserializer.rs @@ -73,9 +73,9 @@ impl FooterDeserializer { } /// Prefix more data to the existing buffer when requested by the deserializer. - pub fn prefix_data(&mut self, more_data: ByteBuffer) { + pub fn prefix_data(&mut self, more_data: &ByteBuffer) { let mut buffer = ByteBufferMut::with_capacity(self.buffer.len() + more_data.len()); - buffer.extend_from_slice(&more_data); + buffer.extend_from_slice(more_data); buffer.extend_from_slice(&self.buffer); self.buffer = buffer.freeze(); } @@ -157,7 +157,7 @@ impl FooterDeserializer { &self.buffer, &postscript.footer, &postscript.layout, - dtype, + &dtype, file_stats, )?)) } @@ -245,7 +245,7 @@ impl FooterDeserializer { initial_read: &[u8], footer_segment: &PostscriptSegment, layout_segment: &PostscriptSegment, - dtype: DType, + dtype: &DType, file_stats: Option, ) -> VortexResult