diff --git a/Cargo.lock b/Cargo.lock index 29d2e4b604..f5168f0dee 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -632,9 +632,9 @@ checksum = "37b2a672a2cb129a2e41c10b1224bb368f9f37a2b16b612598138befd7b37eb5" [[package]] name = "cc" -version = "1.1.15" +version = "1.1.16" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "57b6a275aa2903740dc87da01c62040406b8812552e97129a63ea8850a17c6e6" +checksum = "e9d013ecb737093c0e86b151a7b837993cf9ec6c502946cfb44bedc392421e0b" dependencies = [ "jobserver", "libc", @@ -713,9 +713,9 @@ dependencies = [ [[package]] name = "clap" -version = "4.5.16" +version = "4.5.17" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ed6719fffa43d0d87e5fd8caeab59be1554fb028cd30edc88fc4369b17971019" +checksum = "3e5a21b8495e732f1b3c364c9949b201ca7bae518c502c80256c96ad79eaf6ac" dependencies = [ "clap_builder", "clap_derive", @@ -723,9 +723,9 @@ dependencies = [ [[package]] name = "clap_builder" -version = "4.5.15" +version = "4.5.17" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "216aec2b177652e3846684cbfe25c9964d18ec45234f0f5da5157b207ed1aab6" +checksum = "8cf2dd12af7a047ad9d6da2b6b249759a22a7abc0f474c1dae1777afa4b21a73" dependencies = [ "anstream", "anstyle", @@ -974,9 +974,9 @@ dependencies = [ [[package]] name = "dashmap" -version = "6.0.1" +version = "6.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "804c8821570c3f8b70230c2ba75ffa5c0f9a4189b9a432b6656c536712acae28" +checksum = "5041cc499144891f3790297212f32a74fb938e5136a14943f338ef9e0ae276cf" dependencies = [ "cfg-if", "crossbeam-utils", @@ -1882,16 +1882,16 @@ dependencies = [ [[package]] name = "hyper-rustls" -version = "0.27.2" +version = "0.27.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5ee4be2c948921a1a5320b629c4193916ed787a7f7f293fd3f7f5a6c9de74155" +checksum = "08afdbb5c31130e3034af566421053ab03787c640246a446327f550d11bcb333" dependencies = [ "futures-util", "http", "hyper", "hyper-util", "rustls", - "rustls-native-certs", + "rustls-native-certs 0.8.0", "rustls-pki-types", "tokio", "tokio-rustls", @@ -3330,7 +3330,7 @@ dependencies = [ "pin-project-lite", "quinn", "rustls", - "rustls-native-certs", + "rustls-native-certs 0.7.3", "rustls-pemfile", "rustls-pki-types", "serde", @@ -3419,9 +3419,9 @@ dependencies = [ [[package]] name = "rustix" -version = "0.38.35" +version = "0.38.36" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a85d50532239da68e9addb745ba38ff4612a242c1c7ceea689c4bc7c2f43c36f" +checksum = "3f55e80d50763938498dd5ebb18647174e0c76dc38c5505294bb224624f30f36" dependencies = [ "bitflags 2.6.0", "errno", @@ -3457,6 +3457,19 @@ dependencies = [ "security-framework", ] +[[package]] +name = "rustls-native-certs" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fcaf18a4f2be7326cd874a5fa579fae794320a0f388d365dca7e480e55f83f8a" +dependencies = [ + "openssl-probe", + "rustls-pemfile", + "rustls-pki-types", + "schannel", + "security-framework", +] + [[package]] name = "rustls-pemfile" version = "2.1.3" @@ -3599,9 +3612,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.127" +version = "1.0.128" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8043c06d9f82bd7271361ed64f415fe5e12a77fdb52e573e7f06a516dea329ad" +checksum = "6ff5456707a1de34e7e37f2a6fd3d3f808c318259cbd01ab6377795054b483d8" dependencies = [ "itoa", "memchr", @@ -4055,9 +4068,9 @@ dependencies = [ [[package]] name = "tokio-util" -version = "0.7.11" +version = "0.7.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9cf6b47b3771c49ac75ad09a6162f53ad4b8088b76ac60e8ec1455b31a189fe1" +checksum = "61e7c3654c13bcd040d4a03abee2c75b1d14a37b423cf5a813ceae1cc903ec6a" dependencies = [ "bytes", "futures-core", @@ -5251,9 +5264,9 @@ dependencies = [ [[package]] name = "zstd-sys" -version = "2.0.12+zstd.1.5.6" +version = "2.0.13+zstd.1.5.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0a4e40c320c3cb459d9a9ff6de98cff88f4751ee9275d140e2be94a2b74e4c13" +checksum = "38ff0f21cfee8f97d94cef41359e0c89aa6113028ab0291aa8ca0038995a95aa" dependencies = [ "cc", "pkg-config", diff --git a/Cargo.toml b/Cargo.toml index 67948f278a..8e044c81ad 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -131,7 +131,7 @@ uuid = "1.8.0" vortex-alp = { version = "0.8.0", path = "./encodings/alp" } vortex-array = { version = "0.8.0", path = "./vortex-array" } vortex-buffer = { version = "0.8.0", path = "./vortex-buffer" } -vortex-bytebool = { version = "0.8.0", path = "./encodings/byte-bool" } +vortex-bytebool = { version = "0.8.0", path = "./encodings/bytebool" } vortex-datafusion = { version = "0.8.0", path = "./vortex-datafusion" } vortex-datetime-dtype = { version = "0.8.0", path = "./vortex-datetime-dtype" } vortex-datetime-parts = { version = "0.8.0", path = "./encodings/datetime-parts" } @@ -171,11 +171,31 @@ unsafe_op_in_unsafe_fn = "deny" [workspace.lints.clippy] all = { level = "deny", priority = -1 } +as_ptr_cast_mut = { level = "deny" } +borrow_as_ptr = { level = "deny" } +collection_is_never_read = { level = "deny" } +cognitive_complexity = { level = "deny" } +debug_assert_with_mut_call = { level = "deny" } +derive_partial_eq_without_eq = { level = "deny" } +exit = { level = "deny" } +expect_fun_call = { level = "deny" } +expect_used = { level = "deny" } +equatable_if_let = { level = "deny" } +fallible_impl_from = { level = "deny" } +get_unwrap = { level = "deny" } +host_endian_bytes = { level = "deny" } if_then_some_else_none = { level = "deny" } +inconsistent_struct_constructor = { level = "deny" } +manual_assert = { level = "deny" } +manual_is_variant_and = { level = "deny" } mem_forget = { level = "deny" } or_fun_call = "deny" panic_in_result_fn = { level = "deny" } +panic = { level = "deny" } same_name_method = { level = "deny" } tests_outside_test_module = { level = "deny" } +# todo = { level = "deny" } +# unimplemented = { level = "deny" } unwrap_in_result = { level = "deny" } +unwrap_used = { level = "deny" } use_debug = { level = "deny" } diff --git a/bench-vortex/Cargo.toml b/bench-vortex/Cargo.toml index 51d0d91f9a..c2e53f4188 100644 --- a/bench-vortex/Cargo.toml +++ b/bench-vortex/Cargo.toml @@ -12,8 +12,13 @@ include = { workspace = true } edition = { workspace = true } rust-version = { workspace = true } -[lints] -workspace = true +[lints.rust] +warnings = "deny" +unsafe_op_in_unsafe_fn = "deny" + +[lints.clippy] +all = { level = "deny", priority = -1 } +or_fun_call = "deny" [dependencies] anyhow = { workspace = true } diff --git a/bench-vortex/benches/datafusion_benchmark.rs b/bench-vortex/benches/datafusion_benchmark.rs index f969ebacd9..88d316d9d9 100644 --- a/bench-vortex/benches/datafusion_benchmark.rs +++ b/bench-vortex/benches/datafusion_benchmark.rs @@ -1,5 +1,3 @@ -#![allow(clippy::use_debug)] - use std::collections::HashSet; use std::sync::Arc; @@ -83,7 +81,7 @@ fn toy_dataset_arrow() -> RecordBatch { } fn toy_dataset_vortex(compress: bool) -> Array { - let uncompressed = toy_dataset_arrow().into(); + let uncompressed = toy_dataset_arrow().try_into().unwrap(); if !compress { return uncompressed; diff --git a/bench-vortex/benches/tpch_benchmark.rs b/bench-vortex/benches/tpch_benchmark.rs index 30d8372696..1b5ae80219 100644 --- a/bench-vortex/benches/tpch_benchmark.rs +++ b/bench-vortex/benches/tpch_benchmark.rs @@ -1,5 +1,3 @@ -#![allow(clippy::use_debug)] - use bench_vortex::tpch::dbgen::{DBGen, DBGenOptions}; use bench_vortex::tpch::{load_datasets, run_tpch_query, tpch_queries, Format}; use criterion::{criterion_group, criterion_main, Criterion}; diff --git a/bench-vortex/src/bin/tpch_benchmark.rs b/bench-vortex/src/bin/tpch_benchmark.rs index 06fedcee55..e8207476b0 100644 --- a/bench-vortex/src/bin/tpch_benchmark.rs +++ b/bench-vortex/src/bin/tpch_benchmark.rs @@ -1,5 +1,3 @@ -#![allow(clippy::use_debug)] - use std::collections::HashMap; use std::process::ExitCode; use std::sync; diff --git a/bench-vortex/src/data_downloads.rs b/bench-vortex/src/data_downloads.rs index ba1e6cb44e..88c7ceccb8 100644 --- a/bench-vortex/src/data_downloads.rs +++ b/bench-vortex/src/data_downloads.rs @@ -45,7 +45,7 @@ pub fn data_vortex_uncompressed(fname_out: &str, downloaded_data: PathBuf) -> Pa let array = ChunkedArray::try_new( reader .into_iter() - .map(|batch_result| Array::from(batch_result.unwrap())) + .map(|batch_result| Array::try_from(batch_result.unwrap()).unwrap()) .collect(), dtype, ) diff --git a/bench-vortex/src/lib.rs b/bench-vortex/src/lib.rs index dea92a1d68..f88cc7b149 100644 --- a/bench-vortex/src/lib.rs +++ b/bench-vortex/src/lib.rs @@ -190,7 +190,8 @@ pub fn compress_taxi_data() -> Array { let chunks = reader .into_iter() .map(|batch_result| batch_result.unwrap()) - .map(Array::from) + .map(Array::try_from) + .map(Result::unwrap) .map(|array| { uncompressed_size += array.nbytes(); compressor.compress(&array).unwrap() @@ -288,7 +289,7 @@ mod test { let struct_arrow: ArrowStructArray = record_batch.into(); let arrow_array: ArrowArrayRef = Arc::new(struct_arrow); let vortex_array = Array::from_arrow(arrow_array.clone(), false); - let vortex_as_arrow = vortex_array.into_canonical().unwrap().into_arrow(); + let vortex_as_arrow = vortex_array.into_canonical().unwrap().into_arrow().unwrap(); assert_eq!(vortex_as_arrow.deref(), arrow_array.deref()); } } @@ -309,7 +310,7 @@ mod test { let vortex_array = Array::from_arrow(arrow_array.clone(), false); let compressed = compressor.compress(&vortex_array).unwrap(); - let compressed_as_arrow = compressed.into_canonical().unwrap().into_arrow(); + let compressed_as_arrow = compressed.into_canonical().unwrap().into_arrow().unwrap(); assert_eq!(compressed_as_arrow.deref(), arrow_array.deref()); } } diff --git a/bench-vortex/src/reader.rs b/bench-vortex/src/reader.rs index bfa33aa683..90fbb5318c 100644 --- a/bench-vortex/src/reader.rs +++ b/bench-vortex/src/reader.rs @@ -20,6 +20,7 @@ use object_store::ObjectStore; use parquet::arrow::arrow_reader::{ArrowReaderOptions, ParquetRecordBatchReaderBuilder}; use parquet::arrow::async_reader::{AsyncFileReader, ParquetObjectReader}; use parquet::arrow::ParquetRecordBatchStreamBuilder; +use parquet::file::metadata::RowGroupMetaData; use serde::{Deserialize, Serialize}; use stream::StreamExt; use vortex::array::{ChunkedArray, PrimitiveArray}; @@ -58,7 +59,7 @@ pub async fn open_vortex(path: &Path) -> VortexResult { .into_array_stream() .collect_chunked() .await - .map(|a| a.into_array()) + .map(IntoArray::into_array) } pub async fn rewrite_parquet_as_vortex( @@ -101,7 +102,7 @@ pub fn compress_parquet_to_vortex(parquet_path: &Path) -> VortexResult( .metadata() .row_groups() .iter() - .map(|rg| rg.num_rows()) + .map(RowGroupMetaData::num_rows) .scan(0i64, |acc, x| { *acc += x; Some(*acc) @@ -238,7 +239,7 @@ async fn parquet_take_from_stream( let row_group_indices = row_groups .keys() .sorted() - .map(|i| row_groups.get(i).unwrap().clone()) + .map(|i| row_groups[i].clone()) .collect_vec(); let reader = builder diff --git a/bench-vortex/src/tpch/dbgen.rs b/bench-vortex/src/tpch/dbgen.rs index edd5a0eadc..1c66ec947e 100644 --- a/bench-vortex/src/tpch/dbgen.rs +++ b/bench-vortex/src/tpch/dbgen.rs @@ -175,7 +175,7 @@ fn get_or_cache_toolchain( zip_file .url() .path_segments() - .and_then(|segments| segments.last()) + .and_then(Iterator::last) .unwrap(), ); diff --git a/bench-vortex/src/tpch/mod.rs b/bench-vortex/src/tpch/mod.rs index 289fe2cfd6..8c2e2ecd58 100644 --- a/bench-vortex/src/tpch/mod.rs +++ b/bench-vortex/src/tpch/mod.rs @@ -247,8 +247,8 @@ async fn register_vortex_file( let sts = record_batches .iter() .cloned() - .map(Array::from) - .map(|a| a.into_struct().unwrap()) + .map(Array::try_from) + .map(|a| a.unwrap().into_struct().unwrap()) .collect::>(); let mut arrays_map: HashMap, Vec> = HashMap::default(); @@ -272,7 +272,7 @@ async fn register_vortex_file( .iter() .map(|field| { let name: Arc = field.name().as_str().into(); - let dtype = types_map.get(&name).unwrap().clone(); + let dtype = types_map[&name].clone(); let chunks = arrays_map.remove(&name).unwrap(); ( diff --git a/clippy.toml b/clippy.toml index 154626ef4e..59cb72d114 100644 --- a/clippy.toml +++ b/clippy.toml @@ -1 +1,2 @@ +allow-expect-in-tests = true allow-unwrap-in-tests = true diff --git a/encodings/alp/benches/alp_compress.rs b/encodings/alp/benches/alp_compress.rs index 573258789e..d88728557f 100644 --- a/encodings/alp/benches/alp_compress.rs +++ b/encodings/alp/benches/alp_compress.rs @@ -1,3 +1,5 @@ +#![allow(clippy::unwrap_used)] + use arrow::array::{as_primitive_array, ArrowNativeTypeOp, ArrowPrimitiveType}; use arrow::datatypes::{Float32Type, Float64Type}; use divan::{black_box, Bencher}; @@ -49,7 +51,7 @@ where fn alp_canonicalize_sum(array: ALPArray) -> T::Native { let array = array.into_canonical().unwrap().into_arrow(); - let arrow_primitive = as_primitive_array::(array.as_ref()); + let arrow_primitive = as_primitive_array::(array.as_ref().unwrap()); arrow_primitive .iter() .fold(T::default_value(), |acc, value| { diff --git a/encodings/alp/src/alp.rs b/encodings/alp/src/alp.rs index 60a5db0be4..27e24e0e4b 100644 --- a/encodings/alp/src/alp.rs +++ b/encodings/alp/src/alp.rs @@ -4,6 +4,7 @@ use std::mem::size_of; use itertools::Itertools; use num_traits::{Float, NumCast, PrimInt, Zero}; use serde::{Deserialize, Serialize}; +use vortex_error::vortex_panic; const SAMPLE_SIZE: usize = 32; @@ -20,7 +21,7 @@ impl Display for Exponents { } pub trait ALPFloat: Float + Display + 'static { - type ALPInt: PrimInt; + type ALPInt: PrimInt + Display; const FRACTIONAL_BITS: u8; const MAX_EXPONENT: u8; @@ -118,7 +119,14 @@ pub trait ALPFloat: Float + Display + 'static { #[inline] fn decode_single(encoded: Self::ALPInt, exponents: Exponents) -> Self { - let encoded_float: Self = Self::from(encoded).unwrap(); + let encoded_float: Self = Self::from(encoded).unwrap_or_else(|| { + vortex_panic!( + "Failed to convert encoded value {} from {} to {} in ALPFloat::decode_single", + encoded, + std::any::type_name::(), + std::any::type_name::() + ) + }); encoded_float * Self::F10[exponents.f as usize] * Self::IF10[exponents.e as usize] } } diff --git a/encodings/alp/src/array.rs b/encodings/alp/src/array.rs index f087e73fbb..a51fea6159 100644 --- a/encodings/alp/src/array.rs +++ b/encodings/alp/src/array.rs @@ -12,7 +12,7 @@ use vortex::{ impl_encoding, Array, ArrayDType, ArrayDef, ArrayTrait, Canonical, IntoArray, IntoCanonical, }; use vortex_dtype::{DType, PType}; -use vortex_error::{vortex_bail, VortexResult}; +use vortex_error::{vortex_bail, vortex_panic, VortexExpect as _, VortexResult}; use crate::alp::Exponents; use crate::compress::{alp_encode, decompress}; @@ -84,7 +84,7 @@ impl ALPArray { pub fn encoded(&self) -> Array { self.array() .child(0, &self.metadata().encoded_dtype, self.len()) - .expect("Missing encoded array") + .vortex_expect("Missing encoded child in ALPArray") } #[inline] @@ -95,8 +95,8 @@ impl ALPArray { pub fn patches(&self) -> Option { self.metadata().patches_dtype.as_ref().map(|dt| { self.array().child(1, dt, self.len()).unwrap_or_else(|| { - panic!( - "Missing patches with present metadata flag; dtype: {}, patches_len: {}", + vortex_panic!( + "Missing patches with present metadata flag; patches dtype: {}, patches_len: {}", dt, self.len() ) @@ -106,7 +106,9 @@ impl ALPArray { #[inline] pub fn ptype(&self) -> PType { - self.dtype().try_into().unwrap() + self.dtype() + .try_into() + .vortex_expect("Failed to convert DType to PType") } } @@ -197,7 +199,9 @@ impl PrimitiveArrayTrait for ALPArray { let encoded = self .encoded() .with_dyn(|a| a.as_primitive_array_unchecked().i32_accessor()) - .unwrap_or_else(|| panic!("This is is an invariant of the ALP algorithm")); + .vortex_expect( + "Failed to get underlying encoded i32 array for ALP-encoded f32 array", + ); Some(Arc::new(ALPAccessor::new( encoded, @@ -210,7 +214,6 @@ impl PrimitiveArrayTrait for ALPArray { } } - #[allow(clippy::unwrap_in_result)] fn f64_accessor(&self) -> Option> { match self.dtype() { DType::Primitive(PType::F64, _) => { @@ -221,7 +224,9 @@ impl PrimitiveArrayTrait for ALPArray { let encoded = self .encoded() .with_dyn(|a| a.as_primitive_array_unchecked().i64_accessor()) - .expect("This is is an invariant of the ALP algorithm"); + .vortex_expect( + "Failed to get underlying encoded i64 array for ALP-encoded f64 array", + ); Some(Arc::new(ALPAccessor::new( encoded, patches, diff --git a/encodings/alp/src/compress.rs b/encodings/alp/src/compress.rs index ca5dbda148..2266eab2d0 100644 --- a/encodings/alp/src/compress.rs +++ b/encodings/alp/src/compress.rs @@ -2,7 +2,7 @@ use vortex::array::{PrimitiveArray, Sparse, SparseArray}; use vortex::validity::Validity; use vortex::{Array, ArrayDType, ArrayDef, IntoArray, IntoArrayVariant}; use vortex_dtype::{NativePType, PType}; -use vortex_error::{vortex_bail, VortexResult}; +use vortex_error::{vortex_bail, VortexExpect as _, VortexResult}; use vortex_scalar::Scalar; use crate::alp::ALPFloat; @@ -43,7 +43,7 @@ where len, Scalar::null(values.dtype().as_nullable()), ) - .unwrap() + .vortex_expect("Failed to create SparseArray for ALP patches") .into_array() }), ) @@ -62,7 +62,8 @@ pub fn decompress(array: ALPArray) -> VortexResult { let encoded = array.encoded().into_primitive()?; let validity = encoded.validity(); - let decoded = match_each_alp_float_ptype!(array.dtype().try_into()?, |$T| { + let ptype = array.dtype().try_into()?; + let decoded = match_each_alp_float_ptype!(ptype, |$T| { PrimitiveArray::from_vec( decompress_primitive::<$T>(encoded.into_maybe_null_slice(), array.exponents()), validity, diff --git a/encodings/byte-bool/Cargo.toml b/encodings/bytebool/Cargo.toml similarity index 100% rename from encodings/byte-bool/Cargo.toml rename to encodings/bytebool/Cargo.toml diff --git a/encodings/byte-bool/src/compute/mod.rs b/encodings/bytebool/src/compute/mod.rs similarity index 100% rename from encodings/byte-bool/src/compute/mod.rs rename to encodings/bytebool/src/compute/mod.rs diff --git a/encodings/byte-bool/src/lib.rs b/encodings/bytebool/src/lib.rs similarity index 88% rename from encodings/byte-bool/src/lib.rs rename to encodings/bytebool/src/lib.rs index 05e6c6e8b2..8d2f4d8fff 100644 --- a/encodings/byte-bool/src/lib.rs +++ b/encodings/bytebool/src/lib.rs @@ -11,7 +11,7 @@ use vortex::visitor::{AcceptArrayVisitor, ArrayVisitor}; use vortex::{impl_encoding, ArrayDef, ArrayTrait, Canonical, IntoCanonical, TypedArray}; use vortex_buffer::Buffer; use vortex_dtype::DType; -use vortex_error::VortexResult; +use vortex_error::{VortexExpect as _, VortexResult}; mod compute; mod stats; @@ -64,7 +64,9 @@ impl ByteBoolArray { } pub fn buffer(&self) -> &Buffer { - self.array().buffer().expect("missing mandatory buffer") + self.array() + .buffer() + .vortex_expect("ByteBoolArray is missing the underlying buffer") } fn maybe_null_slice(&self) -> &[bool] { @@ -93,7 +95,8 @@ impl BoolArrayTrait for ByteBoolArray { impl From> for ByteBoolArray { fn from(value: Vec) -> Self { - Self::try_from_vec(value, Validity::AllValid).unwrap() + Self::try_from_vec(value, Validity::AllValid) + .vortex_expect("Failed to create ByteBoolArray from Vec") } } @@ -102,9 +105,13 @@ impl From>> for ByteBoolArray { let validity = Validity::from_iter(value.iter()); // This doesn't reallocate, and the compiler even vectorizes it - let data = value.into_iter().map(|b| b.unwrap_or_default()).collect(); + let data = value + .into_iter() + .map(std::option::Option::unwrap_or_default) + .collect(); - Self::try_from_vec(data, validity).unwrap() + Self::try_from_vec(data, validity) + .vortex_expect("Failed to create ByteBoolArray from nullable bools") } } diff --git a/encodings/byte-bool/src/stats.rs b/encodings/bytebool/src/stats.rs similarity index 100% rename from encodings/byte-bool/src/stats.rs rename to encodings/bytebool/src/stats.rs diff --git a/encodings/datetime-parts/src/array.rs b/encodings/datetime-parts/src/array.rs index 34722985cc..c1b68cd0b7 100644 --- a/encodings/datetime-parts/src/array.rs +++ b/encodings/datetime-parts/src/array.rs @@ -7,7 +7,7 @@ use vortex::variants::{ArrayVariants, ExtensionArrayTrait}; use vortex::visitor::{AcceptArrayVisitor, ArrayVisitor}; use vortex::{impl_encoding, Array, ArrayDType, ArrayDef, ArrayTrait, Canonical, IntoCanonical}; use vortex_dtype::DType; -use vortex_error::{vortex_bail, VortexResult}; +use vortex_error::{vortex_bail, VortexExpect as _, VortexResult}; use crate::compute::decode_to_temporal; @@ -65,19 +65,19 @@ impl DateTimePartsArray { pub fn days(&self) -> Array { self.array() .child(0, &self.metadata().days_dtype, self.len()) - .expect("Missing days array") + .vortex_expect("DatetimePartsArray missing days array") } pub fn seconds(&self) -> Array { self.array() .child(1, &self.metadata().seconds_dtype, self.len()) - .expect("Missing seconds array") + .vortex_expect("DatetimePartsArray missing seconds array") } pub fn subsecond(&self) -> Array { self.array() .child(2, &self.metadata().subseconds_dtype, self.len()) - .expect("Missing subsecond array") + .vortex_expect("DatetimePartsArray missing subsecond array") } } diff --git a/encodings/datetime-parts/src/compute.rs b/encodings/datetime-parts/src/compute.rs index 000d676784..de9205c73c 100644 --- a/encodings/datetime-parts/src/compute.rs +++ b/encodings/datetime-parts/src/compute.rs @@ -5,7 +5,7 @@ use vortex::validity::ArrayValidity; use vortex::{Array, ArrayDType, IntoArray, IntoArrayVariant}; use vortex_datetime_dtype::{TemporalMetadata, TimeUnit}; use vortex_dtype::DType; -use vortex_error::{vortex_bail, VortexResult}; +use vortex_error::{vortex_bail, VortexResult, VortexUnwrap as _}; use vortex_scalar::Scalar; use crate::DateTimePartsArray; @@ -79,7 +79,7 @@ impl ScalarAtFn for DateTimePartsArray { } fn scalar_at_unchecked(&self, index: usize) -> Scalar { - ::scalar_at(self, index).unwrap() + ::scalar_at(self, index).vortex_unwrap() } } @@ -118,7 +118,7 @@ pub fn decode_to_temporal(array: &DateTimePartsArray) -> VortexResult(T); @@ -65,7 +66,7 @@ pub fn dict_encode_typed_primitive( } } }) - .unwrap(); + .vortex_expect("Failed to dictionary encode primitive array"); let values_validity = if array.dtype().is_nullable() { let mut validity = vec![true; values.len()]; @@ -86,7 +87,7 @@ pub fn dict_encode_typed_primitive( pub fn dict_encode_varbin(array: &VarBinArray) -> (PrimitiveArray, VarBinArray) { array .with_iterator(|iter| dict_encode_typed_varbin(array.dtype().clone(), iter)) - .unwrap() + .vortex_expect("Failed to dictionary encode varbin array") } fn lookup_bytes<'a, T: NativePType + AsPrimitive>( @@ -165,7 +166,7 @@ where dtype, values_validity, ) - .unwrap(), + .vortex_expect("Failed to create VarBinArray dictionary during encoding"), ) } diff --git a/encodings/dict/src/compute.rs b/encodings/dict/src/compute.rs index e5a6f0b7ee..0b31b42c8b 100644 --- a/encodings/dict/src/compute.rs +++ b/encodings/dict/src/compute.rs @@ -1,7 +1,7 @@ use vortex::compute::unary::{scalar_at, scalar_at_unchecked, ScalarAtFn}; use vortex::compute::{slice, take, ArrayCompute, SliceFn, TakeFn}; use vortex::{Array, IntoArray}; -use vortex_error::VortexResult; +use vortex_error::{VortexExpect, VortexResult}; use vortex_scalar::Scalar; use crate::DictArray; @@ -30,7 +30,7 @@ impl ScalarAtFn for DictArray { let dict_index: usize = scalar_at_unchecked(&self.codes(), index) .as_ref() .try_into() - .unwrap(); + .vortex_expect("Invalid dict index"); scalar_at_unchecked(&self.values(), dict_index) } diff --git a/encodings/dict/src/dict.rs b/encodings/dict/src/dict.rs index 7cda4a4943..d30fe314f2 100644 --- a/encodings/dict/src/dict.rs +++ b/encodings/dict/src/dict.rs @@ -13,7 +13,7 @@ use vortex::{ IntoCanonical, }; use vortex_dtype::{match_each_integer_ptype, DType}; -use vortex_error::{vortex_bail, VortexResult}; +use vortex_error::{vortex_bail, vortex_panic, VortexExpect as _, VortexResult}; impl_encoding!("vortex.dict", 20u16, Dict); @@ -44,14 +44,14 @@ impl DictArray { pub fn values(&self) -> Array { self.array() .child(0, self.dtype(), self.metadata().values_len) - .expect("Missing values") + .vortex_expect("DictArray is missing its values child array") } #[inline] pub fn codes(&self) -> Array { self.array() .child(1, &self.metadata().codes_dtype, self.len()) - .expect("Missing codes") + .vortex_expect("DictArray is missing its codes child array") } } @@ -66,24 +66,28 @@ impl IntoCanonical for DictArray { impl ArrayValidity for DictArray { fn is_valid(&self, index: usize) -> bool { let values_index = scalar_at(&self.codes(), index) - .unwrap() + .unwrap_or_else(|err| { + vortex_panic!(err, "Failed to get index {} from DictArray codes", index) + }) .as_ref() .try_into() - .unwrap(); + .vortex_expect("Failed to convert dictionary code to usize"); self.values().with_dyn(|a| a.is_valid(values_index)) } fn logical_validity(&self) -> LogicalValidity { if self.dtype().is_nullable() { - let primitive_codes = self.codes().into_primitive().unwrap(); + let primitive_codes = self + .codes() + .into_primitive() + .vortex_expect("Failed to convert DictArray codes to primitive array"); match_each_integer_ptype!(primitive_codes.ptype(), |$P| { ArrayAccessor::<$P>::with_iterator(&primitive_codes, |iter| { LogicalValidity::Array( BoolArray::from(iter.flatten().map(|c| *c != 0).collect::>()) .into_array(), ) - }) - .unwrap() + }).vortex_expect("Failed to convert DictArray codes into logical validity") }) } else { LogicalValidity::AllValid(self.len()) diff --git a/encodings/fastlanes/benches/bitpacking_take.rs b/encodings/fastlanes/benches/bitpacking_take.rs index fba3d28c28..b96b7cbec5 100644 --- a/encodings/fastlanes/benches/bitpacking_take.rs +++ b/encodings/fastlanes/benches/bitpacking_take.rs @@ -1,3 +1,5 @@ +#![allow(clippy::unwrap_used)] + use criterion::{black_box, criterion_group, criterion_main, Criterion}; use itertools::Itertools; use rand::distributions::Uniform; diff --git a/encodings/fastlanes/src/bitpacking/compress.rs b/encodings/fastlanes/src/bitpacking/compress.rs index 9755fe7f2c..1338f528a4 100644 --- a/encodings/fastlanes/src/bitpacking/compress.rs +++ b/encodings/fastlanes/src/bitpacking/compress.rs @@ -257,31 +257,32 @@ pub unsafe fn unpack_single_primitive( unsafe { BitPacking::unchecked_unpack_single(bit_width, packed_chunk, index_in_chunk) } } -pub fn find_best_bit_width(array: &PrimitiveArray) -> Option { - let bit_width_freq = array.statistics().compute_bit_width_freq()?; +pub fn find_best_bit_width(array: &PrimitiveArray) -> VortexResult { + let bit_width_freq = array + .statistics() + .compute_bit_width_freq() + .ok_or_else(|| vortex_err!(ComputeError: "Failed to compute bit width frequency"))?; - Some(best_bit_width( - &bit_width_freq, - bytes_per_exception(array.ptype()), - )) + best_bit_width(&bit_width_freq, bytes_per_exception(array.ptype())) } /// Assuming exceptions cost 1 value + 1 u32 index, figure out the best bit-width to use. /// We could try to be clever, but we can never really predict how the exceptions will compress. -fn best_bit_width(bit_width_freq: &[usize], bytes_per_exception: usize) -> usize { - let len: usize = bit_width_freq.iter().sum(); - +fn best_bit_width(bit_width_freq: &[usize], bytes_per_exception: usize) -> VortexResult { if bit_width_freq.len() > u8::MAX as usize { - panic!("Too many bit widths"); + vortex_bail!("Too many bit widths"); } + let len: usize = bit_width_freq.iter().sum(); let mut num_packed = 0; let mut best_cost = len * bytes_per_exception; let mut best_width = 0; for (bit_width, freq) in bit_width_freq.iter().enumerate() { + let packed_cost = ((bit_width * len) + 7) / 8; // round up to bytes + num_packed += *freq; - let packed_cost = ((bit_width * len) + 7) / 8; let exceptions_cost = (len - num_packed) * bytes_per_exception; + let cost = exceptions_cost + packed_cost; if cost < best_cost { best_cost = cost; @@ -289,7 +290,7 @@ fn best_bit_width(bit_width_freq: &[usize], bytes_per_exception: usize) -> usize } } - best_width + Ok(best_width) } fn bytes_per_exception(ptype: PType) -> usize { @@ -315,7 +316,10 @@ mod test { // 10 1-bit values, 20 2-bit, etc. let freq = vec![0, 10, 20, 15, 1, 0, 0, 0]; // 3-bits => (46 * 3) + (8 * 1 * 5) => 178 bits => 23 bytes and zero exceptions - assert_eq!(best_bit_width(&freq, bytes_per_exception(PType::U8)), 3); + assert_eq!( + best_bit_width(&freq, bytes_per_exception(PType::U8)).unwrap(), + 3 + ); } #[test] diff --git a/encodings/fastlanes/src/bitpacking/compute/scalar_at.rs b/encodings/fastlanes/src/bitpacking/compute/scalar_at.rs index 286b66c8b8..841850b3d6 100644 --- a/encodings/fastlanes/src/bitpacking/compute/scalar_at.rs +++ b/encodings/fastlanes/src/bitpacking/compute/scalar_at.rs @@ -1,6 +1,6 @@ use vortex::compute::unary::{scalar_at_unchecked, ScalarAtFn}; use vortex::ArrayDType; -use vortex_error::VortexResult; +use vortex_error::{VortexResult, VortexUnwrap as _}; use vortex_scalar::Scalar; use crate::{unpack_single, BitPackedArray}; @@ -18,7 +18,7 @@ impl ScalarAtFn for BitPackedArray { } fn scalar_at_unchecked(&self, index: usize) -> Scalar { - self.scalar_at(index).unwrap() + self.scalar_at(index).vortex_unwrap() } } diff --git a/encodings/fastlanes/src/bitpacking/compute/search_sorted.rs b/encodings/fastlanes/src/bitpacking/compute/search_sorted.rs index 9a33fe158d..04c79671fe 100644 --- a/encodings/fastlanes/src/bitpacking/compute/search_sorted.rs +++ b/encodings/fastlanes/src/bitpacking/compute/search_sorted.rs @@ -10,7 +10,7 @@ use vortex::compute::{ use vortex::validity::Validity; use vortex::{ArrayDType, IntoArrayVariant}; use vortex_dtype::{match_each_unsigned_integer_ptype, NativePType}; -use vortex_error::{VortexError, VortexResult}; +use vortex_error::{VortexError, VortexExpect as _, VortexResult}; use vortex_scalar::Scalar; use crate::{unpack_single_primitive, BitPackedArray}; @@ -67,13 +67,16 @@ struct BitPackedSearch { impl BitPackedSearch { pub fn new(array: &BitPackedArray) -> Self { Self { - packed: array.packed().into_primitive().unwrap(), + packed: array + .packed() + .into_primitive() + .vortex_expect("Failed to get packed bytes as PrimitiveArray"), offset: array.offset(), length: array.len(), bit_width: array.bit_width(), min_patch_offset: array.patches().map(|p| { SparseArray::try_from(p) - .expect("Only Sparse patches are supported") + .vortex_expect("Only sparse patches are supported") .min_index() }), validity: array.validity(), diff --git a/encodings/fastlanes/src/bitpacking/mod.rs b/encodings/fastlanes/src/bitpacking/mod.rs index 8773fcf342..f83a6e4e10 100644 --- a/encodings/fastlanes/src/bitpacking/mod.rs +++ b/encodings/fastlanes/src/bitpacking/mod.rs @@ -7,7 +7,9 @@ use vortex::variants::{ArrayVariants, PrimitiveArrayTrait}; use vortex::visitor::{AcceptArrayVisitor, ArrayVisitor}; use vortex::{impl_encoding, Array, ArrayDType, ArrayDef, ArrayTrait, Canonical, IntoCanonical}; use vortex_dtype::{Nullability, PType}; -use vortex_error::{vortex_bail, vortex_err, VortexResult}; +use vortex_error::{ + vortex_bail, vortex_err, vortex_panic, VortexError, VortexExpect as _, VortexResult, +}; mod compress; mod compute; @@ -113,7 +115,7 @@ impl BitPackedArray { &self.dtype().with_nullability(Nullability::NonNullable), self.packed_len(), ) - .expect("Missing packed array") + .vortex_expect("BitpackedArray is missing packed child bytes array") } #[inline] @@ -159,7 +161,13 @@ impl BitPackedArray { #[inline] pub fn ptype(&self) -> PType { - self.dtype().try_into().unwrap() + self.dtype().try_into().unwrap_or_else(|err: VortexError| { + vortex_panic!( + err, + "Failed to convert BitpackedArray DType {} to PType", + self.dtype() + ) + }) } #[inline] diff --git a/encodings/fastlanes/src/delta/mod.rs b/encodings/fastlanes/src/delta/mod.rs index 7b8f7798f1..a091446d53 100644 --- a/encodings/fastlanes/src/delta/mod.rs +++ b/encodings/fastlanes/src/delta/mod.rs @@ -8,7 +8,7 @@ use vortex::variants::{ArrayVariants, PrimitiveArrayTrait}; use vortex::visitor::{AcceptArrayVisitor, ArrayVisitor}; use vortex::{impl_encoding, Array, ArrayDType, ArrayDef, ArrayTrait, Canonical, IntoCanonical}; use vortex_dtype::match_each_unsigned_integer_ptype; -use vortex_error::{vortex_bail, VortexResult}; +use vortex_error::{vortex_bail, vortex_panic, VortexExpect as _, VortexResult}; mod compress; mod compute; @@ -61,19 +61,25 @@ impl DeltaArray { pub fn bases(&self) -> Array { self.array() .child(0, self.dtype(), self.bases_len()) - .expect("Missing bases") + .vortex_expect("DeltaArray is missing bases child array") } #[inline] pub fn deltas(&self) -> Array { self.array() .child(1, self.dtype(), self.len()) - .expect("Missing deltas") + .vortex_expect("DeltaArray is missing deltas child array") } #[inline] fn lanes(&self) -> usize { - let ptype = self.dtype().try_into().unwrap(); + let ptype = self.dtype().try_into().unwrap_or_else(|err| { + vortex_panic!( + err, + "Failed to convert DeltaArray DType {} to PType", + self.dtype() + ) + }); match_each_unsigned_integer_ptype!(ptype, |$T| { <$T as fastlanes::FastLanes>::LANES }) diff --git a/encodings/fastlanes/src/for/compute.rs b/encodings/fastlanes/src/for/compute.rs index 4172f9dcfa..6ae56cf63e 100644 --- a/encodings/fastlanes/src/for/compute.rs +++ b/encodings/fastlanes/src/for/compute.rs @@ -8,7 +8,7 @@ use vortex::compute::{ }; use vortex::{Array, ArrayDType, IntoArray}; use vortex_dtype::{match_each_integer_ptype, NativePType}; -use vortex_error::{VortexError, VortexResult}; +use vortex_error::{VortexError, VortexExpect as _, VortexResult}; use vortex_scalar::{PValue, PrimitiveScalar, Scalar}; use crate::FoRArray; @@ -50,8 +50,10 @@ impl ScalarAtFn for FoRArray { fn scalar_at_unchecked(&self, index: usize) -> Scalar { let encoded_scalar = scalar_at_unchecked(&self.encoded(), index).reinterpret_cast(self.ptype()); - let encoded = PrimitiveScalar::try_from(&encoded_scalar).unwrap(); - let reference = PrimitiveScalar::try_from(self.reference()).unwrap(); + let encoded = + PrimitiveScalar::try_from(&encoded_scalar).vortex_expect("Invalid encoded scalar"); + let reference = + PrimitiveScalar::try_from(self.reference()).vortex_expect("Invalid reference scalar"); match_each_integer_ptype!(encoded.ptype(), |$P| { encoded.typed_value::<$P>().map(|v| (v << self.shift()).wrapping_add(reference.typed_value::<$P>().unwrap())) diff --git a/encodings/fastlanes/src/for/mod.rs b/encodings/fastlanes/src/for/mod.rs index 45bb7f1e8d..d1246339c6 100644 --- a/encodings/fastlanes/src/for/mod.rs +++ b/encodings/fastlanes/src/for/mod.rs @@ -8,7 +8,7 @@ use vortex::variants::{ArrayVariants, PrimitiveArrayTrait}; use vortex::visitor::{AcceptArrayVisitor, ArrayVisitor}; use vortex::{impl_encoding, Array, ArrayDType, ArrayDef, ArrayTrait, Canonical, IntoCanonical}; use vortex_dtype::{DType, PType}; -use vortex_error::{vortex_bail, VortexResult}; +use vortex_error::{vortex_bail, vortex_panic, VortexExpect as _, VortexResult}; use vortex_scalar::Scalar; mod compress; @@ -50,7 +50,7 @@ impl FoRArray { }; self.array() .child(0, dtype, self.len()) - .expect("Missing FoR child") + .vortex_expect("FoRArray is missing encoded child array") } #[inline] @@ -65,7 +65,13 @@ impl FoRArray { #[inline] pub fn ptype(&self) -> PType { - self.dtype().try_into().unwrap() + self.dtype().try_into().unwrap_or_else(|err| { + vortex_panic!( + err, + "Failed to convert FoRArray DType {} to PType", + self.dtype() + ) + }) } } diff --git a/encodings/fsst/src/array.rs b/encodings/fsst/src/array.rs index 2051874652..8b820ef46c 100644 --- a/encodings/fsst/src/array.rs +++ b/encodings/fsst/src/array.rs @@ -8,7 +8,7 @@ use vortex::variants::{ArrayVariants, BinaryArrayTrait, Utf8ArrayTrait}; use vortex::visitor::AcceptArrayVisitor; use vortex::{impl_encoding, Array, ArrayDType, ArrayDef, ArrayTrait, IntoCanonical}; use vortex_dtype::{DType, Nullability, PType}; -use vortex_error::{vortex_bail, VortexResult}; +use vortex_error::{vortex_bail, VortexExpect, VortexResult}; impl_encoding!("vortex.fsst", 24u16, FSST); @@ -80,46 +80,46 @@ impl FSSTArray { pub fn symbols(&self) -> Array { self.array() .child(0, &SYMBOLS_DTYPE, self.metadata().symbols_len) - .expect("FSSTArray must have a symbols child array") + .vortex_expect("FSSTArray must have a symbols child array") } /// Access the symbol table array pub fn symbol_lengths(&self) -> Array { self.array() .child(1, &SYMBOL_LENS_DTYPE, self.metadata().symbols_len) - .expect("FSSTArray must have a symbols child array") + .vortex_expect("FSSTArray must have a symbols child array") } /// Access the codes array pub fn codes(&self) -> Array { self.array() .child(2, &self.metadata().codes_dtype, self.len()) - .expect("FSSTArray must have a codes child array") + .vortex_expect("FSSTArray must have a codes child array") } /// Build a [`Decompressor`][fsst::Decompressor] that can be used to decompress values from /// this array, and pass it to the given function. /// /// This is private to the crate to avoid leaking `fsst-rs` types as part of the public API. - pub(crate) fn with_decompressor(&self, apply: F) -> R + pub(crate) fn with_decompressor(&self, apply: F) -> VortexResult where - F: FnOnce(Decompressor) -> R, + F: FnOnce(Decompressor) -> VortexResult, { // canonicalize the symbols child array, so we can view it contiguously let symbols_array = self .symbols() .into_canonical() - .unwrap() + .map_err(|err| err.with_context("Failed to canonicalize symbols array"))? .into_primitive() - .expect("Symbols must be a Primitive Array"); + .map_err(|err| err.with_context("Symbols must be a Primitive Array"))?; let symbols = symbols_array.maybe_null_slice::(); let symbol_lengths_array = self .symbol_lengths() .into_canonical() - .unwrap() + .map_err(|err| err.with_context("Failed to canonicalize symbol_lengths array"))? .into_primitive() - .unwrap(); + .map_err(|err| err.with_context("Symbol lengths must be a Primitive Array"))?; let symbol_lengths = symbol_lengths_array.maybe_null_slice::(); // Transmute the 64-bit symbol values into fsst `Symbol`s. diff --git a/encodings/fsst/src/compress.rs b/encodings/fsst/src/compress.rs index 96213e28c8..e1b95d03db 100644 --- a/encodings/fsst/src/compress.rs +++ b/encodings/fsst/src/compress.rs @@ -7,6 +7,7 @@ use vortex::array::{PrimitiveArray, VarBinArray, VarBinViewArray}; use vortex::validity::Validity; use vortex::{Array, ArrayDType, IntoArray}; use vortex_dtype::DType; +use vortex_error::{vortex_bail, VortexExpect as _, VortexResult}; use crate::FSSTArray; @@ -15,29 +16,25 @@ use crate::FSSTArray; /// # Panics /// /// If the `strings` array is not encoded as either [`VarBinArray`] or [`VarBinViewArray`]. -pub fn fsst_compress(strings: &Array, compressor: &Compressor) -> FSSTArray { +pub fn fsst_compress(strings: &Array, compressor: &Compressor) -> VortexResult { let len = strings.len(); let dtype = strings.dtype().clone(); // Compress VarBinArray if let Ok(varbin) = VarBinArray::try_from(strings) { - let compressed = varbin + return varbin .with_iterator(|iter| fsst_compress_iter(iter, len, dtype, compressor)) - .unwrap(); - - return compressed; + .map_err(|err| err.with_context("Failed to compress VarBinArray with FSST")); } // Compress VarBinViewArray if let Ok(varbin_view) = VarBinViewArray::try_from(strings) { - let compressed = varbin_view + return varbin_view .with_iterator(|iter| fsst_compress_iter(iter, len, dtype, compressor)) - .unwrap(); - - return compressed; + .map_err(|err| err.with_context("Failed to compress VarBinViewArray with FSST")); } - panic!( + vortex_bail!( "cannot fsst_compress array with unsupported encoding {:?}", strings.encoding().id() ) @@ -48,17 +45,17 @@ pub fn fsst_compress(strings: &Array, compressor: &Compressor) -> FSSTArray { /// # Panics /// /// If the provided array is not FSST compressible. -pub fn fsst_train_compressor(array: &Array) -> Compressor { +pub fn fsst_train_compressor(array: &Array) -> VortexResult { if let Ok(varbin) = VarBinArray::try_from(array) { varbin .with_iterator(|iter| fsst_train_compressor_iter(iter)) - .unwrap() + .map_err(|err| err.with_context("Failed to train FSST Compressor from VarBinArray")) } else if let Ok(varbin_view) = VarBinViewArray::try_from(array) { varbin_view .with_iterator(|iter| fsst_train_compressor_iter(iter)) - .unwrap() + .map_err(|err| err.with_context("Failed to train FSST Compressor from VarBinViewArray")) } else { - panic!( + vortex_bail!( "cannot fsst_compress array with unsupported encoding {:?}", array.encoding().id() ) @@ -118,5 +115,5 @@ where PrimitiveArray::from_vec(symbol_lengths_vec, Validity::NonNullable).into_array(); FSSTArray::try_new(dtype, symbols, symbol_lengths, codes.into_array()) - .expect("building FSSTArray from parts") + .vortex_expect("Failed to build FSSTArray from parts; this should never happen") } diff --git a/encodings/fsst/src/compute.rs b/encodings/fsst/src/compute.rs index 5159932ec6..cb86442aa7 100644 --- a/encodings/fsst/src/compute.rs +++ b/encodings/fsst/src/compute.rs @@ -3,7 +3,7 @@ use vortex::compute::unary::{scalar_at_unchecked, ScalarAtFn}; use vortex::compute::{filter, slice, take, ArrayCompute, FilterFn, SliceFn, TakeFn}; use vortex::{Array, ArrayDType, IntoArray}; use vortex_buffer::Buffer; -use vortex_error::VortexResult; +use vortex_error::{vortex_err, VortexResult, VortexUnwrap}; use vortex_scalar::Scalar; use crate::FSSTArray; @@ -57,19 +57,21 @@ impl TakeFn for FSSTArray { impl ScalarAtFn for FSSTArray { fn scalar_at(&self, index: usize) -> VortexResult { - Ok(self.scalar_at_unchecked(index)) - } - - fn scalar_at_unchecked(&self, index: usize) -> Scalar { let compressed = scalar_at_unchecked(&self.codes(), index); - let binary_datum = compressed.value().as_buffer().unwrap().unwrap(); + let binary_datum = compressed + .value() + .as_buffer()? + .ok_or_else(|| vortex_err!("Expected a binary scalar, found {}", compressed.dtype()))?; self.with_decompressor(|decompressor| { let decoded_buffer: Buffer = decompressor.decompress(binary_datum.as_slice()).into(); - - varbin_scalar(decoded_buffer, self.dtype()) + Ok(varbin_scalar(decoded_buffer, self.dtype())) }) } + + fn scalar_at_unchecked(&self, index: usize) -> Scalar { + ::scalar_at(self, index).vortex_unwrap() + } } impl FilterFn for FSSTArray { diff --git a/encodings/fsst/tests/fsst_tests.rs b/encodings/fsst/tests/fsst_tests.rs index fd94402bda..9029dbf020 100644 --- a/encodings/fsst/tests/fsst_tests.rs +++ b/encodings/fsst/tests/fsst_tests.rs @@ -28,9 +28,11 @@ fn fsst_array() -> Array { .finish(DType::Utf8(Nullability::NonNullable)) .into_array(); - let compressor = fsst_train_compressor(&input_array); + let compressor = fsst_train_compressor(&input_array).unwrap(); - fsst_compress(&input_array, &compressor).into_array() + fsst_compress(&input_array, &compressor) + .unwrap() + .into_array() } #[rstest] diff --git a/encodings/roaring/src/boolean/compute.rs b/encodings/roaring/src/boolean/compute.rs index 9ad79a0379..a9d3e92ecf 100644 --- a/encodings/roaring/src/boolean/compute.rs +++ b/encodings/roaring/src/boolean/compute.rs @@ -32,6 +32,6 @@ impl SliceFn for RoaringBoolArray { let slice_bitmap = Bitmap::from_range(start as u32..stop as u32); let bitmap = self.bitmap().and(&slice_bitmap).add_offset(-(start as i64)); - Self::try_new(bitmap, stop - start).map(|a| a.into_array()) + Self::try_new(bitmap, stop - start).map(IntoArray::into_array) } } diff --git a/encodings/roaring/src/boolean/mod.rs b/encodings/roaring/src/boolean/mod.rs index 6023e432a9..3b7a7883b3 100644 --- a/encodings/roaring/src/boolean/mod.rs +++ b/encodings/roaring/src/boolean/mod.rs @@ -16,7 +16,7 @@ use vortex::{ use vortex_buffer::Buffer; use vortex_dtype::DType; use vortex_dtype::Nullability::{NonNullable, Nullable}; -use vortex_error::{vortex_bail, vortex_err, VortexResult}; +use vortex_error::{vortex_bail, vortex_err, VortexExpect as _, VortexResult}; mod compress; mod compute; @@ -51,7 +51,7 @@ impl RoaringBoolArray { Bitmap::deserialize::( self.array() .buffer() - .expect("RoaringBoolArray buffer is missing") + .vortex_expect("RoaringBoolArray buffer is missing") .as_ref(), ) } @@ -60,7 +60,7 @@ impl RoaringBoolArray { if array.encoding().id() == Bool::ID { roaring_bool_encode(BoolArray::try_from(array)?).map(|a| a.into_array()) } else { - Err(vortex_err!("RoaringInt can only encode boolean arrays")) + vortex_bail!("RoaringInt can only encode boolean arrays") } } } diff --git a/encodings/roaring/src/integer/compress.rs b/encodings/roaring/src/integer/compress.rs index 106079b2f8..47faea2600 100644 --- a/encodings/roaring/src/integer/compress.rs +++ b/encodings/roaring/src/integer/compress.rs @@ -2,7 +2,7 @@ use croaring::Bitmap; use num_traits::NumCast; use vortex::array::PrimitiveArray; use vortex_dtype::{NativePType, PType}; -use vortex_error::{vortex_bail, VortexResult}; +use vortex_error::{vortex_bail, vortex_err, VortexResult}; use crate::RoaringIntArray; @@ -12,7 +12,7 @@ pub fn roaring_int_encode(parray: PrimitiveArray) -> VortexResult roaring_encode_primitive::(parray.maybe_null_slice()), PType::U32 => roaring_encode_primitive::(parray.maybe_null_slice()), PType::U64 => roaring_encode_primitive::(parray.maybe_null_slice()), - _ => vortex_bail!("Unsupported ptype {}", parray.ptype()), + _ => vortex_bail!("Unsupported PType {}", parray.ptype()), } } @@ -20,7 +20,15 @@ fn roaring_encode_primitive( values: &[T], ) -> VortexResult { let mut bitmap = Bitmap::new(); - bitmap.extend(values.iter().map(|i| i.to_u32().unwrap())); + bitmap.extend( + values + .iter() + .map(|i| { + i.to_u32() + .ok_or_else(|| vortex_err!("Failed to cast value {} to u32", i)) + }) + .collect::>>()?, + ); bitmap.run_optimize(); bitmap.shrink_to_fit(); RoaringIntArray::try_new(bitmap, T::PTYPE) diff --git a/encodings/roaring/src/integer/compute.rs b/encodings/roaring/src/integer/compute.rs index ae5c0c7025..8c7699c3bb 100644 --- a/encodings/roaring/src/integer/compute.rs +++ b/encodings/roaring/src/integer/compute.rs @@ -1,7 +1,7 @@ use vortex::compute::unary::ScalarAtFn; use vortex::compute::ArrayCompute; use vortex_dtype::PType; -use vortex_error::{vortex_err, VortexResult}; +use vortex_error::{vortex_err, VortexResult, VortexUnwrap as _}; use vortex_scalar::Scalar; use crate::RoaringIntArray; @@ -29,6 +29,6 @@ impl ScalarAtFn for RoaringIntArray { } fn scalar_at_unchecked(&self, index: usize) -> Scalar { - ::scalar_at(self, index).unwrap() + ::scalar_at(self, index).vortex_unwrap() } } diff --git a/encodings/roaring/src/integer/mod.rs b/encodings/roaring/src/integer/mod.rs index 1e5481393b..1fbedaa471 100644 --- a/encodings/roaring/src/integer/mod.rs +++ b/encodings/roaring/src/integer/mod.rs @@ -14,7 +14,7 @@ use vortex::{ use vortex_buffer::Buffer; use vortex_dtype::Nullability::NonNullable; use vortex_dtype::{DType, PType}; -use vortex_error::{vortex_bail, vortex_err, VortexResult}; +use vortex_error::{vortex_bail, VortexExpect as _, VortexResult}; mod compress; mod compute; @@ -49,7 +49,7 @@ impl RoaringIntArray { Bitmap::deserialize::( self.array() .buffer() - .expect("RoaringBoolArray buffer is missing") + .vortex_expect("RoaringBoolArray buffer is missing") .as_ref(), ) } @@ -62,7 +62,7 @@ impl RoaringIntArray { if array.encoding().id() == Primitive::ID { Ok(roaring_int_encode(PrimitiveArray::try_from(array)?)?.into_array()) } else { - Err(vortex_err!("RoaringInt can only encode primitive arrays")) + vortex_bail!("RoaringInt can only encode primitive arrays") } } } diff --git a/encodings/runend-bool/src/array.rs b/encodings/runend-bool/src/array.rs index 93150ea5a9..c37ac69da1 100644 --- a/encodings/runend-bool/src/array.rs +++ b/encodings/runend-bool/src/array.rs @@ -10,7 +10,7 @@ use vortex::{ IntoCanonical, }; use vortex_dtype::{DType, Nullability}; -use vortex_error::{vortex_bail, VortexResult}; +use vortex_error::{vortex_bail, VortexExpect as _, VortexResult}; use crate::compress::runend_bool_decode; @@ -96,7 +96,7 @@ impl RunEndBoolArray { pub fn ends(&self) -> Array { self.array() .child(0, &self.metadata().ends_dtype, self.metadata().num_runs) - .expect("missing ends") + .vortex_expect("RunEndBoolArray is missing its run ends") } } diff --git a/encodings/runend-bool/src/compress.rs b/encodings/runend-bool/src/compress.rs index f98e2ec7c5..1ecfb4cbc2 100644 --- a/encodings/runend-bool/src/compress.rs +++ b/encodings/runend-bool/src/compress.rs @@ -5,7 +5,7 @@ use num_traits::{AsPrimitive, FromPrimitive}; use vortex::array::{BoolArray, PrimitiveArray}; use vortex::validity::Validity; use vortex_dtype::{match_each_integer_ptype, NativePType}; -use vortex_error::VortexResult; +use vortex_error::{vortex_panic, VortexExpect as _, VortexResult}; pub fn runend_bool_encode(elements: &BoolArray) -> (PrimitiveArray, bool) { let (arr, start) = runend_bool_encode_slice(&elements.boolean_buffer()); @@ -28,7 +28,11 @@ pub fn runend_bool_encode_slice(elements: &BooleanBuffer) -> (Vec, bool) { ends.push(s as u64); ends.push(e as u64); } - if *ends.last().unwrap() != elements.len() as u64 { + + let last_end = ends.last().vortex_expect( + "RunEndBoolArray cannot have empty run ends (by construction); this should be impossible", + ); + if *last_end != elements.len() as u64 { ends.push(elements.len() as u64) } @@ -54,8 +58,20 @@ pub fn runend_bool_decode_slice + FromPrimit offset: usize, length: usize, ) -> Vec { - let offset_e = E::from_usize(offset).unwrap(); - let length_e = E::from_usize(length).unwrap(); + let offset_e = E::from_usize(offset).unwrap_or_else(|| { + vortex_panic!( + "offset {} cannot be converted to {}", + offset, + std::any::type_name::() + ) + }); + let length_e = E::from_usize(length).unwrap_or_else(|| { + vortex_panic!( + "length {} cannot be converted to {}", + length, + std::any::type_name::() + ) + }); let trimmed_ends = run_ends .iter() .map(|v| *v - offset_e) diff --git a/encodings/runend-bool/src/compute.rs b/encodings/runend-bool/src/compute.rs index 9fce236893..ef5b89cfcb 100644 --- a/encodings/runend-bool/src/compute.rs +++ b/encodings/runend-bool/src/compute.rs @@ -3,7 +3,7 @@ use vortex::compute::unary::ScalarAtFn; use vortex::compute::{slice, ArrayCompute, SliceFn, TakeFn}; use vortex::{Array, IntoArray, IntoArrayVariant, ToArray}; use vortex_dtype::match_each_integer_ptype; -use vortex_error::{vortex_bail, VortexResult}; +use vortex_error::{vortex_bail, VortexExpect as _, VortexResult}; use vortex_scalar::Scalar; use crate::compress::value_at_index; @@ -36,7 +36,7 @@ impl ScalarAtFn for RunEndBoolArray { let start = self.start(); Scalar::from(value_at_index( self.find_physical_index(index) - .expect("Search must be implemented for the underlying index array"), + .vortex_expect("Search must be implemented for the underlying index array"), start, )) } diff --git a/encodings/runend/src/compress.rs b/encodings/runend/src/compress.rs index ce5a70d590..f50d6a9a28 100644 --- a/encodings/runend/src/compress.rs +++ b/encodings/runend/src/compress.rs @@ -7,7 +7,7 @@ use vortex::stats::{ArrayStatistics, Stat}; use vortex::validity::Validity; use vortex::ArrayDType; use vortex_dtype::{match_each_integer_ptype, match_each_native_ptype, NativePType, Nullability}; -use vortex_error::VortexResult; +use vortex_error::{vortex_panic, VortexResult}; pub fn runend_encode(array: &PrimitiveArray) -> (PrimitiveArray, PrimitiveArray) { let validity = if array.dtype().nullability() == Nullability::NonNullable { @@ -92,8 +92,20 @@ pub fn runend_decode_primitive< offset: usize, length: usize, ) -> Vec { - let offset_e = E::from_usize(offset).unwrap(); - let length_e = E::from_usize(length).unwrap(); + let offset_e = E::from_usize(offset).unwrap_or_else(|| { + vortex_panic!( + "offset {} cannot be converted to {}", + offset, + std::any::type_name::() + ) + }); + let length_e = E::from_usize(length).unwrap_or_else(|| { + vortex_panic!( + "length {} cannot be converted to {}", + length, + std::any::type_name::() + ) + }); let trimmed_ends = run_ends .iter() .map(|v| *v - offset_e) diff --git a/encodings/runend/src/compute.rs b/encodings/runend/src/compute.rs index 826175fada..9385aede58 100644 --- a/encodings/runend/src/compute.rs +++ b/encodings/runend/src/compute.rs @@ -4,7 +4,7 @@ use vortex::compute::{filter, slice, take, ArrayCompute, SliceFn, TakeFn}; use vortex::validity::Validity; use vortex::{Array, ArrayDType, IntoArray, IntoArrayVariant}; use vortex_dtype::match_each_integer_ptype; -use vortex_error::{vortex_bail, VortexResult}; +use vortex_error::{vortex_bail, VortexExpect as _, VortexResult}; use vortex_scalar::Scalar; use crate::RunEndArray; @@ -31,7 +31,7 @@ impl ScalarAtFn for RunEndArray { fn scalar_at_unchecked(&self, index: usize) -> Scalar { let idx = self .find_physical_index(index) - .expect("Search must be implemented for the underlying index array"); + .vortex_expect("Search must be implemented for the underlying index array"); scalar_at_unchecked(&self.values(), idx) } } diff --git a/encodings/runend/src/runend.rs b/encodings/runend/src/runend.rs index b7d517db70..250a4307b0 100644 --- a/encodings/runend/src/runend.rs +++ b/encodings/runend/src/runend.rs @@ -13,7 +13,7 @@ use vortex::{ IntoCanonical, }; use vortex_dtype::DType; -use vortex_error::{vortex_bail, VortexResult}; +use vortex_error::{vortex_bail, VortexExpect as _, VortexResult}; use crate::compress::{runend_decode, runend_encode}; @@ -113,14 +113,14 @@ impl RunEndArray { pub fn ends(&self) -> Array { self.array() .child(0, &self.metadata().ends_dtype, self.metadata().num_runs) - .expect("missing ends") + .vortex_expect("RunEndArray is missing its run ends") } #[inline] pub fn values(&self) -> Array { self.array() .child(1, self.dtype(), self.metadata().num_runs) - .expect("missing values") + .vortex_expect("RunEndArray is missing its values") } } diff --git a/encodings/zigzag/src/compress.rs b/encodings/zigzag/src/compress.rs index 4586f3533b..41bd53e358 100644 --- a/encodings/zigzag/src/compress.rs +++ b/encodings/zigzag/src/compress.rs @@ -14,7 +14,10 @@ pub fn zigzag_encode(parray: PrimitiveArray) -> VortexResult { PType::I16 => zigzag_encode_primitive::(parray.into_maybe_null_slice(), validity), PType::I32 => zigzag_encode_primitive::(parray.into_maybe_null_slice(), validity), PType::I64 => zigzag_encode_primitive::(parray.into_maybe_null_slice(), validity), - _ => vortex_bail!("Unsupported ptype {}", parray.ptype()), + _ => vortex_bail!( + "ZigZag can only encode signed integers, got {}", + parray.ptype() + ), }; ZigZagArray::try_new(encoded.into_array()) } @@ -29,15 +32,18 @@ where PrimitiveArray::from_vec(values.into_iter().map(|v| T::encode(v)).collect(), validity) } -pub fn zigzag_decode(parray: PrimitiveArray) -> PrimitiveArray { +pub fn zigzag_decode(parray: PrimitiveArray) -> VortexResult { let validity = parray.validity(); - match parray.ptype() { + Ok(match parray.ptype() { PType::U8 => zigzag_decode_primitive::(parray.into_maybe_null_slice(), validity), PType::U16 => zigzag_decode_primitive::(parray.into_maybe_null_slice(), validity), PType::U32 => zigzag_decode_primitive::(parray.into_maybe_null_slice(), validity), PType::U64 => zigzag_decode_primitive::(parray.into_maybe_null_slice(), validity), - _ => panic!("Unsupported ptype {}", parray.ptype()), - } + _ => vortex_bail!( + "ZigZag can only decode unsigned integers, got {}", + parray.ptype() + ), + }) } fn zigzag_decode_primitive( diff --git a/encodings/zigzag/src/compute.rs b/encodings/zigzag/src/compute.rs index 028966a080..6eb8517676 100644 --- a/encodings/zigzag/src/compute.rs +++ b/encodings/zigzag/src/compute.rs @@ -2,7 +2,7 @@ use vortex::compute::unary::{scalar_at_unchecked, ScalarAtFn}; use vortex::compute::{slice, ArrayCompute, SliceFn}; use vortex::{Array, ArrayDType, IntoArray}; use vortex_dtype::match_each_unsigned_integer_ptype; -use vortex_error::{vortex_err, VortexResult}; +use vortex_error::{vortex_err, VortexResult, VortexUnwrap as _}; use vortex_scalar::{PrimitiveScalar, Scalar}; use zigzag::{ZigZag as ExternalZigZag, ZigZag}; @@ -30,7 +30,8 @@ impl ScalarAtFn for ZigZagArray { Ok(Scalar::primitive( <<$P as ZigZagEncoded>::Int>::decode(pscalar.typed_value::<$P>().ok_or_else(|| { vortex_err!( - "Cannot decode provided scalar: expected u8, got ptype {}", + "Cannot decode provided scalar: expected {}, got ptype {}", + std::any::type_name::<$P>(), pscalar.ptype() ) })?), @@ -40,7 +41,7 @@ impl ScalarAtFn for ZigZagArray { } fn scalar_at_unchecked(&self, index: usize) -> Scalar { - ::scalar_at(self, index).unwrap() + ::scalar_at(self, index).vortex_unwrap() } } diff --git a/encodings/zigzag/src/zigzag.rs b/encodings/zigzag/src/zigzag.rs index 3a06dadb0d..86826ffa80 100644 --- a/encodings/zigzag/src/zigzag.rs +++ b/encodings/zigzag/src/zigzag.rs @@ -9,7 +9,9 @@ use vortex::{ IntoCanonical, }; use vortex_dtype::{DType, PType}; -use vortex_error::{vortex_bail, vortex_err, VortexResult}; +use vortex_error::{ + vortex_bail, vortex_err, vortex_panic, VortexExpect as _, VortexResult, VortexUnwrap as _, +}; use crate::compress::zigzag_encode; use crate::zigzag_decode; @@ -43,15 +45,17 @@ impl ZigZagArray { } pub fn encoded(&self) -> Array { - let ptype = PType::try_from(self.dtype()).expect("ptype"); + let ptype = PType::try_from(self.dtype()).unwrap_or_else(|err| { + vortex_panic!(err, "Failed to convert DType {} to PType", self.dtype()) + }); let encoded = DType::from(ptype.to_unsigned()).with_nullability(self.dtype().nullability()); self.array() .child(0, &encoded, self.len()) - .expect("Missing encoded array") + .vortex_expect("ZigZagArray is missing its encoded child array") } pub fn ptype(&self) -> PType { - PType::try_from(self.dtype()).expect("must be a ptype") + PType::try_from(self.dtype()).vortex_unwrap() } } @@ -85,8 +89,6 @@ impl ArrayStatisticsCompute for ZigZagArray {} impl IntoCanonical for ZigZagArray { fn into_canonical(self) -> VortexResult { - Ok(Canonical::Primitive(zigzag_decode( - self.encoded().into_primitive()?, - ))) + zigzag_decode(self.encoded().into_primitive()?).map(Canonical::Primitive) } } diff --git a/pyvortex/src/array.rs b/pyvortex/src/array.rs index 01baa833a8..3802edfcd0 100644 --- a/pyvortex/src/array.rs +++ b/pyvortex/src/array.rs @@ -56,10 +56,10 @@ impl PyArray { let chunks: Vec = chunked_array .chunks() .map(|chunk| -> PyResult { - Ok(chunk + chunk .into_canonical() - .map_err(PyVortexError::map_err)? - .into_arrow()) + .and_then(|arr| arr.into_arrow()) + .map_err(PyVortexError::map_err) }) .collect::>>()?; if chunks.is_empty() { @@ -81,8 +81,8 @@ impl PyArray { Ok(vortex .clone() .into_canonical() + .and_then(|arr| arr.into_arrow()) .map_err(PyVortexError::map_err)? - .into_arrow() .into_data() .to_pyarrow(py)? .into_bound(py)) diff --git a/pyvortex/src/encode.rs b/pyvortex/src/encode.rs index ced01879cc..028ba95524 100644 --- a/pyvortex/src/encode.rs +++ b/pyvortex/src/encode.rs @@ -9,6 +9,7 @@ use vortex::array::ChunkedArray; use vortex::arrow::{FromArrowArray, FromArrowType}; use vortex::{Array, IntoArray}; use vortex_dtype::DType; +use vortex_error::VortexError; use crate::array::PyArray; use crate::error::PyVortexError; @@ -53,10 +54,8 @@ pub fn _encode<'py>(obj: &Bound<'py, PyAny>) -> PyResult> { let dtype = DType::from_arrow(array_stream.schema()); let chunks = array_stream .into_iter() - .map(|b| { - b.map(Array::from) - .map_err(|e| PyValueError::new_err(e.to_string())) - }) + .map(|b| b.map_err(VortexError::ArrowError)) + .map(|b| b.and_then(Array::try_from).map_err(PyVortexError::map_err)) .collect::>>()?; Bound::new( obj.py(), @@ -67,6 +66,8 @@ pub fn _encode<'py>(obj: &Bound<'py, PyAny>) -> PyResult> { ), ) } else { - Err(PyValueError::new_err("Cannot convert object to enc array")) + Err(PyValueError::new_err( + "Cannot convert object to Vortex array", + )) } } diff --git a/pyvortex/src/error.rs b/pyvortex/src/error.rs index ae27695909..0c81136455 100644 --- a/pyvortex/src/error.rs +++ b/pyvortex/src/error.rs @@ -14,6 +14,12 @@ impl PyVortexError { } } +impl From for PyVortexError { + fn from(val: VortexError) -> Self { + PyVortexError::new(val) + } +} + impl From for PyErr { fn from(value: PyVortexError) -> Self { PyValueError::new_err(value.0.to_string()) diff --git a/pyvortex/src/io.rs b/pyvortex/src/io.rs index 36709b880c..95eee00574 100644 --- a/pyvortex/src/io.rs +++ b/pyvortex/src/io.rs @@ -9,7 +9,7 @@ use tokio::fs::File; use vortex::array::ChunkedArray; use vortex::{Array, Context}; use vortex_dtype::field::Field; -use vortex_error::VortexResult; +use vortex_error::{vortex_panic, VortexResult}; use vortex_serde::layouts::{ LayoutContext, LayoutDeserializer, LayoutReaderBuilder, LayoutWriter, Projection, RowFilter, }; @@ -157,7 +157,11 @@ pub fn read<'py>( let vecs: Vec = stream.try_collect().await?; if vecs.len() == 1 { - Ok(vecs.into_iter().next().unwrap()) + vecs.into_iter().next().ok_or_else(|| { + vortex_panic!( + "Should be impossible: vecs.len() == 1 but couldn't get first element" + ) + }) } else { ChunkedArray::try_new(vecs, dtype).map(|e| e.into()) } diff --git a/vortex-array/benches/compare.rs b/vortex-array/benches/compare.rs index e29446abbe..aad1cb86c6 100644 --- a/vortex-array/benches/compare.rs +++ b/vortex-array/benches/compare.rs @@ -1,3 +1,5 @@ +#![allow(clippy::unwrap_used)] + use criterion::{black_box, criterion_group, criterion_main, Criterion}; use itertools::Itertools; use rand::distributions::Uniform; diff --git a/vortex-array/benches/fn.rs b/vortex-array/benches/fn.rs index d5bcc07091..c07c48462b 100644 --- a/vortex-array/benches/fn.rs +++ b/vortex-array/benches/fn.rs @@ -1,3 +1,5 @@ +#![allow(clippy::unwrap_used)] + use arrow_array::types::UInt32Type; use arrow_array::UInt32Array; use criterion::{criterion_group, criterion_main, BatchSize, Criterion}; diff --git a/vortex-array/benches/iter.rs b/vortex-array/benches/iter.rs index a919e7b18e..c2d8682d9b 100644 --- a/vortex-array/benches/iter.rs +++ b/vortex-array/benches/iter.rs @@ -1,3 +1,5 @@ +#![allow(clippy::unwrap_used)] + use criterion::{criterion_group, criterion_main, BatchSize, Criterion}; use itertools::Itertools; use vortex::array::PrimitiveArray; diff --git a/vortex-array/benches/scalar_subtract.rs b/vortex-array/benches/scalar_subtract.rs index f6608f850e..977be839e8 100644 --- a/vortex-array/benches/scalar_subtract.rs +++ b/vortex-array/benches/scalar_subtract.rs @@ -1,3 +1,5 @@ +#![allow(clippy::unwrap_used)] + use criterion::{black_box, criterion_group, criterion_main, Criterion}; use itertools::Itertools; use rand::distributions::Uniform; diff --git a/vortex-array/src/array/bool/compute/boolean.rs b/vortex-array/src/array/bool/compute/boolean.rs index 091fa72c52..baf1035998 100644 --- a/vortex-array/src/array/bool/compute/boolean.rs +++ b/vortex-array/src/array/bool/compute/boolean.rs @@ -9,10 +9,10 @@ use crate::{Array, IntoCanonical}; impl OrFn for BoolArray { fn or(&self, array: &Array) -> VortexResult { - let lhs = self.clone().into_canonical()?.into_arrow(); + let lhs = self.clone().into_canonical()?.into_arrow()?; let lhs = lhs.as_boolean(); - let rhs = array.clone().into_canonical()?.into_arrow(); + let rhs = array.clone().into_canonical()?.into_arrow()?; let rhs = rhs.as_boolean(); let array = boolean::or(lhs, rhs)?; @@ -23,10 +23,10 @@ impl OrFn for BoolArray { impl AndFn for BoolArray { fn and(&self, array: &Array) -> VortexResult { - let lhs = self.clone().into_canonical()?.into_arrow(); + let lhs = self.clone().into_canonical()?.into_arrow()?; let lhs = lhs.as_boolean(); - let rhs = array.clone().into_canonical()?.into_arrow(); + let rhs = array.clone().into_canonical()?.into_arrow()?; let rhs = rhs.as_boolean(); let array = boolean::and(lhs, rhs)?; diff --git a/vortex-array/src/array/bool/mod.rs b/vortex-array/src/array/bool/mod.rs index 26065bb8b7..34c9ccb87e 100644 --- a/vortex-array/src/array/bool/mod.rs +++ b/vortex-array/src/array/bool/mod.rs @@ -4,7 +4,7 @@ use itertools::Itertools; use serde::{Deserialize, Serialize}; use vortex_buffer::Buffer; use vortex_dtype::DType; -use vortex_error::VortexResult; +use vortex_error::{VortexExpect as _, VortexResult}; use crate::stats::StatsSet; use crate::validity::{ArrayValidity, LogicalValidity, Validity, ValidityMetadata}; @@ -27,7 +27,9 @@ pub struct BoolMetadata { impl BoolArray { pub fn buffer(&self) -> &Buffer { - self.array().buffer().expect("missing buffer") + self.array() + .buffer() + .vortex_expect("Missing buffer in BoolArray") } pub fn boolean_buffer(&self) -> BooleanBuffer { @@ -72,8 +74,7 @@ impl BoolArray { pub fn from_vec(bools: Vec, validity: Validity) -> Self { let buffer = BooleanBuffer::from(bools); - Self::try_new(buffer, validity) - .unwrap_or_else(|err| panic!("Failed to create BoolArray from vec: {}", err)) + Self::try_new(buffer, validity).vortex_expect("Failed to create BoolArray from vec") } } @@ -98,7 +99,7 @@ impl BoolArrayTrait for BoolArray { impl From for BoolArray { fn from(value: BooleanBuffer) -> Self { Self::try_new(value, Validity::NonNullable) - .unwrap_or_else(|err| panic!("Failed to create BoolArray from BooleanBuffer: {}", err)) + .vortex_expect("Failed to create BoolArray from BooleanBuffer") } } @@ -121,12 +122,8 @@ impl FromIterator> for BoolArray { }) .collect::>(); - Self::try_new(BooleanBuffer::from(values), Validity::from(validity)).unwrap_or_else(|err| { - panic!( - "Failed to create BoolArray from iterator of Option: {}", - err - ) - }) + Self::try_new(BooleanBuffer::from(values), Validity::from(validity)) + .vortex_expect("Failed to create BoolArray from iterator of Option") } } diff --git a/vortex-array/src/array/chunked/canonical.rs b/vortex-array/src/array/chunked/canonical.rs index fff2df73d2..a4bf4d38d3 100644 --- a/vortex-array/src/array/chunked/canonical.rs +++ b/vortex-array/src/array/chunked/canonical.rs @@ -152,7 +152,7 @@ fn swizzle_struct_chunks( field_chunks.push( chunk .field(field_idx) - .expect("all chunks must have same dtype"), + .ok_or_else(|| vortex_err!("All chunks must have same dtype; missing field at index {}, current chunk dtype: {}", field_idx, chunk.dtype()))?, ); } let field_array = ChunkedArray::try_new(field_chunks, field_dtype.clone())?; @@ -230,7 +230,9 @@ fn pack_varbin(chunks: &[Array], validity: Validity, dtype: &DType) -> VortexRes slice(&chunk.bytes(), first_offset_value, last_offset_value)?.into_primitive()?; data_bytes.extend_from_slice(primitive_bytes.buffer()); - let adjustment_from_previous = *offsets.last().expect("offsets has at least one element"); + let adjustment_from_previous = *offsets + .last() + .ok_or_else(|| vortex_err!("VarBinArray offsets must have at least one element"))?; offsets.extend( offsets_arr .maybe_null_slice::() diff --git a/vortex-array/src/array/chunked/compute/mod.rs b/vortex-array/src/array/chunked/compute/mod.rs index e8be9d1e15..fc90a85aa4 100644 --- a/vortex-array/src/array/chunked/compute/mod.rs +++ b/vortex-array/src/array/chunked/compute/mod.rs @@ -1,5 +1,5 @@ use vortex_dtype::{DType, Nullability}; -use vortex_error::{vortex_err, VortexResult}; +use vortex_error::{vortex_err, vortex_panic, VortexResult}; use vortex_scalar::Scalar; use crate::array::chunked::ChunkedArray; @@ -51,7 +51,12 @@ impl ScalarAtFn for ChunkedArray { fn scalar_at_unchecked(&self, index: usize) -> Scalar { let (chunk_index, chunk_offset) = self.find_chunk_idx(index); - scalar_at_unchecked(&self.chunk(chunk_index).unwrap(), chunk_offset) + scalar_at_unchecked( + &self + .chunk(chunk_index) + .unwrap_or_else(|| vortex_panic!(OutOfBounds: chunk_index, 0, self.nchunks())), + chunk_offset, + ) } } diff --git a/vortex-array/src/array/chunked/compute/take.rs b/vortex-array/src/array/chunked/compute/take.rs index e23bbaff7c..cb595f1201 100644 --- a/vortex-array/src/array/chunked/compute/take.rs +++ b/vortex-array/src/array/chunked/compute/take.rs @@ -95,9 +95,7 @@ fn take_strict_sorted(chunked: &ChunkedArray, indices: &Array) -> VortexResult VortexResult Array { self.array() .child(0, &Self::ENDS_DTYPE, self.nchunks() + 1) - .expect("missing chunk ends") + .vortex_expect("Missing chunk ends in ChunkedArray") } pub fn find_chunk_idx(&self, index: usize) -> (usize, usize) { assert!(index <= self.len(), "Index out of bounds of the array"); let search_result = search_sorted(&self.chunk_offsets(), index, SearchSortedSide::Left) - .unwrap_or_else(|err| { - panic!("Search sorted failed in find_chunk_idx: {}", err); - }); + .vortex_expect("Search sorted failed in find_chunk_idx"); let index_chunk = match search_result { SearchResult::Found(i) => { if i == self.nchunks() { @@ -109,9 +107,7 @@ impl ChunkedArray { }; let chunk_start = &scalar_at(&self.chunk_offsets(), index_chunk) .and_then(|s| usize::try_from(&s)) - .unwrap_or_else(|err| { - panic!("Failed to find chunk start in find_chunk_idx: {}", err); - }); + .vortex_expect("Failed to find chunk start in find_chunk_idx"); let index_in_chunk = index - chunk_start; (index_chunk, index_in_chunk) @@ -120,11 +116,11 @@ impl ChunkedArray { pub fn chunks(&self) -> impl Iterator + '_ { (0..self.nchunks()).map(|c| { self.chunk(c).unwrap_or_else(|| { - panic!( + vortex_panic!( "Chunk should {} exist but doesn't (nchunks: {})", c, self.nchunks() - ); + ) }) }) } @@ -146,10 +142,8 @@ impl FromIterator for ChunkedArray { let dtype = chunks .first() .map(|c| c.dtype().clone()) - .expect("Cannot create a chunked array from an empty iterator"); - Self::try_new(chunks, dtype).unwrap_or_else(|err| { - panic!("Failed to create chunked array from iterator: {}", err); - }) + .vortex_expect("Cannot infer DType from an empty iterator"); + Self::try_new(chunks, dtype).vortex_expect("Failed to create chunked array from iterator") } } @@ -167,7 +161,7 @@ impl ArrayValidity for ChunkedArray { fn is_valid(&self, index: usize) -> bool { let (chunk, offset_in_chunk) = self.find_chunk_idx(index); self.chunk(chunk) - .expect("must be a valid chunk index") + .unwrap_or_else(|| vortex_panic!(OutOfBounds: chunk, 0, self.nchunks())) .with_dyn(|a| a.is_valid(offset_in_chunk)) } diff --git a/vortex-array/src/array/chunked/variants.rs b/vortex-array/src/array/chunked/variants.rs index 45547ed9ef..1d820f0725 100644 --- a/vortex-array/src/array/chunked/variants.rs +++ b/vortex-array/src/array/chunked/variants.rs @@ -1,4 +1,5 @@ use vortex_dtype::DType; +use vortex_error::vortex_panic; use crate::array::chunked::ChunkedArray; use crate::variants::{ @@ -71,9 +72,10 @@ impl StructArrayTrait for ChunkedArray { let projected_dtype = self.dtype().as_struct().and_then(|s| s.dtypes().get(idx))?; let chunked = ChunkedArray::try_new(chunks, projected_dtype.clone()) .unwrap_or_else(|err| { - panic!( - "Failed to create new chunked array with dtype {}: {}", - projected_dtype, err + vortex_panic!( + err, + "Failed to create new chunked array with dtype {}", + projected_dtype ) }) .into_array(); diff --git a/vortex-array/src/array/constant/compute.rs b/vortex-array/src/array/constant/compute.rs index e676912c21..bb56f191aa 100644 --- a/vortex-array/src/array/constant/compute.rs +++ b/vortex-array/src/array/constant/compute.rs @@ -101,7 +101,7 @@ impl SearchSortedFn for ConstantArray { impl CompareFn for ConstantArray { fn compare(&self, rhs: &Array, operator: Operator) -> VortexResult { - if let Some(true) = rhs.statistics().get_as::(Stat::IsConstant) { + if rhs.statistics().get_as::(Stat::IsConstant) == Some(true) { let lhs = self.scalar(); let rhs = scalar_at(rhs, 0)?; @@ -109,8 +109,8 @@ impl CompareFn for ConstantArray { Ok(ConstantArray::new(scalar, self.len()).into_array()) } else { - let datum = Arc::::from(self.scalar()); - let rhs = rhs.clone().into_canonical()?.into_arrow(); + let datum = Arc::::try_from(self.scalar())?; + let rhs = rhs.clone().into_canonical()?.into_arrow()?; let rhs = rhs.as_ref(); let boolean_array = match operator { @@ -156,7 +156,7 @@ fn constant_array_bool_impl( fallback_fn: impl Fn(&Array, &Array) -> Option>, ) -> VortexResult { // If the right side is constant - if let Some(true) = other.statistics().get_as::(Stat::IsConstant) { + if other.statistics().get_as::(Stat::IsConstant) == Some(true) { let lhs = constant_array.scalar().value().as_bool()?; let rhs = scalar_at(other, 0)?.value().as_bool()?; diff --git a/vortex-array/src/array/constant/mod.rs b/vortex-array/src/array/constant/mod.rs index 1c07ee376e..f1e4052116 100644 --- a/vortex-array/src/array/constant/mod.rs +++ b/vortex-array/src/array/constant/mod.rs @@ -1,7 +1,7 @@ use std::collections::HashMap; use serde::{Deserialize, Serialize}; -use vortex_error::VortexResult; +use vortex_error::{vortex_panic, VortexResult}; use vortex_scalar::Scalar; use crate::stats::{Stat, StatsSet}; @@ -37,6 +37,7 @@ impl ConstantArray { (Stat::IsSorted, true.into()), (Stat::RunCount, 1.into()), ])); + Self::try_from_parts( scalar.dtype().clone(), length, @@ -48,10 +49,12 @@ impl ConstantArray { stats, ) .unwrap_or_else(|err| { - panic!( - "Failed to create Constant array of length {} from scalar {}: {}", - length, scalar, err - ); + vortex_panic!( + err, + "Failed to create Constant array of length {} from scalar {}", + length, + scalar + ) }) } diff --git a/vortex-array/src/array/constant/variants.rs b/vortex-array/src/array/constant/variants.rs index 6e1344dff8..39a8d5687b 100644 --- a/vortex-array/src/array/constant/variants.rs +++ b/vortex-array/src/array/constant/variants.rs @@ -2,7 +2,7 @@ use std::iter; use std::sync::Arc; use vortex_dtype::{DType, PType}; -use vortex_error::VortexError; +use vortex_error::{VortexError, VortexExpect as _}; use vortex_scalar::{Scalar, StructScalar}; use crate::array::constant::ConstantArray; @@ -53,9 +53,11 @@ impl NullArrayTrait for ConstantArray {} impl BoolArrayTrait for ConstantArray { fn maybe_null_indices_iter(&self) -> Box> { - let value = self.scalar().value().as_bool().unwrap_or_else(|err| { - panic!("Failed to get bool value from constant array: {}", err); - }); + let value = self + .scalar() + .value() + .as_bool() + .vortex_expect("Failed to get bool value from constant array"); if value.unwrap_or(false) { Box::new(0..self.len()) } else { @@ -65,9 +67,11 @@ impl BoolArrayTrait for ConstantArray { fn maybe_null_slices_iter(&self) -> Box> { // Must be a boolean scalar - let value = self.scalar().value().as_bool().unwrap_or_else(|err| { - panic!("Failed to get bool value from constant array: {}", err); - }); + let value = self + .scalar() + .value() + .as_bool() + .vortex_expect("Failed to get bool value from constant array"); if value.unwrap_or(false) { Box::new(iter::once((0, self.len()))) @@ -91,7 +95,7 @@ where } fn value_unchecked(&self, _index: usize) -> T { - T::try_from(self.scalar().clone()).unwrap() + T::try_from(self.scalar().clone()).vortex_expect("Failed to convert scalar to value") } fn array_validity(&self) -> Validity { diff --git a/vortex-array/src/array/datetime/mod.rs b/vortex-array/src/array/datetime/mod.rs index 688d9901e1..ddb2392cf3 100644 --- a/vortex-array/src/array/datetime/mod.rs +++ b/vortex-array/src/array/datetime/mod.rs @@ -3,7 +3,7 @@ mod test; use vortex_datetime_dtype::{TemporalMetadata, TimeUnit, DATE_ID, TIMESTAMP_ID, TIME_ID}; use vortex_dtype::{DType, ExtDType}; -use vortex_error::VortexError; +use vortex_error::{vortex_panic, VortexError}; use crate::array::ExtensionArray; use crate::{Array, ArrayDType, ArrayData, IntoArray, ToArrayData}; @@ -84,7 +84,7 @@ impl TemporalArray { Some(TemporalMetadata::Date(time_unit).into()), ) } - _ => panic!("invalid TimeUnit {time_unit} for vortex.date"), + _ => vortex_panic!("invalid TimeUnit {time_unit} for vortex.date"), }; Self { @@ -116,7 +116,7 @@ impl TemporalArray { match time_unit { TimeUnit::S | TimeUnit::Ms => assert_width!(i32, array), TimeUnit::Us | TimeUnit::Ns => assert_width!(i64, array), - TimeUnit::D => panic!("invalid unit D for vortex.time data"), + TimeUnit::D => vortex_panic!("invalid unit D for vortex.time data"), } let temporal_metadata = TemporalMetadata::Time(time_unit); diff --git a/vortex-array/src/array/extension/mod.rs b/vortex-array/src/array/extension/mod.rs index 8f3f11e0db..8e2783060a 100644 --- a/vortex-array/src/array/extension/mod.rs +++ b/vortex-array/src/array/extension/mod.rs @@ -1,6 +1,6 @@ use serde::{Deserialize, Serialize}; use vortex_dtype::{DType, ExtDType, ExtID}; -use vortex_error::VortexResult; +use vortex_error::{VortexExpect as _, VortexResult}; use crate::stats::ArrayStatisticsCompute; use crate::validity::{ArrayValidity, LogicalValidity}; @@ -28,13 +28,13 @@ impl ExtensionArray { [storage].into(), Default::default(), ) - .expect("Invalid ExtensionArray") + .vortex_expect("Invalid ExtensionArray") } pub fn storage(&self) -> Array { self.array() .child(0, &self.metadata().storage_dtype, self.len()) - .expect("Missing storage array") + .vortex_expect("Missing storage array for ExtensionArray") } #[allow(dead_code)] diff --git a/vortex-array/src/array/null/mod.rs b/vortex-array/src/array/null/mod.rs index 7bb406dce0..708d90d3cb 100644 --- a/vortex-array/src/array/null/mod.rs +++ b/vortex-array/src/array/null/mod.rs @@ -2,7 +2,7 @@ use std::sync::Arc; use serde::{Deserialize, Serialize}; use vortex_dtype::DType; -use vortex_error::VortexResult; +use vortex_error::{VortexExpect as _, VortexResult}; use crate::stats::{ArrayStatisticsCompute, Stat, StatsSet}; use crate::validity::{ArrayValidity, LogicalValidity, Validity}; @@ -28,7 +28,7 @@ impl NullArray { Arc::new([]), StatsSet::nulls(len, &DType::Null), ) - .expect("NullArray::new cannot fail") + .vortex_expect("NullArray::new should never fail!") } } diff --git a/vortex-array/src/array/primitive/compute/take.rs b/vortex-array/src/array/primitive/compute/take.rs index 2603dcedbc..7564cd7f89 100644 --- a/vortex-array/src/array/primitive/compute/take.rs +++ b/vortex-array/src/array/primitive/compute/take.rs @@ -1,6 +1,6 @@ use num_traits::PrimInt; use vortex_dtype::{match_each_integer_ptype, match_each_native_ptype, NativePType}; -use vortex_error::VortexResult; +use vortex_error::{vortex_panic, VortexResult}; use crate::array::primitive::PrimitiveArray; use crate::compute::TakeFn; @@ -26,7 +26,7 @@ fn take_primitive(array: &[T], indices .iter() .map(|&idx| { array[idx.to_usize().unwrap_or_else(|| { - panic!("Failed to convert index to usize: {}", idx); + vortex_panic!("Failed to convert index to usize: {}", idx); })] }) .collect() diff --git a/vortex-array/src/array/primitive/mod.rs b/vortex-array/src/array/primitive/mod.rs index 37a44d99d9..6f0d0ef0d7 100644 --- a/vortex-array/src/array/primitive/mod.rs +++ b/vortex-array/src/array/primitive/mod.rs @@ -9,7 +9,7 @@ use num_traits::AsPrimitive; use serde::{Deserialize, Serialize}; use vortex_buffer::Buffer; use vortex_dtype::{match_each_native_ptype, DType, NativePType, PType}; -use vortex_error::{vortex_bail, VortexResult}; +use vortex_error::{vortex_bail, vortex_panic, VortexError, VortexExpect as _, VortexResult}; use crate::elementwise::{dyn_cast_array_iter, BinaryFn, UnaryFn}; use crate::iter::{Accessor, AccessorRef, Batch, ITER_BATCH_SIZE}; @@ -49,13 +49,15 @@ impl PrimitiveArray { DType::from(ptype).with_nullability(validity.nullability()), length, PrimitiveMetadata { - validity: validity.to_metadata(length).expect("invalid validity"), + validity: validity + .to_metadata(length) + .vortex_expect("Invalid validity"), }, Some(buffer), validity.into_array().into_iter().collect_vec().into(), StatsSet::new(), ) - .expect("should be valid"), + .vortex_expect("PrimitiveArray::new should never fail!"), } } @@ -90,13 +92,15 @@ impl PrimitiveArray { pub fn ptype(&self) -> PType { // TODO(ngates): we can't really cache this anywhere? - self.dtype().try_into().unwrap_or_else(|err| { - panic!("Failed to convert dtype {} to ptype: {}", self.dtype(), err); + self.dtype().try_into().unwrap_or_else(|err: VortexError| { + vortex_panic!(err, "Failed to convert dtype {} to ptype", self.dtype()) }) } pub fn buffer(&self) -> &Buffer { - self.array().buffer().expect("missing buffer") + self.array() + .buffer() + .vortex_expect("Missing buffer in PrimitiveArray") } pub fn maybe_null_slice(&self) -> &[T] { @@ -172,7 +176,7 @@ impl PrimitiveArray { pub fn into_buffer(self) -> Buffer { self.into_array() .into_buffer() - .expect("PrimitiveArray must have a buffer") + .vortex_expect("PrimitiveArray must have a buffer") } } @@ -312,7 +316,7 @@ impl AcceptArrayVisitor for PrimitiveArray { impl Array { pub fn as_primitive(&self) -> PrimitiveArray { - PrimitiveArray::try_from(self).expect("expected primitive array") + PrimitiveArray::try_from(self).vortex_expect("Expected primitive array") } } @@ -413,6 +417,7 @@ impl BinaryFn for PrimitiveArray { } } +#[allow(clippy::unwrap_used)] fn process_batch O>( lhs: &[I], batch: Batch, diff --git a/vortex-array/src/array/sparse/compute/mod.rs b/vortex-array/src/array/sparse/compute/mod.rs index eda604a9a1..b437c0a0ed 100644 --- a/vortex-array/src/array/sparse/compute/mod.rs +++ b/vortex-array/src/array/sparse/compute/mod.rs @@ -1,4 +1,4 @@ -use vortex_error::VortexResult; +use vortex_error::{VortexResult, VortexUnwrap as _}; use vortex_scalar::Scalar; use crate::array::sparse::SparseArray; @@ -38,14 +38,11 @@ impl ScalarAtFn for SparseArray { } fn scalar_at_unchecked(&self, index: usize) -> Scalar { - match self - .find_index(index) - .expect("Must be able to find the index") - { - None => self.fill_value().clone().cast(self.dtype()).unwrap(), + match self.find_index(index).vortex_unwrap() { + None => self.fill_value().clone().cast(self.dtype()).vortex_unwrap(), Some(idx) => scalar_at_unchecked(&self.values(), idx) .cast(self.dtype()) - .unwrap(), + .vortex_unwrap(), } } } diff --git a/vortex-array/src/array/sparse/mod.rs b/vortex-array/src/array/sparse/mod.rs index a54b436ccb..ed723cc653 100644 --- a/vortex-array/src/array/sparse/mod.rs +++ b/vortex-array/src/array/sparse/mod.rs @@ -1,6 +1,6 @@ use ::serde::{Deserialize, Serialize}; use vortex_dtype::{match_each_integer_ptype, DType}; -use vortex_error::{vortex_bail, VortexResult}; +use vortex_error::{vortex_bail, vortex_panic, VortexExpect as _, VortexResult}; use vortex_scalar::Scalar; use crate::array::constant::ConstantArray; @@ -96,7 +96,7 @@ impl SparseArray { pub fn values(&self) -> Array { self.array() .child(1, self.dtype(), self.metadata().indices_len) - .expect("missing child array") + .vortex_expect("Missing child array in SparseArray") } #[inline] @@ -107,7 +107,7 @@ impl SparseArray { &self.metadata().indices_dtype, self.metadata().indices_len, ) - .expect("missing indices array") + .vortex_expect("Missing indices array in SparseArray") } #[inline] @@ -127,9 +127,10 @@ impl SparseArray { /// Return indices as a vector of usize with the indices_offset applied. pub fn resolved_indices(&self) -> Vec { - let flat_indices = self.indices().into_primitive().unwrap_or_else(|err| { - panic!("Failed to convert indices to primitive array: {}", err); - }); + let flat_indices = self + .indices() + .into_primitive() + .vortex_expect("Failed to convert SparseArray indices to primitive array"); match_each_integer_ptype!(flat_indices.ptype(), |$P| { flat_indices .maybe_null_slice::<$P>() @@ -142,9 +143,7 @@ impl SparseArray { pub fn min_index(&self) -> usize { let min_index: usize = scalar_at(&self.indices(), 0) .and_then(|s| s.as_ref().try_into()) - .unwrap_or_else(|err| { - panic!("Failed to get min_index: {}", err); - }); + .vortex_expect("Failed to get min_index from SparseArray"); min_index - self.indices_offset() } } @@ -162,14 +161,10 @@ impl ArrayStatisticsCompute for SparseArray {} impl ArrayValidity for SparseArray { fn is_valid(&self, index: usize) -> bool { - match self.find_index(index).unwrap_or_else(|err| { - panic!( - "Error while finding index {} in sparse array: {}", - index, err - ); - }) { - None => !self.fill_value().is_null(), - Some(idx) => self.values().with_dyn(|a| a.is_valid(idx)), + match self.find_index(index) { + Ok(None) => !self.fill_value().is_null(), + Ok(Some(idx)) => self.values().with_dyn(|a| a.is_valid(idx)), + Err(e) => vortex_panic!(e, "Error while finding index {} in sparse array", index), } } @@ -196,13 +191,7 @@ impl ArrayValidity for SparseArray { false.into(), ) } - .unwrap_or_else(|err| { - panic!( - "Error determining logical validity for sparse array: {}", - err - ); - }); - + .vortex_expect("Error determining logical validity for sparse array"); LogicalValidity::Array(validity.into_array()) } } diff --git a/vortex-array/src/array/struct_/mod.rs b/vortex-array/src/array/struct_/mod.rs index e6d0486301..74860540b0 100644 --- a/vortex-array/src/array/struct_/mod.rs +++ b/vortex-array/src/array/struct_/mod.rs @@ -1,7 +1,7 @@ use serde::{Deserialize, Serialize}; use vortex_dtype::field::Field; use vortex_dtype::{DType, FieldName, FieldNames, StructDType}; -use vortex_error::{vortex_bail, vortex_err, VortexResult}; +use vortex_error::{vortex_bail, vortex_err, vortex_panic, VortexExpect as _, VortexResult}; use crate::stats::{ArrayStatisticsCompute, StatsSet}; use crate::validity::{ArrayValidity, LogicalValidity, Validity, ValidityMetadata}; @@ -30,8 +30,9 @@ impl StructArray { pub fn children(&self) -> impl Iterator + '_ { (0..self.nfields()).map(move |idx| { - self.field(idx) - .unwrap_or_else(|| panic!("Field {} not found, nfields: {}", idx, self.nfields())) + self.field(idx).unwrap_or_else(|| { + vortex_panic!("Field {} not found, nfields: {}", idx, self.nfields()) + }) }) } @@ -79,10 +80,10 @@ impl StructArray { .map(|(name, _)| FieldName::from(name.as_ref())) .collect(); let fields: Vec = items.iter().map(|(_, array)| array.clone()).collect(); - let len = fields.first().unwrap().len(); + let len = fields.first().map(|f| f.len()).unwrap_or(0); Self::try_new(FieldNames::from(names), fields, len, Validity::NonNullable) - .expect("building StructArray with helper") + .vortex_expect("Unexpected error while building StructArray from fields") } // TODO(aduffy): Add equivalent function to support field masks for nested column access. diff --git a/vortex-array/src/array/varbin/builder.rs b/vortex-array/src/array/varbin/builder.rs index f1018b588f..4be72d0d68 100644 --- a/vortex-array/src/array/varbin/builder.rs +++ b/vortex-array/src/array/varbin/builder.rs @@ -2,6 +2,7 @@ use arrow_buffer::NullBufferBuilder; use bytes::BytesMut; use num_traits::AsPrimitive; use vortex_dtype::{DType, NativePType}; +use vortex_error::{vortex_panic, VortexExpect as _}; use crate::array::primitive::PrimitiveArray; use crate::array::varbin::VarBinArray; @@ -37,7 +38,14 @@ impl VarBinBuilder { pub fn push_value(&mut self, value: impl AsRef<[u8]>) { let slice = value.as_ref(); self.offsets - .push(O::from(self.data.len() + slice.len()).unwrap()); + .push(O::from(self.data.len() + slice.len()).unwrap_or_else(|| { + vortex_panic!( + "Failed to convert sum of {} and {} to offset of type {}", + self.data.len(), + slice.len(), + std::any::type_name::() + ) + })); self.data.extend_from_slice(slice); self.validity.append_non_null(); } @@ -72,7 +80,8 @@ impl VarBinBuilder { Validity::NonNullable }; - VarBinArray::try_new(offsets.into_array(), data.into_array(), dtype, validity).unwrap() + VarBinArray::try_new(offsets.into_array(), data.into_array(), dtype, validity) + .vortex_expect("Unexpected error while building VarBinArray") } } diff --git a/vortex-array/src/array/varbin/compute/filter.rs b/vortex-array/src/array/varbin/compute/filter.rs index d0db9c85ab..4d220d6529 100644 --- a/vortex-array/src/array/varbin/compute/filter.rs +++ b/vortex-array/src/array/varbin/compute/filter.rs @@ -1,7 +1,7 @@ use itertools::Itertools; use num_traits::{AsPrimitive, Zero}; use vortex_dtype::{match_each_integer_ptype, DType, NativePType}; -use vortex_error::{vortex_err, VortexResult}; +use vortex_error::{vortex_err, vortex_panic, VortexResult}; use crate::array::varbin::builder::VarBinBuilder; use crate::array::varbin::VarBinArray; @@ -67,24 +67,32 @@ where if let Some(val) = logical_validity.to_null_buffer()? { let mut builder = VarBinBuilder::::with_capacity(selection_count); - predicate.maybe_null_slices_iter().for_each(|(start, end)| { + for (start, end) in predicate.maybe_null_slices_iter() { let null_sl = val.slice(start, end - start); if null_sl.null_count() == 0 { update_non_nullable_slice(data, offsets, &mut builder, start, end) } else { - null_sl.iter().enumerate().for_each(|(idx, valid)| { + for (idx, valid) in null_sl.iter().enumerate() { if valid { - let (s, e) = ( - offsets[idx + start].to_usize().unwrap(), - offsets[idx + start + 1].to_usize().unwrap(), - ); + let s = offsets[idx + start].to_usize().ok_or_else(|| { + vortex_err!( + "Failed to convert offset to usize: {}", + offsets[idx + start] + ) + })?; + let e = offsets[idx + start + 1].to_usize().ok_or_else(|| { + vortex_err!( + "Failed to convert offset to usize: {}", + offsets[idx + start + 1] + ) + })?; builder.push_value(&data[s..e]) } else { builder.push_null() } - }) + } } - }); + } return Ok(builder.finish(dtype)); } @@ -108,11 +116,18 @@ fn update_non_nullable_slice( O: NativePType + 'static + Zero + Copy, usize: AsPrimitive, { - let (offset_start, offset_end) = (&offsets[start], &offsets[end]); - let new_data = &data[offset_start.to_usize().unwrap()..offset_end.to_usize().unwrap()]; + let new_data = { + let offset_start = offsets[start].to_usize().unwrap_or_else(|| { + vortex_panic!("Failed to convert offset to usize: {}", offsets[start]) + }); + let offset_end = offsets[end].to_usize().unwrap_or_else(|| { + vortex_panic!("Failed to convert offset to usize: {}", offsets[end]) + }); + &data[offset_start..offset_end] + }; let new_offsets = offsets[start..end + 1] .iter() - .map(|o| *o - *offset_start) + .map(|o| *o - offsets[start]) .dropping(1); builder.push_values(new_data, new_offsets, end - start) } @@ -144,17 +159,21 @@ fn filter_select_var_bin_by_index_primitive_offset( selection_count: usize, ) -> VortexResult { let mut builder = VarBinBuilder::::with_capacity(selection_count); - predicate.maybe_null_indices_iter().for_each(|idx| { + for idx in predicate.maybe_null_indices_iter() { if validity.is_valid(idx) { let (start, end) = ( - offsets[idx].to_usize().unwrap(), - offsets[idx + 1].to_usize().unwrap(), + offsets[idx].to_usize().ok_or_else(|| { + vortex_err!("Failed to convert offset to usize: {}", offsets[idx]) + })?, + offsets[idx + 1].to_usize().ok_or_else(|| { + vortex_err!("Failed to convert offset to usize: {}", offsets[idx + 1]) + })?, ); builder.push(Some(&data[start..end])) } else { builder.push_null() } - }); + } Ok(builder.finish(dtype)) } diff --git a/vortex-array/src/array/varbin/compute/mod.rs b/vortex-array/src/array/varbin/compute/mod.rs index 584d2af759..16a74bfa15 100644 --- a/vortex-array/src/array/varbin/compute/mod.rs +++ b/vortex-array/src/array/varbin/compute/mod.rs @@ -1,4 +1,4 @@ -use vortex_error::VortexResult; +use vortex_error::{VortexResult, VortexUnwrap as _}; use vortex_scalar::Scalar; use crate::array::varbin::{varbin_scalar, VarBinArray}; @@ -30,6 +30,6 @@ impl ScalarAtFn for VarBinArray { } fn scalar_at_unchecked(&self, index: usize) -> Scalar { - varbin_scalar(self.bytes_at(index).unwrap(), self.dtype()) + varbin_scalar(self.bytes_at(index).vortex_unwrap(), self.dtype()) } } diff --git a/vortex-array/src/array/varbin/compute/take.rs b/vortex-array/src/array/varbin/compute/take.rs index df0032bab9..8f4282d877 100644 --- a/vortex-array/src/array/varbin/compute/take.rs +++ b/vortex-array/src/array/varbin/compute/take.rs @@ -1,6 +1,6 @@ use arrow_buffer::NullBuffer; use vortex_dtype::{match_each_integer_ptype, DType, NativePType}; -use vortex_error::{vortex_bail, VortexResult}; +use vortex_error::{vortex_bail, vortex_err, vortex_panic, VortexResult}; use crate::array::varbin::builder::VarBinBuilder; use crate::array::varbin::VarBinArray; @@ -49,9 +49,15 @@ fn take( let mut builder = VarBinBuilder::::with_capacity(indices.len()); for &idx in indices { - let idx = idx.to_usize().unwrap(); - let start = offsets[idx].to_usize().unwrap(); - let stop = offsets[idx + 1].to_usize().unwrap(); + let idx = idx + .to_usize() + .ok_or_else(|| vortex_err!("Failed to convert index to usize: {}", idx))?; + let start = offsets[idx] + .to_usize() + .ok_or_else(|| vortex_err!("Failed to convert offset to usize: {}", offsets[idx]))?; + let stop = offsets[idx + 1].to_usize().ok_or_else(|| { + vortex_err!("Failed to convert offset to usize: {}", offsets[idx + 1]) + })?; builder.push(Some(&data[start..stop])); } Ok(builder.finish(dtype)) @@ -66,10 +72,16 @@ fn take_nullable( ) -> VarBinArray { let mut builder = VarBinBuilder::::with_capacity(indices.len()); for &idx in indices { - let idx = idx.to_usize().unwrap(); + let idx = idx + .to_usize() + .unwrap_or_else(|| vortex_panic!("Failed to convert index to usize: {}", idx)); if null_buffer.is_valid(idx) { - let start = offsets[idx].to_usize().unwrap(); - let stop = offsets[idx + 1].to_usize().unwrap(); + let start = offsets[idx].to_usize().unwrap_or_else(|| { + vortex_panic!("Failed to convert offset to usize: {}", offsets[idx]) + }); + let stop = offsets[idx + 1].to_usize().unwrap_or_else(|| { + vortex_panic!("Failed to convert offset to usize: {}", offsets[idx + 1]) + }); builder.push(Some(&data[start..stop])); } else { builder.push(None); diff --git a/vortex-array/src/array/varbin/mod.rs b/vortex-array/src/array/varbin/mod.rs index 26bbae4613..99e3f40b24 100644 --- a/vortex-array/src/array/varbin/mod.rs +++ b/vortex-array/src/array/varbin/mod.rs @@ -5,7 +5,10 @@ use serde::{Deserialize, Serialize}; pub use stats::compute_stats; use vortex_buffer::Buffer; use vortex_dtype::{match_each_native_ptype, DType, NativePType, Nullability}; -use vortex_error::{vortex_bail, VortexError, VortexResult}; +use vortex_error::{ + vortex_bail, vortex_err, vortex_panic, VortexError, VortexExpect as _, VortexResult, + VortexUnwrap as _, +}; use vortex_scalar::Scalar; use crate::array::primitive::PrimitiveArray; @@ -75,7 +78,7 @@ impl VarBinArray { pub fn offsets(&self) -> Array { self.array() .child(0, &self.metadata().offsets_dtype, self.len() + 1) - .expect("missing offsets") + .vortex_expect("Missing offsets in VarBinArray") } pub fn first_offset TryFrom<&'a Scalar, Error = VortexError>>( @@ -91,7 +94,7 @@ impl VarBinArray { pub fn bytes(&self) -> Array { self.array() .child(1, &DType::BYTES, self.metadata().bytes_len) - .expect("missing bytes") + .vortex_expect("Missing bytes in VarBinArray") } pub fn validity(&self) -> Validity { @@ -152,10 +155,12 @@ impl VarBinArray { }) .unwrap_or_else(|| { scalar_at(&self.offsets(), index) - .unwrap() + .unwrap_or_else(|err| { + vortex_panic!(err, "Failed to get offset at index: {}", index) + }) .as_ref() .try_into() - .unwrap() + .vortex_expect("Failed to convert offset to usize") }) } @@ -219,7 +224,9 @@ impl<'a> FromIterator> for VarBinArray { pub fn varbin_scalar(value: Buffer, dtype: &DType) -> Scalar { if matches!(dtype, DType::Utf8(_)) { - Scalar::try_utf8(value, dtype.nullability()).unwrap() + Scalar::try_utf8(value, dtype.nullability()) + .map_err(|err| vortex_err!("Failed to create scalar from utf8 buffer: {}", err)) + .vortex_unwrap() } else { Scalar::binary(value, dtype.nullability()) } diff --git a/vortex-array/src/array/varbinview/compute.rs b/vortex-array/src/array/varbinview/compute.rs index 2fe3b41325..f0c0869133 100644 --- a/vortex-array/src/array/varbinview/compute.rs +++ b/vortex-array/src/array/varbinview/compute.rs @@ -1,5 +1,5 @@ use vortex_buffer::Buffer; -use vortex_error::VortexResult; +use vortex_error::{vortex_panic, VortexResult}; use vortex_scalar::Scalar; use crate::array::varbin::varbin_scalar; @@ -25,7 +25,7 @@ impl ScalarAtFn for VarBinViewArray { } fn scalar_at_unchecked(&self, index: usize) -> Scalar { - ::scalar_at(self, index).unwrap() + ::scalar_at(self, index).unwrap_or_else(|err| vortex_panic!(err)) } } diff --git a/vortex-array/src/array/varbinview/mod.rs b/vortex-array/src/array/varbinview/mod.rs index 48ed0a5e5e..1b213b1269 100644 --- a/vortex-array/src/array/varbinview/mod.rs +++ b/vortex-array/src/array/varbinview/mod.rs @@ -10,9 +10,8 @@ use arrow_buffer::ScalarBuffer; use arrow_schema::DataType; use itertools::Itertools; use vortex_dtype::{DType, PType}; -use vortex_error::{vortex_bail, VortexError, VortexResult}; +use vortex_error::{vortex_bail, vortex_panic, VortexError, VortexExpect as _, VortexResult}; -use crate::array::primitive::PrimitiveArray; use crate::array::varbin::VarBinArray; use crate::arrow::FromArrowArray; use crate::compute::slice; @@ -159,8 +158,9 @@ impl VarBinViewArray { fn view_slice(&self) -> &[BinaryView] { unsafe { slice::from_raw_parts( - PrimitiveArray::try_from(self.views()) - .expect("Views must be a primitive array") + self.views() + .into_primitive() + .vortex_expect("Views must be a primitive array") .maybe_null_slice::() .as_ptr() as _, self.views().len() / VIEW_SIZE, @@ -176,14 +176,14 @@ impl VarBinViewArray { pub fn views(&self) -> Array { self.array() .child(0, &DType::BYTES, self.len() * VIEW_SIZE) - .expect("missing views") + .unwrap_or_else(|| vortex_panic!("VarBinViewArray is missing its views")) } #[inline] pub fn bytes(&self, idx: usize) -> Array { self.array() .child(idx + 1, &DType::BYTES, self.metadata().data_lens[idx]) - .expect("Missing data buffer") + .unwrap_or_else(|| vortex_panic!("VarBinViewArray is missing its data buffer")) } pub fn validity(&self) -> Validity { @@ -201,7 +201,8 @@ impl VarBinViewArray { builder.append_value(s); } let array = Array::from_arrow(&builder.finish(), false); - VarBinViewArray::try_from(array).expect("should be var bin view array") + VarBinViewArray::try_from(array) + .vortex_expect("Failed to convert iterator of nullable strings to VarBinViewArray") } pub fn from_iter_nullable_str, I: IntoIterator>>( @@ -212,7 +213,8 @@ impl VarBinViewArray { builder.extend(iter); let array = Array::from_arrow(&builder.finish(), true); - VarBinViewArray::try_from(array).expect("should be var bin view array") + VarBinViewArray::try_from(array) + .vortex_expect("Failed to convert iterator of nullable strings to VarBinViewArray") } pub fn from_iter_bin, I: IntoIterator>(iter: I) -> Self { @@ -222,7 +224,8 @@ impl VarBinViewArray { builder.append_value(b); } let array = Array::from_arrow(&builder.finish(), true); - VarBinViewArray::try_from(array).expect("should be var bin view array") + VarBinViewArray::try_from(array) + .vortex_expect("Failed to convert iterator of bytes to VarBinViewArray") } pub fn from_iter_nullable_bin, I: IntoIterator>>( @@ -232,7 +235,8 @@ impl VarBinViewArray { let mut builder = BinaryViewBuilder::with_capacity(iter.size_hint().0); builder.extend(iter); let array = Array::from_arrow(&builder.finish(), true); - VarBinViewArray::try_from(array).expect("should be var bin view array") + VarBinViewArray::try_from(array) + .vortex_expect("Failed to convert iterator of nullable bytes to VarBinViewArray") } pub fn bytes_at(&self, index: usize) -> VortexResult> { @@ -277,17 +281,17 @@ fn as_arrow(var_bin_view: VarBinViewArray) -> ArrayRef { let views = var_bin_view .views() .into_primitive() - .expect("views must be primitive"); + .vortex_expect("Views must be a primitive array"); assert_eq!(views.ptype(), PType::U8); let nulls = var_bin_view .logical_validity() .to_null_buffer() - .expect("null buffer"); + .vortex_expect("Failed to convert logical validity to null buffer"); let data = (0..var_bin_view.metadata().data_lens.len()) .map(|i| var_bin_view.bytes(i).into_primitive()) .collect::>>() - .expect("bytes arrays must be primitive"); + .vortex_expect("VarBinView byte arrays must be primitive arrays"); if !data.is_empty() { assert_eq!(data[0].ptype(), PType::U8); assert!(data.iter().map(|d| d.ptype()).all_equal()); @@ -310,7 +314,7 @@ fn as_arrow(var_bin_view: VarBinViewArray) -> ArrayRef { data, nulls, )), - _ => panic!("expected utf8 or binary, got {}", var_bin_view.dtype()), + _ => vortex_panic!("Expected utf8 or binary, got {}", var_bin_view.dtype()), } } diff --git a/vortex-array/src/arrow/array.rs b/vortex-array/src/arrow/array.rs index 46b50da97c..fe0e0f11e7 100644 --- a/vortex-array/src/arrow/array.rs +++ b/vortex-array/src/arrow/array.rs @@ -19,6 +19,7 @@ use arrow_schema::{DataType, TimeUnit as ArrowTimeUnit}; use itertools::Itertools; use vortex_datetime_dtype::TimeUnit; use vortex_dtype::{DType, NativePType, PType}; +use vortex_error::{vortex_panic, VortexExpect as _}; use crate::array::{ BoolArray, NullArray, PrimitiveArray, StructArray, TemporalArray, VarBinArray, VarBinViewArray, @@ -37,7 +38,7 @@ impl From for ArrayData { impl From for ArrayData { fn from(value: NullBuffer) -> Self { BoolArray::try_new(value.into_inner(), Validity::NonNullable) - .unwrap() + .vortex_expect("Failed to convert null buffer to BoolArray") .to_array_data() } } @@ -99,7 +100,7 @@ where DataType::Date64 => TemporalArray::new_date(arr.into(), TimeUnit::Ms).into(), DataType::Duration(_) => unimplemented!(), DataType::Interval(_) => unimplemented!(), - _ => panic!("Invalid data type for PrimitiveArray"), + _ => vortex_panic!("Invalid data type for PrimitiveArray: {}", T::DATA_TYPE), } } } @@ -112,7 +113,7 @@ where let dtype = match T::DATA_TYPE { DataType::Binary | DataType::LargeBinary => DType::Binary(nullable.into()), DataType::Utf8 | DataType::LargeUtf8 => DType::Utf8(nullable.into()), - _ => panic!("Invalid data type for ByteArray"), + _ => vortex_panic!("Invalid data type for ByteArray: {}", T::DATA_TYPE), }; VarBinArray::try_new( ArrayData::from(value.offsets().clone()).into(), @@ -120,7 +121,7 @@ where dtype, nulls(value.nulls(), nullable), ) - .unwrap() + .vortex_expect("Failed to convert Arrow GenericByteArray to Vortex VarBinArray") .into() } } @@ -130,7 +131,7 @@ impl FromArrowArray<&GenericByteViewArray> for Array { let dtype = match T::DATA_TYPE { DataType::BinaryView => DType::Binary(nullable.into()), DataType::Utf8View => DType::Utf8(nullable.into()), - _ => panic!("Invalid data type for ByteViewArray"), + _ => vortex_panic!("Invalid data type for ByteViewArray: {}", T::DATA_TYPE), }; VarBinViewArray::try_new( ArrayData::from(value.views().inner().clone()).into(), @@ -142,7 +143,7 @@ impl FromArrowArray<&GenericByteViewArray> for Array { dtype, nulls(value.nulls(), nullable), ) - .unwrap() + .vortex_expect("Failed to convert Arrow GenericByteViewArray to Vortex VarBinViewArray") .into() } } @@ -150,7 +151,7 @@ impl FromArrowArray<&GenericByteViewArray> for Array { impl FromArrowArray<&ArrowBooleanArray> for Array { fn from_arrow(value: &ArrowBooleanArray, nullable: bool) -> Self { BoolArray::try_new(value.values().clone(), nulls(value.nulls(), nullable)) - .unwrap() + .vortex_expect("Failed to convert Arrow BooleanArray to Vortex BoolArray") .into() } } @@ -173,7 +174,7 @@ impl FromArrowArray<&ArrowStructArray> for Array { value.len(), nulls(value.nulls(), nullable), ) - .unwrap() + .vortex_expect("Failed to convert Arrow StructArray to Vortex StructArray") .into() } } @@ -222,11 +223,17 @@ impl FromArrowArray for Array { DataType::Binary => Self::from_arrow(array.as_binary::(), nullable), DataType::LargeBinary => Self::from_arrow(array.as_binary::(), nullable), DataType::BinaryView => Self::from_arrow( - array.as_any().downcast_ref::().unwrap(), + array + .as_any() + .downcast_ref::() + .vortex_expect("Expected Arrow BinaryViewArray for DataType::BinaryView"), nullable, ), DataType::Utf8View => Self::from_arrow( - array.as_any().downcast_ref::().unwrap(), + array + .as_any() + .downcast_ref::() + .vortex_expect("Expected Arrow StringViewArray for DataType::Utf8View"), nullable, ), DataType::Struct(_) => Self::from_arrow(array.as_struct(), nullable), @@ -279,8 +286,8 @@ impl FromArrowArray for Array { Self::from_arrow(array.as_primitive::(), nullable) } }, - _ => panic!( - "TODO(robert): Missing array encoding for dtype {}", + _ => vortex_panic!( + "Array encoding not implementedfor Arrow data type {}", array.data_type().clone() ), } diff --git a/vortex-array/src/arrow/recordbatch.rs b/vortex-array/src/arrow/recordbatch.rs index 68ce12b5f5..286e68e3eb 100644 --- a/vortex-array/src/arrow/recordbatch.rs +++ b/vortex-array/src/arrow/recordbatch.rs @@ -1,15 +1,18 @@ use arrow_array::cast::as_struct_array; use arrow_array::RecordBatch; use itertools::Itertools; +use vortex_error::{vortex_err, VortexError, VortexResult}; use crate::array::StructArray; use crate::arrow::FromArrowArray; use crate::validity::Validity; use crate::{Array, IntoArrayVariant, IntoCanonical}; -impl From for Array { - fn from(value: RecordBatch) -> Self { - StructArray::try_new( +impl TryFrom for Array { + type Error = VortexError; + + fn try_from(value: RecordBatch) -> VortexResult { + Ok(StructArray::try_new( value .schema() .fields() @@ -25,28 +28,29 @@ impl From for Array { .collect(), value.num_rows(), Validity::NonNullable, // Must match FromArrowType for DType - ) - .unwrap() - .into() + )? + .into()) } } -impl From for RecordBatch { - fn from(value: Array) -> Self { - let struct_arr = value - .into_struct() - .expect("RecordBatch can only be constructed from a Vortex StructArray"); - Self::from(struct_arr) +impl TryFrom for RecordBatch { + type Error = VortexError; + + fn try_from(value: Array) -> VortexResult { + let struct_arr = value.into_struct().map_err(|err| { + vortex_err!("RecordBatch can only be constructed from a Vortex StructArray: {err}") + })?; + + RecordBatch::try_from(struct_arr) } } -impl From for RecordBatch { - fn from(value: StructArray) -> Self { - let array_ref = value - .into_canonical() - .expect("Struct arrays must canonicalize") - .into_arrow(); +impl TryFrom for RecordBatch { + type Error = VortexError; + + fn try_from(value: StructArray) -> VortexResult { + let array_ref = value.into_canonical()?.into_arrow()?; let struct_array = as_struct_array(array_ref.as_ref()); - Self::from(struct_array) + Ok(Self::from(struct_array)) } } diff --git a/vortex-array/src/canonical.rs b/vortex-array/src/canonical.rs index 21ba798655..bc213924ee 100644 --- a/vortex-array/src/canonical.rs +++ b/vortex-array/src/canonical.rs @@ -70,24 +70,20 @@ impl Canonical { /// Scalar arrays such as Bool and Primitive canonical arrays should convert with /// zero copies, while more complex variants such as Struct may require allocations if its child /// arrays require decompression. - pub fn into_arrow(self) -> ArrayRef { - match self { - Canonical::Null(a) => null_to_arrow(a), - Canonical::Bool(a) => bool_to_arrow(a), - Canonical::Primitive(a) => primitive_to_arrow(a), - Canonical::Struct(a) => struct_to_arrow(a), - Canonical::VarBin(a) => varbin_to_arrow(a), + pub fn into_arrow(self) -> VortexResult { + Ok(match self { + Canonical::Null(a) => null_to_arrow(a)?, + Canonical::Bool(a) => bool_to_arrow(a)?, + Canonical::Primitive(a) => primitive_to_arrow(a)?, + Canonical::Struct(a) => struct_to_arrow(a)?, + Canonical::VarBin(a) => varbin_to_arrow(a)?, Canonical::Extension(a) => { if !is_temporal_ext_type(a.id()) { - panic!("unsupported extension dtype with ID {}", a.id().as_ref()) + vortex_bail!("unsupported extension dtype with ID {}", a.id().as_ref()) } - - temporal_to_arrow( - TemporalArray::try_from(&a.into_array()) - .expect("array must be known temporal array ext type"), - ) + temporal_to_arrow(TemporalArray::try_from(&a.into_array())?)? } - } + }) } } @@ -96,100 +92,101 @@ impl Canonical { pub fn into_null(self) -> VortexResult { match self { Canonical::Null(a) => Ok(a), - _ => vortex_bail!(InvalidArgument: "cannot unwrap NullArray from {:?}", &self), + _ => vortex_bail!("Cannot unwrap NullArray from {:?}", &self), } } pub fn into_bool(self) -> VortexResult { match self { Canonical::Bool(a) => Ok(a), - _ => vortex_bail!(InvalidArgument: "cannot unwrap BoolArray from {:?}", &self), + _ => vortex_bail!("Cannot unwrap BoolArray from {:?}", &self), } } pub fn into_primitive(self) -> VortexResult { match self { Canonical::Primitive(a) => Ok(a), - _ => vortex_bail!(InvalidArgument: "cannot unwrap PrimitiveArray from {:?}", &self), + _ => vortex_bail!("Cannot unwrap PrimitiveArray from {:?}", &self), } } pub fn into_struct(self) -> VortexResult { match self { Canonical::Struct(a) => Ok(a), - _ => vortex_bail!(InvalidArgument: "cannot unwrap StructArray from {:?}", &self), + _ => vortex_bail!("Cannot unwrap StructArray from {:?}", &self), } } pub fn into_varbin(self) -> VortexResult { match self { Canonical::VarBin(a) => Ok(a), - _ => vortex_bail!(InvalidArgument: "cannot unwrap VarBinArray from {:?}", &self), + _ => vortex_bail!("Cannot unwrap VarBinArray from {:?}", &self), } } pub fn into_extension(self) -> VortexResult { match self { Canonical::Extension(a) => Ok(a), - _ => vortex_bail!(InvalidArgument: "cannot unwrap ExtensionArray from {:?}", &self), + _ => vortex_bail!("Cannot unwrap ExtensionArray from {:?}", &self), } } } -fn null_to_arrow(null_array: NullArray) -> ArrayRef { - Arc::new(ArrowNullArray::new(null_array.len())) +fn null_to_arrow(null_array: NullArray) -> VortexResult { + Ok(Arc::new(ArrowNullArray::new(null_array.len()))) } -fn bool_to_arrow(bool_array: BoolArray) -> ArrayRef { - Arc::new(ArrowBoolArray::new( +fn bool_to_arrow(bool_array: BoolArray) -> VortexResult { + Ok(Arc::new(ArrowBoolArray::new( bool_array.boolean_buffer(), - bool_array - .logical_validity() - .to_null_buffer() - .expect("null buffer"), - )) + bool_array.logical_validity().to_null_buffer()?, + ))) } -fn primitive_to_arrow(primitive_array: PrimitiveArray) -> ArrayRef { +fn primitive_to_arrow(primitive_array: PrimitiveArray) -> VortexResult { fn as_arrow_array_primitive( array: &PrimitiveArray, - ) -> ArrowPrimitiveArray { - ArrowPrimitiveArray::new( + ) -> VortexResult>> { + Ok(Arc::new(ArrowPrimitiveArray::new( ScalarBuffer::::new(array.buffer().clone().into_arrow(), 0, array.len()), - array - .logical_validity() - .to_null_buffer() - .expect("null buffer"), - ) + array.logical_validity().to_null_buffer()?, + ))) } - match primitive_array.ptype() { - PType::U8 => Arc::new(as_arrow_array_primitive::(&primitive_array)), - PType::U16 => Arc::new(as_arrow_array_primitive::(&primitive_array)), - PType::U32 => Arc::new(as_arrow_array_primitive::(&primitive_array)), - PType::U64 => Arc::new(as_arrow_array_primitive::(&primitive_array)), - PType::I8 => Arc::new(as_arrow_array_primitive::(&primitive_array)), - PType::I16 => Arc::new(as_arrow_array_primitive::(&primitive_array)), - PType::I32 => Arc::new(as_arrow_array_primitive::(&primitive_array)), - PType::I64 => Arc::new(as_arrow_array_primitive::(&primitive_array)), - PType::F16 => Arc::new(as_arrow_array_primitive::(&primitive_array)), - PType::F32 => Arc::new(as_arrow_array_primitive::(&primitive_array)), - PType::F64 => Arc::new(as_arrow_array_primitive::(&primitive_array)), - } + Ok(match primitive_array.ptype() { + PType::U8 => as_arrow_array_primitive::(&primitive_array)?, + PType::U16 => as_arrow_array_primitive::(&primitive_array)?, + PType::U32 => as_arrow_array_primitive::(&primitive_array)?, + PType::U64 => as_arrow_array_primitive::(&primitive_array)?, + PType::I8 => as_arrow_array_primitive::(&primitive_array)?, + PType::I16 => as_arrow_array_primitive::(&primitive_array)?, + PType::I32 => as_arrow_array_primitive::(&primitive_array)?, + PType::I64 => as_arrow_array_primitive::(&primitive_array)?, + PType::F16 => as_arrow_array_primitive::(&primitive_array)?, + PType::F32 => as_arrow_array_primitive::(&primitive_array)?, + PType::F64 => as_arrow_array_primitive::(&primitive_array)?, + }) } -fn struct_to_arrow(struct_array: StructArray) -> ArrayRef { - let field_arrays: Vec = struct_array - .children() - .map(|f| { - let canonical = f.into_canonical().unwrap(); - match canonical { - // visit nested structs recursively - Canonical::Struct(a) => struct_to_arrow(a), - _ => canonical.into_arrow(), - } - }) - .collect(); +fn struct_to_arrow(struct_array: StructArray) -> VortexResult { + let field_arrays: Vec = + Iterator::zip(struct_array.names().iter(), struct_array.children()) + .map(|(name, f)| { + let canonical = f.into_canonical().map_err(|err| { + err.with_context(format!("Failed to canonicalize field {}", name)) + })?; + match canonical { + // visit nested structs recursively + Canonical::Struct(a) => struct_to_arrow(a), + _ => canonical.into_arrow().map_err(|err| { + err.with_context(format!( + "Failed to convert canonicalized field {} to arrow", + name + )) + }), + } + }) + .collect::>>()?; let arrow_fields: Fields = struct_array .names() @@ -206,43 +203,45 @@ fn struct_to_arrow(struct_array: StructArray) -> ArrayRef { .map(Arc::new) .collect(); - let nulls = struct_array - .logical_validity() - .to_null_buffer() - .expect("null buffer"); + let nulls = struct_array.logical_validity().to_null_buffer()?; - Arc::new(ArrowStructArray::new(arrow_fields, field_arrays, nulls)) + Ok(Arc::new(ArrowStructArray::try_new( + arrow_fields, + field_arrays, + nulls, + )?)) } -fn varbin_to_arrow(varbin_array: VarBinArray) -> ArrayRef { +fn varbin_to_arrow(varbin_array: VarBinArray) -> VortexResult { let offsets = varbin_array .offsets() .into_primitive() - .expect("flatten_primitive"); + .map_err(|err| err.with_context("Failed to canonicalize offsets"))?; let offsets = match offsets.ptype() { PType::I32 | PType::I64 => offsets, PType::U64 => offsets.reinterpret_cast(PType::I64), PType::U32 => offsets.reinterpret_cast(PType::I32), // Unless it's u64, everything else can be converted into an i32. _ => try_cast(&offsets.to_array(), PType::I32.into()) - .expect("cast to i32") - .into_primitive() - .expect("flatten_primitive"), + .and_then(|a| a.into_primitive()) + .map_err(|err| err.with_context("Failed to cast offsets to PrimitiveArray of i32"))?, }; let nulls = varbin_array .logical_validity() .to_null_buffer() - .expect("null buffer"); + .map_err(|err| err.with_context("Failed to get null buffer from logical validity"))?; let data = varbin_array .bytes() .into_primitive() - .expect("flatten_primitive"); - assert_eq!(data.ptype(), PType::U8); + .map_err(|err| err.with_context("Failed to canonicalize bytes"))?; + if data.ptype() != PType::U8 { + vortex_bail!("Expected bytes to be of type U8, got {}", data.ptype()); + } let data = data.buffer(); // Switch on Arrow DType. - match varbin_array.dtype() { + Ok(match varbin_array.dtype() { DType::Binary(_) => match offsets.ptype() { PType::I32 => Arc::new(unsafe { BinaryArray::new_unchecked( @@ -258,7 +257,7 @@ fn varbin_to_arrow(varbin_array: VarBinArray) -> ArrayRef { nulls, ) }), - _ => panic!("Invalid offsets type"), + _ => vortex_bail!("Invalid offsets type {}", offsets.ptype()), }, DType::Utf8(_) => match offsets.ptype() { PType::I32 => Arc::new(unsafe { @@ -275,27 +274,22 @@ fn varbin_to_arrow(varbin_array: VarBinArray) -> ArrayRef { nulls, ) }), - _ => panic!("Invalid offsets type"), + _ => vortex_bail!("Invalid offsets type {}", offsets.ptype()), }, - _ => panic!( + _ => vortex_bail!( "expected utf8 or binary instead of {}", varbin_array.dtype() ), - } + }) } -fn temporal_to_arrow(temporal_array: TemporalArray) -> ArrayRef { +fn temporal_to_arrow(temporal_array: TemporalArray) -> VortexResult { macro_rules! extract_temporal_values { ($values:expr, $prim:ty) => {{ - let temporal_values = try_cast($values, <$prim as NativePType>::PTYPE.into()) - .expect("values must cast to primitive type") - .into_primitive() - .expect("must be primitive array"); + let temporal_values = + try_cast($values, <$prim as NativePType>::PTYPE.into())?.into_primitive()?; let len = temporal_values.len(); - let nulls = temporal_values - .logical_validity() - .to_null_buffer() - .expect("null buffer"); + let nulls = temporal_values.logical_validity().to_null_buffer()?; let scalars = ScalarBuffer::<$prim>::new(temporal_values.into_buffer().into_arrow(), 0, len); @@ -303,7 +297,7 @@ fn temporal_to_arrow(temporal_array: TemporalArray) -> ArrayRef { }}; } - match temporal_array.temporal_metadata() { + Ok(match temporal_array.temporal_metadata() { TemporalMetadata::Date(time_unit) => match time_unit { TimeUnit::D => { let (scalars, nulls) = @@ -315,7 +309,7 @@ fn temporal_to_arrow(temporal_array: TemporalArray) -> ArrayRef { extract_temporal_values!(&temporal_array.temporal_values(), i64); Arc::new(Date64Array::new(scalars, nulls)) } - _ => panic!( + _ => vortex_bail!( "Invalid TimeUnit {time_unit} for {}", temporal_array.ext_dtype().id() ), @@ -341,7 +335,7 @@ fn temporal_to_arrow(temporal_array: TemporalArray) -> ArrayRef { extract_temporal_values!(&temporal_array.temporal_values(), i64); Arc::new(Time64NanosecondArray::new(scalars, nulls)) } - _ => panic!( + _ => vortex_bail!( "Invalid TimeUnit {time_unit} for {}", temporal_array.ext_dtype().id() ), @@ -353,13 +347,13 @@ fn temporal_to_arrow(temporal_array: TemporalArray) -> ArrayRef { TimeUnit::Us => Arc::new(TimestampMicrosecondArray::new(scalars, nulls)), TimeUnit::Ms => Arc::new(TimestampMillisecondArray::new(scalars, nulls)), TimeUnit::S => Arc::new(TimestampSecondArray::new(scalars, nulls)), - _ => panic!( + _ => vortex_bail!( "Invalid TimeUnit {time_unit} for {}", temporal_array.ext_dtype().id() ), } } - } + }) } /// Support trait for transmuting an array into its [vortex_dtype::DType]'s canonical encoding. @@ -502,6 +496,7 @@ mod test { .into_canonical() .unwrap() .into_arrow() + .unwrap() .as_any() .downcast_ref::() .cloned() @@ -575,6 +570,7 @@ mod test { .into_canonical() .unwrap() .into_arrow() + .unwrap() .as_struct() ); } diff --git a/vortex-array/src/compute/compare.rs b/vortex-array/src/compute/compare.rs index a2b42a039c..0c0b802427 100644 --- a/vortex-array/src/compute/compare.rs +++ b/vortex-array/src/compute/compare.rs @@ -96,8 +96,8 @@ pub fn compare(left: &Array, right: &Array, operator: Operator) -> VortexResult< } // Fallback to arrow on canonical types - let lhs = left.clone().into_canonical()?.into_arrow(); - let rhs = right.clone().into_canonical()?.into_arrow(); + let lhs = left.clone().into_canonical()?.into_arrow()?; + let rhs = right.clone().into_canonical()?.into_arrow()?; let array = match operator { Operator::Eq => cmp::eq(&lhs.as_ref(), &rhs.as_ref())?, diff --git a/vortex-array/src/compute/filter.rs b/vortex-array/src/compute/filter.rs index 3ec0178a73..71f5f6b936 100644 --- a/vortex-array/src/compute/filter.rs +++ b/vortex-array/src/compute/filter.rs @@ -40,8 +40,8 @@ pub fn filter(array: &Array, predicate: &Array) -> VortexResult { filter_fn.filter(predicate) } else { // Fallback: implement using Arrow kernels. - let array_ref = array.clone().into_canonical()?.into_arrow(); - let predicate_ref = predicate.clone().into_canonical()?.into_arrow(); + let array_ref = array.clone().into_canonical()?.into_arrow()?; + let predicate_ref = predicate.clone().into_canonical()?.into_arrow()?; let filtered = arrow_select::filter::filter(array_ref.as_ref(), predicate_ref.as_boolean())?; diff --git a/vortex-array/src/compute/unary/scalar_at.rs b/vortex-array/src/compute/unary/scalar_at.rs index 530a8c0a56..29bdaa6094 100644 --- a/vortex-array/src/compute/unary/scalar_at.rs +++ b/vortex-array/src/compute/unary/scalar_at.rs @@ -1,4 +1,4 @@ -use vortex_error::{vortex_bail, vortex_err, VortexResult}; +use vortex_error::{vortex_bail, vortex_err, vortex_panic, VortexResult}; use vortex_scalar::Scalar; use crate::{Array, ArrayDType}; @@ -29,6 +29,5 @@ pub fn scalar_at(array: &Array, index: usize) -> VortexResult { pub fn scalar_at_unchecked(array: &Array, index: usize) -> Scalar { array .with_dyn(|a| a.scalar_at().map(|s| s.scalar_at_unchecked(index))) - .ok_or_else(|| vortex_err!(NotImplemented: "scalar_at", array.encoding().id())) - .unwrap() + .unwrap_or_else(|| vortex_panic!(NotImplemented: "scalar_at", array.encoding().id())) } diff --git a/vortex-array/src/data.rs b/vortex-array/src/data.rs index 14c88a9785..e97b6192ff 100644 --- a/vortex-array/src/data.rs +++ b/vortex-array/src/data.rs @@ -2,7 +2,7 @@ use std::sync::{Arc, RwLock}; use vortex_buffer::Buffer; use vortex_dtype::DType; -use vortex_error::VortexResult; +use vortex_error::{vortex_panic, VortexResult}; use vortex_scalar::Scalar; use crate::encoding::EncodingRef; @@ -136,7 +136,7 @@ impl Statistics for ArrayData { self.stats_map .read() .unwrap_or_else(|_| { - panic!( + vortex_panic!( "Failed to acquire read lock on stats map while getting {}", stat ) @@ -148,7 +148,7 @@ impl Statistics for ArrayData { fn to_set(&self) -> StatsSet { self.stats_map .read() - .unwrap_or_else(|_| panic!("Failed to acquire read lock on stats map")) + .unwrap_or_else(|_| vortex_panic!("Failed to acquire read lock on stats map")) .clone() } @@ -156,9 +156,10 @@ impl Statistics for ArrayData { self.stats_map .write() .unwrap_or_else(|_| { - panic!( + vortex_panic!( "Failed to acquire write lock on stats map while setting {} to {}", - stat, value + stat, + value ) }) .set(stat, value); @@ -171,7 +172,9 @@ impl Statistics for ArrayData { self.stats_map .write() - .unwrap_or_else(|_| panic!("Failed to write to stats map while computing {}", stat)) + .unwrap_or_else(|_| { + vortex_panic!("Failed to write to stats map while computing {}", stat) + }) .extend( self.to_array() .with_dyn(|a| a.compute_statistics(stat)) diff --git a/vortex-array/src/elementwise.rs b/vortex-array/src/elementwise.rs index c6f8eabb9e..2e6c34418d 100644 --- a/vortex-array/src/elementwise.rs +++ b/vortex-array/src/elementwise.rs @@ -19,6 +19,7 @@ pub trait UnaryFn { ) -> VortexResult; } +#[allow(clippy::unwrap_used)] pub fn dyn_cast_array_iter(array: &Array) -> Box>> { match PType::try_from(array.dtype()).unwrap() { PType::U8 => Box::new( diff --git a/vortex-array/src/encoding.rs b/vortex-array/src/encoding.rs index e537a58659..1c57eb9059 100644 --- a/vortex-array/src/encoding.rs +++ b/vortex-array/src/encoding.rs @@ -1,7 +1,7 @@ use std::fmt::{Debug, Display, Formatter}; use std::hash::{Hash, Hasher}; -use vortex_error::VortexResult; +use vortex_error::{vortex_panic, VortexResult}; use crate::canonical::{Canonical, IntoCanonical}; use crate::{Array, ArrayDef, ArrayTrait}; @@ -80,8 +80,14 @@ pub trait ArrayEncodingExt { where F: for<'b> FnMut(&'b (dyn ArrayTrait + 'b)) -> R, { - let typed = - <::Array as TryFrom>::try_from(array.clone()).unwrap(); + let typed = <::Array as TryFrom>::try_from(array.clone()) + .unwrap_or_else(|err| { + vortex_panic!( + err, + "Failed to convert array to {}", + std::any::type_name::<::Array>() + ) + }); f(&typed) } } diff --git a/vortex-array/src/implementation.rs b/vortex-array/src/implementation.rs index 9e39c81d3f..12be23b7ff 100644 --- a/vortex-array/src/implementation.rs +++ b/vortex-array/src/implementation.rs @@ -1,6 +1,6 @@ use vortex_buffer::Buffer; use vortex_dtype::DType; -use vortex_error::{vortex_bail, VortexError, VortexResult}; +use vortex_error::{vortex_bail, VortexError, VortexExpect as _, VortexResult}; use crate::encoding::{ArrayEncoding, ArrayEncodingExt, ArrayEncodingRef, EncodingId, EncodingRef}; use crate::stats::{ArrayStatistics, Statistics}; @@ -242,7 +242,10 @@ where buffer: None, children: vec![], }; - array.with_dyn(|a| a.accept(&mut visitor).unwrap()); + array.with_dyn(|a| { + a.accept(&mut visitor) + .vortex_expect("Error while visiting Array View children") + }); ArrayData::try_new( encoding, array.dtype().clone(), @@ -252,7 +255,7 @@ where visitor.children.into(), stats, ) - .unwrap() + .vortex_expect("Failed to create ArrayData from Array View") } } } diff --git a/vortex-array/src/iter/mod.rs b/vortex-array/src/iter/mod.rs index c7a46826a9..e4f7b7093a 100644 --- a/vortex-array/src/iter/mod.rs +++ b/vortex-array/src/iter/mod.rs @@ -3,7 +3,7 @@ use std::sync::Arc; pub use adapter::*; pub use ext::*; use vortex_dtype::{DType, NativePType}; -use vortex_error::VortexResult; +use vortex_error::{VortexExpect as _, VortexResult}; use crate::validity::Validity; use crate::Array; @@ -200,9 +200,7 @@ impl Iterator for VectorizedArrayIter { let validity = self .validity .slice(self.current_idx, self.current_idx + data.len()) - .unwrap_or_else(|_| { - panic!("The slice bounds should always be within the array's limits") - }); + .vortex_expect("The slice bounds should always be within the array's limits"); self.current_idx += data.len(); let batch = Batch::new_from_vec(data, validity); diff --git a/vortex-array/src/lib.rs b/vortex-array/src/lib.rs index 049747aba0..aef62fd141 100644 --- a/vortex-array/src/lib.rs +++ b/vortex-array/src/lib.rs @@ -22,7 +22,7 @@ pub use typed::*; pub use view::*; use vortex_buffer::Buffer; use vortex_dtype::DType; -use vortex_error::VortexResult; +use vortex_error::{vortex_panic, VortexExpect, VortexResult}; use crate::compute::ArrayCompute; use crate::encoding::{ArrayEncodingRef, EncodingRef}; @@ -192,10 +192,16 @@ impl Array { result = Some(f(array)); Ok(()) }) - .unwrap(); + .unwrap_or_else(|err| { + vortex_panic!( + err, + "Failed to convert Array to {}", + std::any::type_name::() + ) + }); // Now we unwrap the optional, which we know to be populated by the closure. - result.unwrap() + result.vortex_expect("Failed to get result from Array::with_dyn") } } @@ -254,7 +260,8 @@ pub trait ArrayTrait: { fn nbytes(&self) -> usize { let mut visitor = NBytesVisitor(0); - self.accept(&mut visitor).unwrap(); + self.accept(&mut visitor) + .vortex_expect("Failed to get nbytes from Array"); visitor.0 } } diff --git a/vortex-array/src/stats/mod.rs b/vortex-array/src/stats/mod.rs index 395cbe52b6..f338367582 100644 --- a/vortex-array/src/stats/mod.rs +++ b/vortex-array/src/stats/mod.rs @@ -6,7 +6,7 @@ use itertools::Itertools; pub use statsset::*; use vortex_dtype::Nullability::NonNullable; use vortex_dtype::{DType, NativePType}; -use vortex_error::{VortexError, VortexResult}; +use vortex_error::{vortex_panic, VortexError, VortexResult}; use vortex_scalar::Scalar; use crate::Array; @@ -96,11 +96,11 @@ impl dyn Statistics + '_ { .map(|s| U::try_from(&s)) .transpose() .unwrap_or_else(|err| { - panic!( - "Failed to cast stat {} to {}: {}", + vortex_panic!( + err, + "Failed to cast stat {} to {}", stat, - std::any::type_name::(), - err + std::any::type_name::() ) }) } @@ -113,7 +113,9 @@ impl dyn Statistics + '_ { .map(|s| s.cast(&DType::Primitive(U::PTYPE, NonNullable))) .transpose() .and_then(|maybe| maybe.as_ref().map(U::try_from).transpose()) - .unwrap_or_else(|err| panic!("Failed to cast stat {} to {}: {}", stat, U::PTYPE, err)) + .unwrap_or_else(|err| { + vortex_panic!(err, "Failed to cast stat {} to {}", stat, U::PTYPE) + }) } pub fn compute_as TryFrom<&'a Scalar, Error = VortexError>>( @@ -124,11 +126,11 @@ impl dyn Statistics + '_ { .map(|s| U::try_from(&s)) .transpose() .unwrap_or_else(|err| { - panic!( - "Failed to compute stat {} as {}: {}", + vortex_panic!( + err, + "Failed to compute stat {} as {}", stat, - std::any::type_name::(), - err + std::any::type_name::() ) }) } @@ -142,12 +144,7 @@ impl dyn Statistics + '_ { .transpose() .and_then(|maybe| maybe.as_ref().map(U::try_from).transpose()) .unwrap_or_else(|err| { - panic!( - "Failed to compute stat {} as cast {}: {}", - stat, - U::PTYPE, - err - ) + vortex_panic!(err, "Failed to compute stat {} as cast {}", stat, U::PTYPE) }) } diff --git a/vortex-array/src/stats/statsset.rs b/vortex-array/src/stats/statsset.rs index 4152f7cabf..0124516f8d 100644 --- a/vortex-array/src/stats/statsset.rs +++ b/vortex-array/src/stats/statsset.rs @@ -4,7 +4,7 @@ use std::collections::HashMap; use enum_iterator::all; use itertools::Itertools; use vortex_dtype::DType; -use vortex_error::VortexError; +use vortex_error::{vortex_panic, VortexError, VortexExpect}; use vortex_scalar::Scalar; use crate::stats::Stat; @@ -73,11 +73,11 @@ impl StatsSet { fn get_as TryFrom<&'a Scalar, Error = VortexError>>(&self, stat: Stat) -> Option { self.get(stat).map(|v| { T::try_from(v).unwrap_or_else(|err| { - panic!( - "Failed to get stat {} as {}: {}", + vortex_panic!( + err, + "Failed to get stat {} as {}", stat, - std::any::type_name::(), - err + std::any::type_name::() ) }) }) @@ -184,7 +184,9 @@ impl StatsSet { fn merge_scalar_stat(&mut self, other: &Self, stat: Stat) { if let Entry::Occupied(mut e) = self.values.entry(stat) { if let Some(other_value) = other.get_as::(stat) { - let self_value: usize = e.get().try_into().unwrap(); + let self_value: usize = e.get().try_into().unwrap_or_else(|err: VortexError| { + vortex_panic!(err, "Failed to get stat {} as usize", stat) + }); e.insert((self_value + other_value).into()); } else { e.remove(); @@ -204,7 +206,9 @@ impl StatsSet { if let Entry::Occupied(mut e) = self.values.entry(stat) { if let Some(other_value) = other.get_as::>(stat) { // TODO(robert): Avoid the copy here. We could e.get_mut() but need to figure out casting - let self_value: Vec = e.get().try_into().unwrap(); + let self_value: Vec = e.get().try_into().unwrap_or_else(|err: VortexError| { + vortex_panic!(err, "Failed to get stat {} as Vec", stat) + }); e.insert( self_value .iter() @@ -223,7 +227,10 @@ impl StatsSet { fn merge_run_count(&mut self, other: &Self) { if let Entry::Occupied(mut e) = self.values.entry(Stat::RunCount) { if let Some(other_value) = other.get_as::(Stat::RunCount) { - let self_value: usize = e.get().try_into().unwrap(); + let self_value: usize = e + .get() + .try_into() + .vortex_expect("Failed to get run count as usize"); e.insert((self_value + other_value + 1).into()); } else { e.remove(); diff --git a/vortex-array/src/stream/ext.rs b/vortex-array/src/stream/ext.rs index 7f5008b3d1..58a8a7ef4e 100644 --- a/vortex-array/src/stream/ext.rs +++ b/vortex-array/src/stream/ext.rs @@ -15,8 +15,9 @@ pub trait ArrayStreamExt: ArrayStream { { async { let dtype = self.dtype().clone(); - let chunks: Vec = self.try_collect().await.unwrap(); - ChunkedArray::try_new(chunks, dtype) + self.try_collect() + .await + .and_then(|chunks| ChunkedArray::try_new(chunks, dtype)) } } diff --git a/vortex-array/src/typed.rs b/vortex-array/src/typed.rs index 2e25015c52..29c8123265 100644 --- a/vortex-array/src/typed.rs +++ b/vortex-array/src/typed.rs @@ -2,7 +2,7 @@ use std::sync::{Arc, OnceLock}; use vortex_buffer::Buffer; use vortex_dtype::DType; -use vortex_error::{vortex_bail, VortexError, VortexResult}; +use vortex_error::{vortex_bail, vortex_panic, VortexError, VortexResult}; use crate::stats::StatsSet; use crate::{Array, ArrayData, ArrayDef, AsArray, IntoArray, ToArray, TryDeserializeArrayMetadata}; @@ -44,7 +44,7 @@ impl TypedArray { .as_any() .downcast_ref::() .unwrap_or_else(|| { - panic!( + vortex_panic!( "Failed to downcast metadata to {} for typed array with ID {} and encoding {}", std::any::type_name::(), D::ID.as_ref(), @@ -55,7 +55,7 @@ impl TypedArray { .lazy_metadata .get_or_init(|| { D::Metadata::try_deserialize_metadata(v.metadata()).unwrap_or_else(|err| { - panic!( + vortex_panic!( "Failed to deserialize ArrayView metadata for typed array with ID {} and encoding {}: {}", D::ID.as_ref(), D::ENCODING.id().as_ref(), diff --git a/vortex-array/src/validity.rs b/vortex-array/src/validity.rs index 6cae93de60..d1b661a0e6 100644 --- a/vortex-array/src/validity.rs +++ b/vortex-array/src/validity.rs @@ -3,13 +3,15 @@ use std::ops::BitAnd; use arrow_buffer::{BooleanBuffer, BooleanBufferBuilder, NullBuffer}; use serde::{Deserialize, Serialize}; use vortex_dtype::{DType, Nullability}; -use vortex_error::{vortex_bail, VortexResult}; +use vortex_error::{ + vortex_bail, vortex_err, vortex_panic, VortexError, VortexExpect as _, VortexResult, +}; use crate::array::BoolArray; use crate::compute::unary::scalar_at_unchecked; use crate::compute::{filter, slice, take}; use crate::stats::ArrayStatistics; -use crate::{Array, IntoArray, IntoArrayVariant}; +use crate::{Array, ArrayDType, IntoArray, IntoArrayVariant}; pub trait ArrayValidity { fn is_valid(&self, index: usize) -> bool; @@ -31,7 +33,7 @@ impl ValidityMetadata { Self::AllValid => Validity::AllValid, Self::AllInvalid => Validity::AllInvalid, Self::Array => match array { - None => panic!("Missing validity array"), + None => vortex_panic!("Missing validity array"), Some(a) => Validity::Array(a), }, } @@ -95,7 +97,15 @@ impl Validity { match self { Self::NonNullable | Self::AllValid => true, Self::AllInvalid => false, - Self::Array(a) => bool::try_from(&scalar_at_unchecked(a, index)).unwrap(), + Self::Array(a) => { + bool::try_from(&scalar_at_unchecked(a, index)).unwrap_or_else(|err| { + vortex_panic!( + err, + "Failed to get bool from Validity Array at index {}", + index + ) + }) + } } } @@ -199,8 +209,17 @@ impl PartialEq for Validity { (Self::AllValid, Self::AllValid) => true, (Self::AllInvalid, Self::AllInvalid) => true, (Self::Array(a), Self::Array(b)) => { - a.clone().into_bool().unwrap().boolean_buffer() - == b.clone().into_bool().unwrap().boolean_buffer() + let a_buffer = a + .clone() + .into_bool() + .vortex_expect("Failed to get Validity Array as BoolArray") + .boolean_buffer(); + let b_buffer = b + .clone() + .into_bool() + .vortex_expect("Failed to get Validity Array as BoolArray") + .boolean_buffer(); + a_buffer == b_buffer } _ => false, } @@ -259,7 +278,7 @@ impl FromIterator for Validity { LogicalValidity::Array(array) => { let array_buffer = array .into_bool() - .expect("validity must flatten to BoolArray") + .vortex_expect("Failed to get Validity Array as BoolArray") .boolean_buffer(); buffer.append_buffer(&array_buffer); } @@ -285,6 +304,24 @@ pub enum LogicalValidity { } impl LogicalValidity { + pub fn try_new_from_array(array: Array) -> VortexResult { + if !matches!(array.dtype(), &Validity::DTYPE) { + vortex_bail!("Expected a non-nullable boolean array"); + } + + let true_count = array + .statistics() + .compute_true_count() + .ok_or_else(|| vortex_err!("Failed to compute true count from validity array"))?; + if true_count == array.len() { + return Ok(Self::AllValid(array.len())); + } else if true_count == 0 { + return Ok(Self::AllInvalid(array.len())); + } + + Ok(Self::Array(array)) + } + pub fn to_null_buffer(&self) -> VortexResult> { match self { Self::AllValid(_) => Ok(None), @@ -328,6 +365,14 @@ impl LogicalValidity { } } +impl TryFrom for LogicalValidity { + type Error = VortexError; + + fn try_from(array: Array) -> VortexResult { + Self::try_new_from_array(array) + } +} + impl IntoArray for LogicalValidity { fn into_array(self) -> Array { match self { diff --git a/vortex-array/src/variants.rs b/vortex-array/src/variants.rs index 6a4da9b5a0..1811768de7 100644 --- a/vortex-array/src/variants.rs +++ b/vortex-array/src/variants.rs @@ -4,6 +4,7 @@ //! encoding, they can use these traits to write encoding-agnostic code. use vortex_dtype::{DType, FieldNames}; +use vortex_error::VortexExpect as _; use crate::iter::{AccessorRef, VectorizedArrayIter}; use crate::{Array, ArrayTrait}; @@ -14,7 +15,7 @@ pub trait ArrayVariants { } fn as_null_array_unchecked(&self) -> &dyn NullArrayTrait { - self.as_null_array().expect("Expected NullArray") + self.as_null_array().vortex_expect("Expected NullArray") } fn as_bool_array(&self) -> Option<&dyn BoolArrayTrait> { @@ -22,7 +23,7 @@ pub trait ArrayVariants { } fn as_bool_array_unchecked(&self) -> &dyn BoolArrayTrait { - self.as_bool_array().expect("Expected BoolArray") + self.as_bool_array().vortex_expect("Expected BoolArray") } fn as_primitive_array(&self) -> Option<&dyn PrimitiveArrayTrait> { @@ -30,7 +31,8 @@ pub trait ArrayVariants { } fn as_primitive_array_unchecked(&self) -> &dyn PrimitiveArrayTrait { - self.as_primitive_array().expect("Expected PrimitiveArray") + self.as_primitive_array() + .vortex_expect("Expected PrimitiveArray") } fn as_utf8_array(&self) -> Option<&dyn Utf8ArrayTrait> { @@ -38,7 +40,7 @@ pub trait ArrayVariants { } fn as_utf8_array_unchecked(&self) -> &dyn Utf8ArrayTrait { - self.as_utf8_array().expect("Expected Utf8Array") + self.as_utf8_array().vortex_expect("Expected Utf8Array") } fn as_binary_array(&self) -> Option<&dyn BinaryArrayTrait> { @@ -46,7 +48,7 @@ pub trait ArrayVariants { } fn as_binary_array_unchecked(&self) -> &dyn BinaryArrayTrait { - self.as_binary_array().expect("Expected BinaryArray") + self.as_binary_array().vortex_expect("Expected BinaryArray") } fn as_struct_array(&self) -> Option<&dyn StructArrayTrait> { @@ -54,7 +56,7 @@ pub trait ArrayVariants { } fn as_struct_array_unchecked(&self) -> &dyn StructArrayTrait { - self.as_struct_array().expect("Expected StructArray") + self.as_struct_array().vortex_expect("Expected StructArray") } fn as_list_array(&self) -> Option<&dyn ListArrayTrait> { @@ -62,7 +64,7 @@ pub trait ArrayVariants { } fn as_list_array_unchecked(&self) -> &dyn ListArrayTrait { - self.as_list_array().expect("Expected ListArray") + self.as_list_array().vortex_expect("Expected ListArray") } fn as_extension_array(&self) -> Option<&dyn ExtensionArrayTrait> { @@ -70,7 +72,8 @@ pub trait ArrayVariants { } fn as_extension_array_unchecked(&self) -> &dyn ExtensionArrayTrait { - self.as_extension_array().expect("Expected ExtensionArray") + self.as_extension_array() + .vortex_expect("Expected ExtensionArray") } } diff --git a/vortex-array/src/view.rs b/vortex-array/src/view.rs index 5f33e0b3a0..cf21b42eb0 100644 --- a/vortex-array/src/view.rs +++ b/vortex-array/src/view.rs @@ -6,7 +6,7 @@ use itertools::Itertools; use log::warn; use vortex_buffer::Buffer; use vortex_dtype::{DType, Nullability}; -use vortex_error::{vortex_bail, vortex_err, VortexError, VortexResult}; +use vortex_error::{vortex_bail, vortex_err, VortexError, VortexExpect as _, VortexResult}; use vortex_scalar::{PValue, Scalar, ScalarValue}; use crate::encoding::EncodingRef; @@ -154,7 +154,7 @@ impl ArrayView { let mut collector = ChildrenCollector::default(); Array::View(self.clone()) .with_dyn(|a| a.accept(&mut collector)) - .unwrap(); + .vortex_expect("Failed to get children"); collector.children } diff --git a/vortex-datafusion/examples/table_provider.rs b/vortex-datafusion/examples/table_provider.rs index fbcb63c547..affa2bbb09 100644 --- a/vortex-datafusion/examples/table_provider.rs +++ b/vortex-datafusion/examples/table_provider.rs @@ -36,8 +36,7 @@ async fn main() -> anyhow::Result<()> { vec![strings, numbers], 8, Validity::NonNullable, - ) - .unwrap(); + )?; let filepath = temp_dir.path().join("a.vtx"); @@ -74,7 +73,7 @@ async fn main() -> anyhow::Result<()> { let ctx = SessionContext::new(); ctx.register_table("vortex_tbl", Arc::clone(&provider) as _)?; - let url = Url::try_from("file://").unwrap(); + let url = Url::try_from("file://")?; ctx.register_object_store(&url, object_store); run_query(&ctx, "SELECT * FROM vortex_tbl").await?; diff --git a/vortex-datafusion/src/datatype.rs b/vortex-datafusion/src/datatype.rs index 3baf21062d..6848850056 100644 --- a/vortex-datafusion/src/datatype.rs +++ b/vortex-datafusion/src/datatype.rs @@ -14,6 +14,7 @@ use arrow_schema::{DataType, Field, FieldRef, Fields, Schema, SchemaBuilder}; use vortex_datetime_dtype::arrow::make_arrow_temporal_dtype; use vortex_datetime_dtype::is_temporal_ext_type; use vortex_dtype::{DType, Nullability, PType}; +use vortex_error::vortex_panic; /// Convert a Vortex [struct DType][DType] to an Arrow [Schema]. /// @@ -23,11 +24,11 @@ use vortex_dtype::{DType, Nullability, PType}; /// has top-level nullability. pub(crate) fn infer_schema(dtype: &DType) -> Schema { let DType::Struct(struct_dtype, nullable) = dtype else { - panic!("only DType::Struct can be converted to arrow schema"); + vortex_panic!("only DType::Struct can be converted to arrow schema"); }; if *nullable != Nullability::NonNullable { - panic!("top-level struct in Schema must be NonNullable"); + vortex_panic!("top-level struct in Schema must be NonNullable"); } let mut builder = SchemaBuilder::with_capacity(struct_dtype.names().len()); @@ -94,7 +95,7 @@ pub(crate) fn infer_data_type(dtype: &DType) -> DataType { if is_temporal_ext_type(ext_dtype.id()) { make_arrow_temporal_dtype(ext_dtype) } else { - panic!("unsupported extension type \"{}\"", ext_dtype.id()) + vortex_panic!("Unsupported extension type \"{}\"", ext_dtype.id()) } } } diff --git a/vortex-datafusion/src/lib.rs b/vortex-datafusion/src/lib.rs index a2b9a154a2..71a1d916cd 100644 --- a/vortex-datafusion/src/lib.rs +++ b/vortex-datafusion/src/lib.rs @@ -219,7 +219,7 @@ impl Stream for VortexRecordBatchStream { let chunk = this .chunks .chunk(this.idx) - .expect("nchunks should match precomputed"); + .ok_or_else(|| vortex_err!("nchunks should match precomputed"))?; this.idx += 1; let struct_array = chunk @@ -233,7 +233,7 @@ impl Stream for VortexRecordBatchStream { exec_datafusion_err!("projection pushdown to Vortex failed: {vortex_err}") })?; - Poll::Ready(Some(Ok(projected_struct.into()))) + Poll::Ready(Some(Ok(projected_struct.try_into()?))) } fn size_hint(&self) -> (usize, Option) { diff --git a/vortex-datafusion/src/memory.rs b/vortex-datafusion/src/memory.rs index 7520568b42..61380793be 100644 --- a/vortex-datafusion/src/memory.rs +++ b/vortex-datafusion/src/memory.rs @@ -14,6 +14,7 @@ use datafusion_physical_plan::{ExecutionMode, ExecutionPlan, Partitioning, PlanP use itertools::Itertools; use vortex::array::ChunkedArray; use vortex::{Array, ArrayDType as _}; +use vortex_error::{VortexError, VortexExpect as _}; use vortex_expr::datafusion::convert_expr_to_vortex; use vortex_expr::VortexExpr; @@ -46,7 +47,8 @@ impl VortexMemTable { Ok(a) => a, _ => { let dtype = array.dtype().clone(); - ChunkedArray::try_new(vec![array], dtype).unwrap() + ChunkedArray::try_new(vec![array], dtype) + .vortex_expect("Failed to wrap array as a ChunkedArray with 1 chunk") } }; @@ -113,7 +115,7 @@ impl TableProvider for VortexMemTable { let output_schema = Arc::new( self.schema_ref .project(output_projection.as_slice()) - .expect("project output schema"), + .map_err(VortexError::from)?, ); let plan_properties = PlanProperties::new( EquivalenceProperties::new(output_schema), diff --git a/vortex-datafusion/src/persistent/opener.rs b/vortex-datafusion/src/persistent/opener.rs index 19f200562c..56f871d8e3 100644 --- a/vortex-datafusion/src/persistent/opener.rs +++ b/vortex-datafusion/src/persistent/opener.rs @@ -5,7 +5,7 @@ use arrow_schema::SchemaRef; use datafusion::datasource::physical_plan::{FileMeta, FileOpenFuture, FileOpener}; use datafusion_common::Result as DFResult; use datafusion_physical_expr::PhysicalExpr; -use futures::{FutureExt as _, TryStreamExt}; +use futures::{FutureExt as _, StreamExt, TryStreamExt}; use object_store::ObjectStore; use vortex::Context; use vortex_expr::datafusion::convert_expr_to_vortex; @@ -55,7 +55,8 @@ impl FileOpener for VortexFileOpener { builder .build() .await? - .map_ok(RecordBatch::from) + .map_ok(RecordBatch::try_from) + .map(|r| r.and_then(|inner| inner)) .map_err(|e| e.into()), ) as _) } diff --git a/vortex-datafusion/src/persistent/provider.rs b/vortex-datafusion/src/persistent/provider.rs index b4206160fb..f98b9217ec 100644 --- a/vortex-datafusion/src/persistent/provider.rs +++ b/vortex-datafusion/src/persistent/provider.rs @@ -81,7 +81,7 @@ impl TableProvider for VortexFileTableProvider { .data_files .iter() .cloned() - .map(|f| f.into()) + .map(Into::into) .collect(), ) .with_projection(projection.cloned()); diff --git a/vortex-datafusion/src/plans.rs b/vortex-datafusion/src/plans.rs index 8ac6b3f852..82cadace10 100644 --- a/vortex-datafusion/src/plans.rs +++ b/vortex-datafusion/src/plans.rs @@ -24,7 +24,7 @@ use vortex::arrow::FromArrowArray; use vortex::compute::take; use vortex::{Array, AsArray as _, IntoArray, IntoArrayVariant, IntoCanonical}; use vortex_dtype::field::Field; -use vortex_error::vortex_err; +use vortex_error::{vortex_err, vortex_panic, VortexError}; use vortex_expr::VortexExpr; /// Physical plan operator that applies a set of [filters][Expr] against the input, producing a @@ -150,28 +150,26 @@ impl Stream for RowIndicesStream { return Poll::Ready(None); } - let next_chunk = this - .chunked_array - .chunk(this.chunk_idx) - .expect("chunk index in-bounds"); + let next_chunk = this.chunked_array.chunk(this.chunk_idx).ok_or_else(|| { + vortex_err!( + "Chunk not found for index {}, nchunks: {}", + this.chunk_idx, + this.chunked_array.nchunks() + ) + })?; this.chunk_idx += 1; // Get the unfiltered record batch. // Since this is a one-shot, we only want to poll the inner future once, to create the // initial batch for us to process. - let vortex_struct = next_chunk - .into_struct() - .expect("chunks must be StructArray") - .project(&this.filter_projection) - .expect("projection should succeed"); + let vortex_struct = next_chunk.into_struct()?.project(&this.filter_projection)?; let selection = this .conjunction_expr .evaluate(vortex_struct.as_array_ref()) .map_err(|e| DataFusionError::External(e.into()))? - .into_canonical() - .unwrap() - .into_arrow(); + .into_canonical()? + .into_arrow()?; // Convert the `selection` BooleanArray into a UInt64Array of indices. let selection_indices = selection @@ -217,7 +215,9 @@ impl TakeRowsExec { row_indices: Arc, table: &ChunkedArray, ) -> Self { - let output_schema = Arc::new(schema_ref.project(projection).unwrap()); + let output_schema = Arc::new(schema_ref.project(projection).unwrap_or_else(|err| { + vortex_panic!("Failed to project schema: {}", VortexError::from(err)) + })); let plan_properties = PlanProperties::new( EquivalenceProperties::new(output_schema.clone()), Partitioning::UnknownPartitioning(1), @@ -347,27 +347,30 @@ where vec![], &opts, ) - .unwrap()))); + .map_err(DataFusionError::from)?))); } let chunk = this .vortex_array .chunk(*this.chunk_idx) - .expect("streamed too many chunks") - .into_struct() - .expect("chunks must be struct-encoded"); + .ok_or_else(|| { + vortex_err!( + "Chunk not found for index {}, nchunks: {}", + this.chunk_idx, + this.vortex_array.nchunks() + ) + })? + .into_struct()?; *this.chunk_idx += 1; // TODO(aduffy): this re-decodes the fields from the filter schema, which is wasteful. // We should find a way to avoid decoding the filter columns and only decode the other // columns, then stitch the StructArray back together from those. - let projected_for_output = chunk.project(this.output_projection).unwrap(); - let decoded = take(&projected_for_output.into_array(), &row_indices) - .expect("take") - .into_canonical() - .expect("into_canonical") - .into_arrow(); + let projected_for_output = chunk.project(this.output_projection)?; + let decoded = take(&projected_for_output.into_array(), &row_indices)? + .into_canonical()? + .into_arrow()?; // Send back a single record batch of the decoded data. let output_batch = RecordBatch::from(decoded.as_struct()); diff --git a/vortex-datetime-dtype/src/arrow.rs b/vortex-datetime-dtype/src/arrow.rs index 472edd7fc9..7af9d2c7fa 100644 --- a/vortex-datetime-dtype/src/arrow.rs +++ b/vortex-datetime-dtype/src/arrow.rs @@ -2,6 +2,7 @@ use arrow_schema::{DataType, TimeUnit as ArrowTimeUnit}; use vortex_dtype::ExtDType; +use vortex_error::{vortex_bail, vortex_panic, VortexError, VortexExpect as _, VortexResult}; use crate::temporal::{TemporalMetadata, DATE_ID, TIMESTAMP_ID, TIME_ID}; use crate::unit::TimeUnit; @@ -53,26 +54,32 @@ pub fn make_temporal_ext_dtype(data_type: &DataType) -> ExtDType { /// panics if the ext_dtype is not a temporal dtype pub fn make_arrow_temporal_dtype(ext_dtype: &ExtDType) -> DataType { match TemporalMetadata::try_from(ext_dtype) - .expect("make_arrow_temporal_dtype must be called with a temporal ExtDType") + .vortex_expect("make_arrow_temporal_dtype must be called with a temporal ExtDType") { TemporalMetadata::Date(time_unit) => match time_unit { TimeUnit::D => DataType::Date32, TimeUnit::Ms => DataType::Date64, - _ => panic!("Invalid TimeUnit {time_unit} for {}", ext_dtype.id()), + _ => { + vortex_panic!(InvalidArgument: "Invalid TimeUnit {} for {}", time_unit, ext_dtype.id()) + } }, TemporalMetadata::Time(time_unit) => match time_unit { TimeUnit::S => DataType::Time32(ArrowTimeUnit::Second), TimeUnit::Ms => DataType::Time32(ArrowTimeUnit::Millisecond), TimeUnit::Us => DataType::Time64(ArrowTimeUnit::Microsecond), TimeUnit::Ns => DataType::Time64(ArrowTimeUnit::Nanosecond), - _ => panic!("Invalid TimeUnit {time_unit} for {}", ext_dtype.id()), + _ => { + vortex_panic!(InvalidArgument: "Invalid TimeUnit {} for {}", time_unit, ext_dtype.id()) + } }, TemporalMetadata::Timestamp(time_unit, tz) => match time_unit { TimeUnit::Ns => DataType::Timestamp(ArrowTimeUnit::Nanosecond, tz.map(|t| t.into())), TimeUnit::Us => DataType::Timestamp(ArrowTimeUnit::Microsecond, tz.map(|t| t.into())), TimeUnit::Ms => DataType::Timestamp(ArrowTimeUnit::Millisecond, tz.map(|t| t.into())), TimeUnit::S => DataType::Timestamp(ArrowTimeUnit::Second, tz.map(|t| t.into())), - _ => panic!("Invalid TimeUnit {time_unit} for {}", ext_dtype.id()), + _ => { + vortex_panic!(InvalidArgument: "Invalid TimeUnit {} for {}", time_unit, ext_dtype.id()) + } }, } } @@ -94,14 +101,16 @@ impl From for TimeUnit { } } -impl From for ArrowTimeUnit { - fn from(value: TimeUnit) -> Self { - match value { +impl TryFrom for ArrowTimeUnit { + type Error = VortexError; + + fn try_from(value: TimeUnit) -> VortexResult { + Ok(match value { TimeUnit::S => Self::Second, TimeUnit::Ms => Self::Millisecond, TimeUnit::Us => Self::Microsecond, TimeUnit::Ns => Self::Nanosecond, - _ => panic!("cannot convert {value} to Arrow TimeUnit"), - } + _ => vortex_bail!("Cannot convert {value} to Arrow TimeUnit"), + }) } } diff --git a/vortex-datetime-dtype/src/temporal.rs b/vortex-datetime-dtype/src/temporal.rs index b26b11dc5a..e824c1952c 100644 --- a/vortex-datetime-dtype/src/temporal.rs +++ b/vortex-datetime-dtype/src/temporal.rs @@ -98,7 +98,7 @@ impl TemporalMetadata { } use vortex_dtype::{ExtDType, ExtMetadata}; -use vortex_error::{vortex_bail, vortex_err, VortexError, VortexResult}; +use vortex_error::{vortex_bail, vortex_err, vortex_panic, VortexError, VortexResult}; impl TryFrom<&ExtDType> for TemporalMetadata { type Error = VortexError; @@ -174,12 +174,12 @@ impl From for ExtMetadata { None => meta.extend_from_slice(0u16.to_le_bytes().as_slice()), Some(tz) => { let tz_bytes = tz.as_bytes(); - let tz_len = u16::try_from(tz_bytes.len()).expect("tz did not fit in u16"); + let tz_len = u16::try_from(tz_bytes.len()) + .unwrap_or_else(|err| vortex_panic!("tz did not fit in u16: {}", err)); meta.extend_from_slice(tz_len.to_le_bytes().as_slice()); meta.extend_from_slice(tz_bytes); } - }; - + } ExtMetadata::from(meta.as_slice()) } } diff --git a/vortex-dtype/src/dtype.rs b/vortex-dtype/src/dtype.rs index c513604962..2aeefd4d11 100644 --- a/vortex-dtype/src/dtype.rs +++ b/vortex-dtype/src/dtype.rs @@ -51,7 +51,7 @@ impl DType { Primitive(_, n) => matches!(n, Nullable), Utf8(n) => matches!(n, Nullable), Binary(n) => matches!(n, Nullable), - Struct(st, _) => st.dtypes().iter().all(|f| f.is_nullable()), + Struct(st, _) => st.dtypes().iter().all(DType::is_nullable), List(_, n) => matches!(n, Nullable), Extension(_, n) => matches!(n, Nullable), } @@ -87,27 +87,19 @@ impl DType { } pub fn is_unsigned_int(&self) -> bool { - PType::try_from(self) - .map(|ptype| ptype.is_unsigned_int()) - .unwrap_or_default() + PType::try_from(self).is_ok_and(PType::is_unsigned_int) } pub fn is_signed_int(&self) -> bool { - PType::try_from(self) - .map(|ptype| ptype.is_signed_int()) - .unwrap_or_default() + PType::try_from(self).is_ok_and(PType::is_signed_int) } pub fn is_int(&self) -> bool { - PType::try_from(self) - .map(|ptype| ptype.is_int()) - .unwrap_or_default() + PType::try_from(self).is_ok_and(PType::is_int) } pub fn is_float(&self) -> bool { - PType::try_from(self) - .map(|ptype| ptype.is_float()) - .unwrap_or_default() + PType::try_from(self).is_ok_and(PType::is_float) } pub fn is_boolean(&self) -> bool { diff --git a/vortex-dtype/src/field.rs b/vortex-dtype/src/field.rs index af18979bd4..044dfe18dd 100644 --- a/vortex-dtype/src/field.rs +++ b/vortex-dtype/src/field.rs @@ -2,6 +2,7 @@ use core::fmt; use std::fmt::{Display, Formatter}; use itertools::Itertools; +use vortex_error::vortex_panic; #[derive(Clone, Debug, PartialEq, Eq, Hash)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] @@ -58,7 +59,7 @@ impl FieldPath { assert_eq!(self.0.len(), 1); match &self.0[0] { Field::Name(name) => name.as_str(), - _ => panic!("FieldPath is not a name"), + _ => vortex_panic!("FieldPath is not a name: {}", self), } } } diff --git a/vortex-error/src/lib.rs b/vortex-error/src/lib.rs index 3c7c83fca7..6e76ea2fed 100644 --- a/vortex-error/src/lib.rs +++ b/vortex-error/src/lib.rs @@ -14,9 +14,10 @@ impl From for ErrString where T: Into>, { + #[allow(clippy::panic)] fn from(msg: T) -> Self { if env::var("VORTEX_PANIC_ON_ERR").as_deref().unwrap_or("") == "1" { - panic!("{}", msg.into()) + panic!("{}\nBacktrace:\n{}", msg.into(), Backtrace::capture()); } else { Self(msg.into()) } @@ -57,6 +58,10 @@ pub enum VortexError { NotImplemented(ErrString, ErrString, Backtrace), #[error("expected type: {0} but instead got {1}\nBacktrace:\n{2}")] MismatchedTypes(ErrString, ErrString, Backtrace), + #[error("{0}\nBacktrace:\n{1}")] + AssertionFailed(ErrString, Backtrace), + #[error("{0}: {1}")] + Context(ErrString, Box), #[error(transparent)] ArrowError( #[from] @@ -136,6 +141,13 @@ pub enum VortexError { #[backtrace] object_store::Error, ), + #[cfg(feature = "datafusion")] + #[error(transparent)] + DataFusion( + #[from] + #[backtrace] + datafusion_common::DataFusionError, + ), #[error(transparent)] JiffError( #[from] @@ -144,7 +156,11 @@ pub enum VortexError { ), } -pub type VortexResult = Result; +impl VortexError { + pub fn with_context>(self, msg: T) -> Self { + VortexError::Context(msg.into(), Box::new(self)) + } +} impl Debug for VortexError { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { @@ -152,6 +168,50 @@ impl Debug for VortexError { } } +pub type VortexResult = Result; + +pub trait VortexUnwrap { + type Output; + + fn vortex_unwrap(self) -> Self::Output; +} + +impl VortexUnwrap for VortexResult { + type Output = T; + + #[inline(always)] + fn vortex_unwrap(self) -> Self::Output { + self.unwrap_or_else(|err| vortex_panic!(err)) + } +} + +pub trait VortexExpect { + type Output; + + fn vortex_expect(self, msg: &str) -> Self::Output; +} + +impl VortexExpect for VortexResult { + type Output = T; + + #[inline(always)] + fn vortex_expect(self, msg: &str) -> Self::Output { + self.unwrap_or_else(|e| vortex_panic!(e.with_context(msg.to_string()))) + } +} + +impl VortexExpect for Option { + type Output = T; + + #[inline(always)] + fn vortex_expect(self, msg: &str) -> Self::Output { + self.unwrap_or_else(|| { + let err = VortexError::AssertionFailed(msg.to_string().into(), Backtrace::capture()); + vortex_panic!(err) + }) + } +} + #[macro_export] macro_rules! vortex_err { (OutOfBounds: $idx:expr, $start:expr, $stop:expr) => {{ @@ -178,6 +238,11 @@ macro_rules! vortex_err { $crate::VortexError::MismatchedTypes($expected.to_string().into(), $actual.to_string().into(), Backtrace::capture()) ) }}; + (Context: $msg:literal, $err:expr) => {{ + $crate::__private::must_use( + $crate::VortexError::Context($msg.into(), Box::new($err)) + ) + }}; ($variant:ident: $fmt:literal $(, $arg:expr)* $(,)?) => {{ use std::backtrace::Backtrace; $crate::__private::must_use( @@ -201,6 +266,39 @@ macro_rules! vortex_bail { }; } +#[macro_export] +macro_rules! vortex_panic { + (OutOfBounds: $idx:expr, $start:expr, $stop:expr) => {{ + $crate::vortex_panic!($crate::vortex_err!(OutOfBounds: $idx, $start, $stop)) + }}; + (NotImplemented: $func:expr, $for_whom:expr) => {{ + $crate::vortex_panic!($crate::vortex_err!(NotImplemented: $func, $for_whom)) + }}; + (MismatchedTypes: $expected:literal, $actual:expr) => {{ + $crate::vortex_panic!($crate::vortex_err!(MismatchedTypes: $expected, $actual)) + }}; + (MismatchedTypes: $expected:expr, $actual:expr) => {{ + $crate::vortex_panic!($crate::vortex_err!(MismatchedTypes: $expected, $actual)) + }}; + (Context: $msg:literal, $err:expr) => {{ + $crate::vortex_panic!($crate::vortex_err!(Context: $msg, $err)) + }}; + ($variant:ident: $fmt:literal $(, $arg:expr)* $(,)?) => { + $crate::vortex_panic!($crate::vortex_err!($variant: $fmt, $($arg),*)) + }; + ($err:expr, $fmt:literal $(, $arg:expr)* $(,)?) => {{ + let err: $crate::VortexError = $err; + panic!("{}", err.with_context(format!($fmt, $($arg),*))) + }}; + ($fmt:literal $(, $arg:expr)* $(,)?) => { + $crate::vortex_panic!($crate::vortex_err!($fmt, $($arg),*)) + }; + ($err:expr) => {{ + let err: $crate::VortexError = $err; + panic!("{}", err) + }}; +} + #[cfg(feature = "datafusion")] impl From for datafusion_common::DataFusionError { fn from(value: VortexError) -> Self { diff --git a/vortex-expr/src/expr.rs b/vortex-expr/src/expr.rs index 7e4b47fa1c..1164349208 100644 --- a/vortex-expr/src/expr.rs +++ b/vortex-expr/src/expr.rs @@ -8,7 +8,7 @@ use vortex::compute::{compare, Operator as ArrayOperator}; use vortex::variants::StructArrayTrait; use vortex::{Array, IntoArray}; use vortex_dtype::field::Field; -use vortex_error::{vortex_bail, vortex_err, VortexResult}; +use vortex_error::{vortex_bail, vortex_err, VortexExpect as _, VortexResult}; use vortex_scalar::Scalar; use crate::Operator; @@ -24,15 +24,19 @@ pub trait VortexExpr: Debug + Send + Sync + PartialEq { // Taken from apache-datafusion, necessary since you can't require VortexExpr implement PartialEq fn unbox_any(any: &dyn Any) -> &dyn Any { if any.is::>() { - any.downcast_ref::>().unwrap().as_any() + any.downcast_ref::>() + .vortex_expect("any.is::> returned true but downcast_ref failed") + .as_any() } else if any.is::>() { - any.downcast_ref::>().unwrap().as_any() + any.downcast_ref::>() + .vortex_expect("any.is::> returned true but downcast_ref failed") + .as_any() } else { any } } -#[derive(Debug, PartialEq, Hash, Clone)] +#[derive(Debug, PartialEq, Hash, Clone, Eq)] pub struct NoOp; #[derive(Debug, Clone)] @@ -44,7 +48,7 @@ pub struct BinaryExpr { impl BinaryExpr { pub fn new(lhs: Arc, operator: Operator, rhs: Arc) -> Self { - Self { lhs, rhs, operator } + Self { lhs, operator, rhs } } pub fn lhs(&self) -> &Arc { @@ -60,7 +64,7 @@ impl BinaryExpr { } } -#[derive(Debug, PartialEq, Hash, Clone)] +#[derive(Debug, PartialEq, Hash, Clone, Eq)] pub struct Column { field: Field, } diff --git a/vortex-flatbuffers/src/generated/array.rs b/vortex-flatbuffers/src/generated/array.rs index 7ba6ba97c7..8f076eaa94 100644 --- a/vortex-flatbuffers/src/generated/array.rs +++ b/vortex-flatbuffers/src/generated/array.rs @@ -93,7 +93,7 @@ impl<'a> flatbuffers::Verifiable for Version { impl flatbuffers::SimpleToVerifyInSlice for Version {} pub enum ArrayOffset {} -#[derive(Copy, Clone, PartialEq)] +#[derive(Copy, Clone, PartialEq, Eq)] pub struct Array<'a> { pub _tab: flatbuffers::Table<'a>, @@ -275,7 +275,7 @@ impl core::fmt::Debug for Array<'_> { } } pub enum ArrayStatsOffset {} -#[derive(Copy, Clone, PartialEq)] +#[derive(Copy, Clone, PartialEq, Eq)] pub struct ArrayStats<'a> { pub _tab: flatbuffers::Table<'a>, diff --git a/vortex-flatbuffers/src/generated/dtype.rs b/vortex-flatbuffers/src/generated/dtype.rs index b86459ec7a..7c2c24b921 100644 --- a/vortex-flatbuffers/src/generated/dtype.rs +++ b/vortex-flatbuffers/src/generated/dtype.rs @@ -250,7 +250,7 @@ impl flatbuffers::SimpleToVerifyInSlice for Type {} pub struct TypeUnionTableOffset {} pub enum NullOffset {} -#[derive(Copy, Clone, PartialEq)] +#[derive(Copy, Clone, PartialEq, Eq)] pub struct Null<'a> { pub _tab: flatbuffers::Table<'a>, @@ -329,7 +329,7 @@ impl core::fmt::Debug for Null<'_> { } } pub enum BoolOffset {} -#[derive(Copy, Clone, PartialEq)] +#[derive(Copy, Clone, PartialEq, Eq)] pub struct Bool<'a> { pub _tab: flatbuffers::Table<'a>, @@ -540,7 +540,7 @@ impl core::fmt::Debug for Primitive<'_> { } } pub enum DecimalOffset {} -#[derive(Copy, Clone, PartialEq)] +#[derive(Copy, Clone, PartialEq, Eq)] pub struct Decimal<'a> { pub _tab: flatbuffers::Table<'a>, @@ -673,7 +673,7 @@ impl core::fmt::Debug for Decimal<'_> { } } pub enum Utf8Offset {} -#[derive(Copy, Clone, PartialEq)] +#[derive(Copy, Clone, PartialEq, Eq)] pub struct Utf8<'a> { pub _tab: flatbuffers::Table<'a>, @@ -770,7 +770,7 @@ impl core::fmt::Debug for Utf8<'_> { } } pub enum BinaryOffset {} -#[derive(Copy, Clone, PartialEq)] +#[derive(Copy, Clone, PartialEq, Eq)] pub struct Binary<'a> { pub _tab: flatbuffers::Table<'a>, diff --git a/vortex-flatbuffers/src/lib.rs b/vortex-flatbuffers/src/lib.rs index 9f16b5fa5a..30acd8338b 100644 --- a/vortex-flatbuffers/src/lib.rs +++ b/vortex-flatbuffers/src/lib.rs @@ -1,5 +1,6 @@ #[cfg(feature = "array")] #[allow(clippy::all)] +#[allow(clippy::derive_partial_eq_without_eq)] #[allow(clippy::unwrap_used)] #[allow(dead_code)] #[allow(non_snake_case)] @@ -12,6 +13,7 @@ pub mod array; #[cfg(feature = "dtype")] #[allow(clippy::all)] +#[allow(clippy::derive_partial_eq_without_eq)] #[allow(clippy::unwrap_used)] #[allow(dead_code)] #[allow(non_snake_case)] @@ -24,6 +26,7 @@ pub mod dtype; #[cfg(feature = "scalar")] #[allow(clippy::all)] +#[allow(clippy::derive_partial_eq_without_eq)] #[allow(clippy::unwrap_used)] #[allow(dead_code)] #[allow(non_snake_case)] @@ -36,6 +39,7 @@ pub mod scalar; #[cfg(feature = "file")] #[allow(clippy::all)] +#[allow(clippy::derive_partial_eq_without_eq)] #[allow(clippy::unwrap_used)] #[allow(dead_code)] #[allow(non_snake_case)] @@ -48,6 +52,7 @@ pub mod footer; #[cfg(feature = "file")] #[allow(clippy::all)] +#[allow(clippy::derive_partial_eq_without_eq)] #[allow(clippy::unwrap_used)] #[allow(dead_code)] #[allow(non_snake_case)] diff --git a/vortex-sampling-compressor/src/compressors/bitpacked.rs b/vortex-sampling-compressor/src/compressors/bitpacked.rs index 0e895820cb..cef069f298 100644 --- a/vortex-sampling-compressor/src/compressors/bitpacked.rs +++ b/vortex-sampling-compressor/src/compressors/bitpacked.rs @@ -30,7 +30,7 @@ impl EncodingCompressor for BitPackedCompressor { return None; } - let bit_width = find_best_bit_width(&parray)?; + let bit_width = find_best_bit_width(&parray).ok()?; // Check that the bit width is less than the type's bit width if bit_width == parray.ptype().bit_width() { @@ -52,8 +52,7 @@ impl EncodingCompressor for BitPackedCompressor { .compute_bit_width_freq() .ok_or_else(|| vortex_err!(ComputeError: "missing bit width frequency"))?; - let bit_width = find_best_bit_width(&parray) - .ok_or_else(|| vortex_err!(ComputeError: "missing bit width frequency"))?; + let bit_width = find_best_bit_width(&parray)?; let num_exceptions = count_exceptions(bit_width, &bit_width_freq); if bit_width == parray.ptype().bit_width() { diff --git a/vortex-sampling-compressor/src/compressors/fsst.rs b/vortex-sampling-compressor/src/compressors/fsst.rs index 13e645db06..b5d8d2834e 100644 --- a/vortex-sampling-compressor/src/compressors/fsst.rs +++ b/vortex-sampling-compressor/src/compressors/fsst.rs @@ -64,7 +64,8 @@ impl EncodingCompressor for FSSTCompressor { let compressor = like .and_then(|mut tree| tree.metadata()) - .unwrap_or_else(|| Arc::new(fsst_train_compressor(array))); + .map(VortexResult::Ok) + .unwrap_or_else(|| Ok(Arc::new(fsst_train_compressor(array)?)))?; let Some(fsst_compressor) = compressor.as_any().downcast_ref::() else { vortex_bail!("Could not downcast metadata as FSST Compressor") @@ -73,11 +74,11 @@ impl EncodingCompressor for FSSTCompressor { let result_array = if array.encoding().id() == VarBin::ID || array.encoding().id() == VarBinView::ID { // For a VarBinArray or VarBinViewArray, compress directly. - fsst_compress(array, fsst_compressor).into_array() + fsst_compress(array, fsst_compressor)?.into_array() } else { vortex_bail!( - InvalidArgument: "unsupported encoding for FSSTCompressor {:?}", - array.encoding().id() + "Unsupported encoding for FSSTCompressor: {}", + array.encoding().id() ) }; diff --git a/vortex-sampling-compressor/src/lib.rs b/vortex-sampling-compressor/src/lib.rs index ad5207672e..4738875418 100644 --- a/vortex-sampling-compressor/src/lib.rs +++ b/vortex-sampling-compressor/src/lib.rs @@ -88,7 +88,7 @@ impl Display for SamplingCompressor<'_> { impl CompressionStrategy for SamplingCompressor<'_> { #[allow(clippy::same_name_method)] fn compress(&self, array: &Array) -> VortexResult { - Self::compress(self, array, None).map(|c| c.into_array()) + Self::compress(self, array, None).map(compressors::CompressedArray::into_array) } fn used_encodings(&self) -> HashSet { @@ -203,7 +203,10 @@ impl<'a> SamplingCompressor<'a> { let chunked = ChunkedArray::try_from(arr)?; let compressed_chunks = chunked .chunks() - .map(|chunk| self.compress_array(&chunk).map(|a| a.into_array())) + .map(|chunk| { + self.compress_array(&chunk) + .map(compressors::CompressedArray::into_array) + }) .collect::>>()?; Ok(CompressedArray::uncompressed( ChunkedArray::try_new(compressed_chunks, chunked.dtype().clone())?.into_array(), @@ -218,7 +221,10 @@ impl<'a> SamplingCompressor<'a> { let strct = StructArray::try_from(arr)?; let compressed_fields = strct .children() - .map(|field| self.compress_array(&field).map(|a| a.into_array())) + .map(|field| { + self.compress_array(&field) + .map(compressors::CompressedArray::into_array) + }) .collect::>>()?; let validity = self.compress_validity(strct.validity())?; Ok(CompressedArray::uncompressed( diff --git a/vortex-scalar/src/arrow.rs b/vortex-scalar/src/arrow.rs index e72dad7b80..32a3a46773 100644 --- a/vortex-scalar/src/arrow.rs +++ b/vortex-scalar/src/arrow.rs @@ -3,29 +3,29 @@ use std::sync::Arc; use arrow_array::*; use vortex_datetime_dtype::{is_temporal_ext_type, TemporalMetadata, TimeUnit}; use vortex_dtype::{DType, PType}; +use vortex_error::{vortex_bail, VortexError}; use crate::{PValue, Scalar}; macro_rules! value_to_arrow_scalar { ($V:expr, $AR:ty) => { - std::sync::Arc::new( + Ok(std::sync::Arc::new( $V.map(<$AR>::new_scalar) .unwrap_or_else(|| arrow_array::Scalar::new(<$AR>::new_null(1))), - ) + )) }; } -impl From<&Scalar> for Arc { - fn from(value: &Scalar) -> Arc { +impl TryFrom<&Scalar> for Arc { + type Error = VortexError; + + fn try_from(value: &Scalar) -> Result, Self::Error> { match value.dtype() { - DType::Null => Arc::new(NullArray::new(1)), - DType::Bool(_) => value_to_arrow_scalar!( - value.value.as_bool().expect("should be a bool"), - BooleanArray - ), + DType::Null => Ok(Arc::new(NullArray::new(1))), + DType::Bool(_) => value_to_arrow_scalar!(value.value.as_bool()?, BooleanArray), DType::Primitive(ptype, _) => { - let pvalue = value.value.as_pvalue().expect("should be pvalue"); - match pvalue { + let pvalue = value.value.as_pvalue()?; + Ok(match pvalue { None => match ptype { PType::U8 => Arc::new(UInt8Array::new_null(1)), PType::U16 => Arc::new(UInt16Array::new_null(1)), @@ -52,22 +52,13 @@ impl From<&Scalar> for Arc { PValue::F32(v) => Arc::new(Float32Array::new_scalar(v)), PValue::F64(v) => Arc::new(Float64Array::new_scalar(v)), }, - } + }) } DType::Utf8(_) => { - value_to_arrow_scalar!( - value - .value - .as_buffer_string() - .expect("should be buffer string"), - StringArray - ) + value_to_arrow_scalar!(value.value.as_buffer_string()?, StringArray) } DType::Binary(_) => { - value_to_arrow_scalar!( - value.value.as_buffer().expect("should be a buffer"), - BinaryArray - ) + value_to_arrow_scalar!(value.value.as_buffer()?, BinaryArray) } DType::Struct(..) => { todo!("struct scalar conversion") @@ -77,8 +68,8 @@ impl From<&Scalar> for Arc { } DType::Extension(ext, _) => { if is_temporal_ext_type(ext.id()) { - let metadata = TemporalMetadata::try_from(ext).unwrap(); - let pv = value.value.as_pvalue().expect("must be a pvalue"); + let metadata = TemporalMetadata::try_from(ext)?; + let pv = value.value.as_pvalue()?; return match metadata { TemporalMetadata::Time(u) => match u { TimeUnit::Ns => value_to_arrow_scalar!( @@ -98,7 +89,7 @@ impl From<&Scalar> for Arc { Time32SecondArray ), TimeUnit::D => { - unreachable!("Unsupported TimeUnit {u} for {}", ext.id()) + vortex_bail!("Unsupported TimeUnit {u} for {}", ext.id()) } }, TemporalMetadata::Date(u) => match u { @@ -108,7 +99,7 @@ impl From<&Scalar> for Arc { TimeUnit::D => { value_to_arrow_scalar!(pv.and_then(|p| p.as_i32()), Date32Array) } - _ => unreachable!("Unsupported TimeUnit {u} for {}", ext.id()), + _ => vortex_bail!("Unsupported TimeUnit {u} for {}", ext.id()), }, TemporalMetadata::Timestamp(u, _) => match u { TimeUnit::Ns => value_to_arrow_scalar!( @@ -128,7 +119,7 @@ impl From<&Scalar> for Arc { TimestampSecondArray ), TimeUnit::D => { - unreachable!("Unsupported TimeUnit {u} for {}", ext.id()) + vortex_bail!("Unsupported TimeUnit {u} for {}", ext.id()) } }, }; diff --git a/vortex-scalar/src/datafusion.rs b/vortex-scalar/src/datafusion.rs index 9f8dd9f24f..c5edb6ac43 100644 --- a/vortex-scalar/src/datafusion.rs +++ b/vortex-scalar/src/datafusion.rs @@ -4,16 +4,19 @@ use vortex_buffer::Buffer; use vortex_datetime_dtype::arrow::make_temporal_ext_dtype; use vortex_datetime_dtype::{is_temporal_ext_type, TemporalMetadata, TimeUnit}; use vortex_dtype::{DType, Nullability, PType}; +use vortex_error::VortexError; use crate::{PValue, Scalar}; -impl From for ScalarValue { - fn from(value: Scalar) -> Self { - match value.dtype { +impl TryFrom for ScalarValue { + type Error = VortexError; + + fn try_from(value: Scalar) -> Result { + Ok(match value.dtype { DType::Null => ScalarValue::Null, - DType::Bool(_) => ScalarValue::Boolean(value.value.as_bool().expect("should be bool")), + DType::Bool(_) => ScalarValue::Boolean(value.value.as_bool()?), DType::Primitive(ptype, _) => { - let pvalue = value.value.as_pvalue().expect("should be pvalue"); + let pvalue = value.value.as_pvalue()?; match pvalue { None => match ptype { PType::U8 => ScalarValue::UInt8(None), @@ -46,16 +49,14 @@ impl From for ScalarValue { DType::Utf8(_) => ScalarValue::Utf8( value .value - .as_buffer_string() - .expect("should be buffer string") + .as_buffer_string()? .map(|b| b.as_str().to_string()), ), DType::Binary(_) => ScalarValue::Binary( value .value - .as_buffer() - .expect("should be buffer") - .map(|b| b.as_slice().to_vec()), + .as_buffer()? + .map(|b| b.into_vec().unwrap_or_else(|buf| buf.as_slice().to_vec())), ), DType::Struct(..) => { todo!("struct scalar conversion") @@ -65,9 +66,9 @@ impl From for ScalarValue { } DType::Extension(ext, _) => { if is_temporal_ext_type(ext.id()) { - let metadata = TemporalMetadata::try_from(&ext).unwrap(); - let pv = value.value.as_pvalue().expect("must be a pvalue"); - return match metadata { + let metadata = TemporalMetadata::try_from(&ext)?; + let pv = value.value.as_pvalue()?; + return Ok(match metadata { TemporalMetadata::Time(u) => match u { TimeUnit::Ns => { ScalarValue::Time64Nanosecond(pv.and_then(|p| p.as_i64())) @@ -109,12 +110,12 @@ impl From for ScalarValue { unreachable!("Unsupported TimeUnit {u} for {}", ext.id()) } }, - }; + }); } todo!("Non temporal extension scalar conversion") } - } + }) } } diff --git a/vortex-scalar/src/list.rs b/vortex-scalar/src/list.rs index 14942057be..2ecac732ab 100644 --- a/vortex-scalar/src/list.rs +++ b/vortex-scalar/src/list.rs @@ -4,7 +4,7 @@ use std::sync::Arc; use itertools::Itertools; use vortex_dtype::DType; use vortex_dtype::Nullability::NonNullable; -use vortex_error::{vortex_bail, VortexError, VortexResult}; +use vortex_error::{vortex_bail, VortexError, VortexExpect as _, VortexResult}; use crate::value::ScalarValue; use crate::Scalar; @@ -53,7 +53,7 @@ impl<'a> ListScalar<'a> { pub fn elements(&self) -> impl Iterator + '_ { self.elements .as_ref() - .map(|e| e.as_ref()) + .map(AsRef::as_ref) .unwrap_or_else(|| &[] as &[ScalarValue]) .iter() .map(|e| Scalar { @@ -110,7 +110,11 @@ where { fn from(value: Vec) -> Self { let scalars = value.into_iter().map(|v| Self::from(v)).collect_vec(); - let element_dtype = scalars.first().expect("Empty list").dtype().clone(); + let element_dtype = scalars + .first() + .vortex_expect("Empty list, could not determine element dtype") + .dtype() + .clone(); let dtype = DType::List(Arc::new(element_dtype), NonNullable); Self { dtype, diff --git a/vortex-scalar/src/primitive.rs b/vortex-scalar/src/primitive.rs index 405a58b6a1..135b65df25 100644 --- a/vortex-scalar/src/primitive.rs +++ b/vortex-scalar/src/primitive.rs @@ -1,9 +1,9 @@ -use core::any::type_name; - use num_traits::NumCast; use vortex_dtype::half::f16; use vortex_dtype::{match_each_native_ptype, DType, NativePType, Nullability, PType}; -use vortex_error::{vortex_bail, vortex_err, VortexError, VortexResult}; +use vortex_error::{ + vortex_bail, vortex_err, vortex_panic, VortexError, VortexResult, VortexUnwrap, +}; use crate::pvalue::PValue; use crate::value::ScalarValue; @@ -36,11 +36,9 @@ impl<'a> PrimitiveScalar<'a> { T::PTYPE ); - self.pvalue.as_ref().map(|pv| { - T::try_from(*pv).unwrap_or_else(|err| { - panic!("Failed to cast {} to {}: {}", pv, type_name::(), err) - }) - }) + self.pvalue + .as_ref() + .map(|pv| T::try_from(*pv).vortex_unwrap()) } pub fn cast(&self, dtype: &DType) -> VortexResult { @@ -94,11 +92,8 @@ impl Scalar { } pub fn reinterpret_cast(&self, ptype: PType) -> Self { - let primitive = PrimitiveScalar::try_from(self).unwrap_or_else(|err| { - panic!( - "Failed to reinterpret cast {} to {}: {}", - self.dtype, ptype, err - ) + let primitive = PrimitiveScalar::try_from(self).unwrap_or_else(|e| { + vortex_panic!(e, "Failed to reinterpret cast {} to {}", self.dtype, ptype) }); if primitive.ptype() == ptype { return self.clone(); diff --git a/vortex-scalar/src/serde/flatbuffers.rs b/vortex-scalar/src/serde/flatbuffers.rs index a137f9912b..2fc80db010 100644 --- a/vortex-scalar/src/serde/flatbuffers.rs +++ b/vortex-scalar/src/serde/flatbuffers.rs @@ -2,7 +2,7 @@ use flatbuffers::{FlatBufferBuilder, WIPOffset}; use itertools::Itertools; use serde::{Deserialize, Serialize}; use vortex_dtype::DType; -use vortex_error::VortexError; +use vortex_error::{VortexError, VortexExpect as _}; use vortex_flatbuffers::{scalar as fb, WriteFlatBuffer}; use crate::{Scalar, ScalarValue}; @@ -54,7 +54,8 @@ impl WriteFlatBuffer for ScalarValue { ) -> WIPOffset> { let mut value_se = flexbuffers::FlexbufferSerializer::new(); self.serialize(&mut value_se) - .expect("Failed to serialize ScalarValue"); + .map_err(VortexError::FlexBuffersSerError) + .vortex_expect("Failed to serialize ScalarValue"); let flex = Some(fbb.create_vector(value_se.view())); fb::ScalarValue::create(fbb, &fb::ScalarValueArgs { flex }) } diff --git a/vortex-serde/benches/ipc_array_reader_take.rs b/vortex-serde/benches/ipc_array_reader_take.rs index 7a649e72b0..dac3367124 100644 --- a/vortex-serde/benches/ipc_array_reader_take.rs +++ b/vortex-serde/benches/ipc_array_reader_take.rs @@ -1,3 +1,4 @@ +#![allow(clippy::unwrap_used)] use std::sync::Arc; use criterion::async_executor::FuturesExecutor; diff --git a/vortex-serde/benches/ipc_take.rs b/vortex-serde/benches/ipc_take.rs index 37aaabfacd..70d1d59424 100644 --- a/vortex-serde/benches/ipc_take.rs +++ b/vortex-serde/benches/ipc_take.rs @@ -1,3 +1,4 @@ +#![allow(clippy::unwrap_used)] use std::sync::Arc; use arrow::ipc::reader::StreamReader as ArrowStreamReader; diff --git a/vortex-serde/src/chunked_reader/mod.rs b/vortex-serde/src/chunked_reader/mod.rs index 3d845ae259..fc1b6585d6 100644 --- a/vortex-serde/src/chunked_reader/mod.rs +++ b/vortex-serde/src/chunked_reader/mod.rs @@ -5,7 +5,7 @@ use vortex::compute::unary::scalar_at; use vortex::stream::ArrayStream; use vortex::{Array, Context}; use vortex_dtype::DType; -use vortex_error::{vortex_bail, VortexResult}; +use vortex_error::{vortex_bail, VortexExpect as _, VortexResult}; use crate::io::VortexReadAt; use crate::stream_reader::StreamArrayReader; @@ -54,10 +54,14 @@ impl ChunkedArrayReader { pub async fn array_stream(&mut self) -> impl ArrayStream + '_ { let mut cursor = Cursor::new(&self.read); - cursor.set_position(u64::try_from(&scalar_at(&self.byte_offsets, 0).unwrap()).unwrap()); + let byte_offset = scalar_at(&self.byte_offsets, 0) + .and_then(|s| u64::try_from(&s)) + .vortex_expect("Failed to convert byte_offset to u64"); + + cursor.set_position(byte_offset); StreamArrayReader::try_new(cursor, self.context.clone()) .await - .unwrap() + .vortex_expect("Failed to create stream array reader") .with_dtype(self.dtype.clone()) .into_array_stream() } diff --git a/vortex-serde/src/chunked_reader/take_rows.rs b/vortex-serde/src/chunked_reader/take_rows.rs index db911eba61..90f00eff85 100644 --- a/vortex-serde/src/chunked_reader/take_rows.rs +++ b/vortex-serde/src/chunked_reader/take_rows.rs @@ -11,7 +11,7 @@ use vortex::stats::ArrayStatistics; use vortex::stream::{ArrayStream, ArrayStreamExt}; use vortex::{Array, ArrayDType, IntoArray, IntoArrayVariant}; use vortex_dtype::PType; -use vortex_error::{vortex_bail, VortexResult}; +use vortex_error::{vortex_bail, vortex_err, VortexResult}; use vortex_scalar::Scalar; use crate::chunked_reader::ChunkedArrayReader; @@ -54,24 +54,30 @@ impl ChunkedArrayReader { // Coalesce the chunks that we're going to read from. let coalesced_chunks = self.coalesce_chunks(chunk_idxs.as_ref()); + let mut start_chunks: Vec = Vec::with_capacity(coalesced_chunks.len()); + let mut stop_chunks: Vec = Vec::with_capacity(coalesced_chunks.len()); + for (i, chunks) in coalesced_chunks.iter().enumerate() { + start_chunks.push( + chunks + .first() + .ok_or_else(|| vortex_err!("Coalesced chunk {i} cannot be empty"))? + .chunk_idx, + ); + stop_chunks.push( + chunks + .last() + .ok_or_else(|| vortex_err!("Coalesced chunk {i} cannot be empty"))? + .chunk_idx + + 1, + ); + } + // Grab the row and byte offsets for each chunk range. - let start_chunks = PrimitiveArray::from( - coalesced_chunks - .iter() - .map(|chunks| chunks[0].chunk_idx) - .collect_vec(), - ) - .into_array(); + let start_chunks = PrimitiveArray::from(start_chunks).into_array(); let start_rows = take(&self.row_offsets, &start_chunks)?.into_primitive()?; let start_bytes = take(&self.byte_offsets, &start_chunks)?.into_primitive()?; - let stop_chunks = PrimitiveArray::from( - coalesced_chunks - .iter() - .map(|chunks| chunks.last().unwrap().chunk_idx + 1) - .collect_vec(), - ) - .into_array(); + let stop_chunks = PrimitiveArray::from(stop_chunks).into_array(); let stop_rows = take(&self.row_offsets, &stop_chunks)?.into_primitive()?; let stop_bytes = take(&self.byte_offsets, &stop_chunks)?.into_primitive()?; @@ -106,7 +112,6 @@ impl ChunkedArrayReader { let _hint = self.read.performance_hint(); chunk_idxs .iter() - .cloned() .map(|chunk_idx| vec![chunk_idx.clone()]) .collect_vec() } @@ -186,7 +191,7 @@ fn find_chunks(row_offsets: &Array, indices: &Array) -> VortexResult u64 { - self.object_store.head(&self.location).await.unwrap().size as u64 + self.object_store + .head(&self.location) + .await + .map_err(VortexError::ObjectStore) + .unwrap_or_else(|err| { + vortex_panic!( + err, + "Failed to get size of object at location {}", + self.location + ) + }) + .size as u64 } } diff --git a/vortex-serde/src/io/tokio.rs b/vortex-serde/src/io/tokio.rs index 6a2b1d6248..065eb84f73 100644 --- a/vortex-serde/src/io/tokio.rs +++ b/vortex-serde/src/io/tokio.rs @@ -7,6 +7,7 @@ use bytes::BytesMut; use tokio::fs::File; use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt}; use vortex_buffer::io_buf::IoBuf; +use vortex_error::{VortexError, VortexUnwrap as _}; use crate::io::{VortexRead, VortexReadAt, VortexWrite}; @@ -49,7 +50,11 @@ impl VortexReadAt for File { } async fn size(&self) -> u64 { - self.metadata().await.unwrap().len() + self.metadata() + .await + .map_err(|err| VortexError::IOError(err).with_context("Failed to get file metadata")) + .vortex_unwrap() + .len() } } diff --git a/vortex-serde/src/layouts/read/batch.rs b/vortex-serde/src/layouts/read/batch.rs index 21bdf0f93f..c48b0c2ff9 100644 --- a/vortex-serde/src/layouts/read/batch.rs +++ b/vortex-serde/src/layouts/read/batch.rs @@ -41,7 +41,7 @@ impl BatchReader { }, None => { debug_assert!( - self.arrays.iter().all(|a| a.is_none()), + self.arrays.iter().all(Option::is_none), "Expected layout to produce an array but it was empty" ); return Ok(None); diff --git a/vortex-serde/src/layouts/read/buffered.rs b/vortex-serde/src/layouts/read/buffered.rs index 2f72ec74cd..61e342ca33 100644 --- a/vortex-serde/src/layouts/read/buffered.rs +++ b/vortex-serde/src/layouts/read/buffered.rs @@ -28,7 +28,7 @@ impl BufferedReader { } fn buffered_row_count(&self) -> usize { - self.arrays.iter().map(|arr| arr.len()).sum() + self.arrays.iter().map(Array::len).sum() } fn buffer(&mut self) -> VortexResult> { diff --git a/vortex-serde/src/layouts/read/cache.rs b/vortex-serde/src/layouts/read/cache.rs index c3dca0fbaf..9b93ade449 100644 --- a/vortex-serde/src/layouts/read/cache.rs +++ b/vortex-serde/src/layouts/read/cache.rs @@ -3,6 +3,7 @@ use std::sync::{Arc, RwLock}; use ahash::HashMap; use bytes::Bytes; use vortex_dtype::DType; +use vortex_error::vortex_panic; use crate::layouts::read::{LayoutPartId, MessageId}; @@ -54,10 +55,11 @@ impl RelativeLayoutCache { pub fn get(&self, path: &[LayoutPartId]) -> Option { self.root .read() - .unwrap_or_else(|err| { - panic!( + .unwrap_or_else(|poison| { + vortex_panic!( "Failed to read from layout cache at path {:?} with error {}", - path, err + path, + poison ); }) .get(&self.absolute_id(path)) @@ -66,10 +68,11 @@ impl RelativeLayoutCache { pub fn remove(&mut self, path: &[LayoutPartId]) -> Option { self.root .write() - .unwrap_or_else(|err| { - panic!( + .unwrap_or_else(|poison| { + vortex_panic!( "Failed to write to layout cache at path {:?} with error {}", - path, err + path, + poison ) }) .remove(&self.absolute_id(path)) diff --git a/vortex-serde/src/layouts/read/layouts.rs b/vortex-serde/src/layouts/read/layouts.rs index 30d04be095..0e3d556c68 100644 --- a/vortex-serde/src/layouts/read/layouts.rs +++ b/vortex-serde/src/layouts/read/layouts.rs @@ -6,7 +6,7 @@ use flatbuffers::{ForwardsUOffset, Vector}; use vortex::Context; use vortex_dtype::field::Field; use vortex_dtype::DType; -use vortex_error::{vortex_bail, vortex_err, VortexResult}; +use vortex_error::{vortex_bail, vortex_err, VortexExpect as _, VortexResult}; use vortex_flatbuffers::footer as fb; use super::projections::Projection; @@ -150,7 +150,9 @@ impl ColumnLayout { let tab = flatbuffers::Table::new(&self.fb_bytes, self.fb_loc); fb::Layout::init_from_table(tab) }; - fb_layout.layout_as_nested_layout().expect("must be nested") + fb_layout + .layout_as_nested_layout() + .vortex_expect("ColumnLayout: Failed to read nested layout from flatbuffer") } fn read_child( @@ -288,7 +290,9 @@ impl ChunkedLayout { let tab = flatbuffers::Table::new(&self.fb_bytes, self.fb_loc); fb::Layout::init_from_table(tab) }; - fb_layout.layout_as_nested_layout().expect("must be nested") + fb_layout + .layout_as_nested_layout() + .vortex_expect("ChunkedLayout: Failed to read nested layout from flatbuffer") } } diff --git a/vortex-serde/src/layouts/read/stream.rs b/vortex-serde/src/layouts/read/stream.rs index 02f7f053c5..9b15395470 100644 --- a/vortex-serde/src/layouts/read/stream.rs +++ b/vortex-serde/src/layouts/read/stream.rs @@ -13,7 +13,7 @@ use vortex::compute::{filter, search_sorted, slice, take, SearchSortedSide}; use vortex::validity::Validity; use vortex::{Array, IntoArray, IntoArrayVariant}; use vortex_dtype::{match_each_integer_ptype, DType}; -use vortex_error::{vortex_err, VortexError, VortexResult}; +use vortex_error::{vortex_err, vortex_panic, VortexError, VortexResult}; use vortex_scalar::Scalar; use crate::io::VortexReadAt; @@ -103,8 +103,8 @@ impl Stream for LayoutBatchStream { if let Some(read) = self.layout.read()? { match read { ReadResult::GetMsgs(messages) => { - let reader = - mem::take(&mut self.reader).expect("Invalid state transition"); + let reader = mem::take(&mut self.reader) + .ok_or_else(|| vortex_err!("Invalid state transition"))?; let read_future = read_ranges(reader, messages).boxed(); self.state = StreamingState::Reading(read_future); } @@ -138,7 +138,10 @@ impl Stream for LayoutBatchStream { } StreamingState::Reading(f) => match ready!(f.poll_unpin(cx)) { Ok((read, buffers)) => { - let mut write_cache = self.messages_cache.write().unwrap(); + let mut write_cache = + self.messages_cache.write().unwrap_or_else(|poison| { + vortex_panic!("Failed to write to message cache: {poison}") + }); for (id, buf) in buffers { write_cache.set(id, buf) } diff --git a/vortex-serde/src/layouts/tests.rs b/vortex-serde/src/layouts/tests.rs index 9122da488f..b80ed68a9f 100644 --- a/vortex-serde/src/layouts/tests.rs +++ b/vortex-serde/src/layouts/tests.rs @@ -1,3 +1,5 @@ +#![allow(clippy::panic)] + use futures::StreamExt; use vortex::array::{ChunkedArray, PrimitiveArray, StructArray, VarBinArray}; use vortex::{ArrayDType, IntoArray, IntoArrayVariant}; diff --git a/vortex-serde/src/layouts/write/writer.rs b/vortex-serde/src/layouts/write/writer.rs index eef6afabbf..1b5d2d7561 100644 --- a/vortex-serde/src/layouts/write/writer.rs +++ b/vortex-serde/src/layouts/write/writer.rs @@ -7,7 +7,7 @@ use vortex::stream::ArrayStream; use vortex::validity::Validity; use vortex::{Array, ArrayDType, IntoArray}; use vortex_dtype::DType; -use vortex_error::{vortex_bail, VortexResult}; +use vortex_error::{vortex_bail, vortex_err, VortexResult}; use crate::io::VortexWrite; use crate::layouts::read::{ChunkedLayoutSpec, ColumnLayoutSpec}; @@ -94,7 +94,7 @@ impl LayoutWriter { row_offsets .last() .map(|off| off + chunk.len() as u64) - .expect("Row offsets should be initialized with a value"), + .ok_or_else(|| vortex_err!("Row offsets should be initialized with a value"))?, ); self.msgs.write_batch(chunk).await?; byte_offsets.push(self.msgs.tell()); @@ -154,7 +154,12 @@ impl LayoutWriter { async fn write_footer(&mut self, footer: Footer) -> VortexResult<(u64, u64)> { let dtype_offset = self.msgs.tell(); self.msgs - .write_dtype(&self.dtype.take().expect("Needed a schema at this point")) + .write_dtype( + &self + .dtype + .take() + .ok_or_else(|| vortex_err!("Schema should be written by now"))?, + ) .await?; let footer_offset = self.msgs.tell(); self.msgs.write_message(footer).await?; diff --git a/vortex-serde/src/message_reader.rs b/vortex-serde/src/message_reader.rs index bff55f431d..4f088eecc5 100644 --- a/vortex-serde/src/message_reader.rs +++ b/vortex-serde/src/message_reader.rs @@ -95,7 +95,9 @@ impl MessageReader { let buf = self.next().await?; let msg = unsafe { root_unchecked::(&buf) } .header_as_schema() - .expect("Checked earlier in the function"); + .ok_or_else(|| { + vortex_err!("Expected schema message; this was checked earlier in the function") + })?; Ok(IPCDType::read_flatbuffer(&msg)?.0) } @@ -276,7 +278,7 @@ impl ArrayBufferReader { .zip( ipc_buffers .iter() - .map(|b| b.offset()) + .map(vortex_flatbuffers::message::Buffer::offset) .skip(1) .chain([all_buffers_size]), ) diff --git a/vortex-serde/src/messages.rs b/vortex-serde/src/messages.rs index ff3d3415be..1913f0c91d 100644 --- a/vortex-serde/src/messages.rs +++ b/vortex-serde/src/messages.rs @@ -4,7 +4,7 @@ use vortex::stats::ArrayStatistics; use vortex::{flatbuffers as fba, Array}; use vortex_buffer::Buffer; use vortex_dtype::DType; -use vortex_error::{vortex_err, VortexError}; +use vortex_error::{vortex_err, VortexError, VortexExpect as _}; use vortex_flatbuffers::message::Compression; use vortex_flatbuffers::{message as fb, FlatBufferRoot, ReadFlatBuffer, WriteFlatBuffer}; @@ -135,10 +135,15 @@ impl<'a> WriteFlatBuffer for IPCArray<'a> { .metadata() .try_serialize_metadata() // TODO(ngates): should we serialize externally to here? - .unwrap(); + .vortex_expect("ArrayView is missing metadata during serialization"); Some(fbb.create_vector(metadata.as_ref())) } - Array::View(v) => Some(fbb.create_vector(v.metadata().unwrap())), + Array::View(v) => Some( + fbb.create_vector( + v.metadata() + .vortex_expect("ArrayView is missing metadata during serialization"), + ), + ), }; let children = column_data diff --git a/vortex-serde/src/stream_reader/mod.rs b/vortex-serde/src/stream_reader/mod.rs index f8568eebd6..622d21fc02 100644 --- a/vortex-serde/src/stream_reader/mod.rs +++ b/vortex-serde/src/stream_reader/mod.rs @@ -7,7 +7,7 @@ use vortex::stream::ArrayStream; use vortex::Context; use vortex_buffer::Buffer; use vortex_dtype::DType; -use vortex_error::VortexResult; +use vortex_error::{VortexExpect as _, VortexResult}; use crate::io::VortexRead; use crate::MessageReader; @@ -41,12 +41,22 @@ impl StreamArrayReader { /// Reads a single array from the stream. pub fn array_stream(&mut self) -> impl ArrayStream + '_ { - let dtype = self.dtype.as_ref().expect("DType not set").deref().clone(); + let dtype = self + .dtype + .as_ref() + .vortex_expect("Cannot read array from stream: DType not set") + .deref() + .clone(); self.msgs.array_stream(self.ctx.clone(), dtype) } pub fn into_array_stream(self) -> impl ArrayStream { - let dtype = self.dtype.as_ref().expect("DType not set").deref().clone(); + let dtype = self + .dtype + .as_ref() + .vortex_expect("Cannot read array from stream: DType not set") + .deref() + .clone(); self.msgs.into_array_stream(self.ctx.clone(), dtype) } diff --git a/vortex-serde/src/stream_writer/tests.rs b/vortex-serde/src/stream_writer/tests.rs index a7632e5702..f23fc74345 100644 --- a/vortex-serde/src/stream_writer/tests.rs +++ b/vortex-serde/src/stream_writer/tests.rs @@ -1,7 +1,7 @@ use std::io::Cursor; use std::sync::Arc; -use arrow_array::cast::AsArray; +use arrow_array::cast::AsArray as _; use arrow_array::types::Int32Type; use arrow_array::PrimitiveArray; use vortex::arrow::FromArrowArray; @@ -31,6 +31,6 @@ async fn broken_data() { .collect_chunked() .await .unwrap(); - let round_tripped = arr.into_canonical().unwrap().into_arrow(); + let round_tripped = arr.into_canonical().unwrap().into_arrow().unwrap(); assert_eq!(&arrow_arr, round_tripped.as_primitive::()); }