diff --git a/STYLE.md b/STYLE.md index b65a1de164f..a6ba99171ae 100644 --- a/STYLE.md +++ b/STYLE.md @@ -55,7 +55,8 @@ - `vortex_ensure!` and `vortex_ensure_eq!` for checking conditions. Prefer `vortex_ensure_eq!(a, b)` with no message: the error already shows both expressions and values. Add a message only for context the expressions lack, and never repeat the values in it. -- Add context to errors using `.with_context()` +- Add context to an existing error with `.with_context()`, which keeps its source and backtrace; + do not format it into a new `vortex_err!`, which discards them. - Include backtraces for better debugging - Use `VortexExpect` trait when unwrapping is appropriate with proper error context. diff --git a/encodings/alp/src/alp/array.rs b/encodings/alp/src/alp/array.rs index 5e667eba8b0..8cc368830ae 100644 --- a/encodings/alp/src/alp/array.rs +++ b/encodings/alp/src/alp/array.rs @@ -144,7 +144,7 @@ impl VTable for ALP { let encoded_ptype = match &dtype { DType::Primitive(PType::F32, n) => DType::Primitive(PType::I32, *n), DType::Primitive(PType::F64, n) => DType::Primitive(PType::I64, *n), - d => vortex_bail!(MismatchedTypes: "f32 or f64", d), + d => vortex_bail!("expected type: f32 or f64 but instead got {}", d), }; let encoded = children.get(0, &encoded_ptype, len)?; @@ -319,7 +319,9 @@ impl ALPData { DType::Primitive(PType::I64, nullability) => { Ok(DType::Primitive(PType::F64, *nullability)) } - _ => vortex_bail!("ALP encoded ints have invalid DType {}", encoded.dtype(),), + _ => { + vortex_bail!("ALP encoded ints have invalid DType {}", encoded.dtype(),) + } } } diff --git a/encodings/datetime-parts/src/array.rs b/encodings/datetime-parts/src/array.rs index 8fd621bdb0d..8511073ba9d 100644 --- a/encodings/datetime-parts/src/array.rs +++ b/encodings/datetime-parts/src/array.rs @@ -323,10 +323,16 @@ impl DateTimePartsData { ); } if !seconds.dtype().is_int() || seconds.dtype().is_nullable() { - vortex_bail!(MismatchedTypes: "non-nullable integer", seconds.dtype()); + vortex_bail!( + "expected type: non-nullable integer but instead got {}", + seconds.dtype() + ); } if !subseconds.dtype().is_int() || subseconds.dtype().is_nullable() { - vortex_bail!(MismatchedTypes: "non-nullable integer", subseconds.dtype()); + vortex_bail!( + "expected type: non-nullable integer but instead got {}", + subseconds.dtype() + ); } if len != seconds.len() || len != subseconds.len() { diff --git a/encodings/datetime-parts/src/canonical.rs b/encodings/datetime-parts/src/canonical.rs index 0b375d84b2d..b613bd229cd 100644 --- a/encodings/datetime-parts/src/canonical.rs +++ b/encodings/datetime-parts/src/canonical.rs @@ -41,11 +41,11 @@ pub fn decode_to_temporal( ctx: &mut ExecutionCtx, ) -> VortexResult { let DType::Extension(ext) = dtype else { - vortex_panic!(Compute: "expected dtype to be DType::Extension variant") + vortex_panic!("expected dtype to be DType::Extension variant") }; let Some(options) = ext.metadata_opt::() else { - vortex_panic!(Compute: "must decode TemporalMetadata from extension metadata"); + vortex_panic!("must decode TemporalMetadata from extension metadata"); }; let divisor = match options.unit { @@ -53,7 +53,7 @@ pub fn decode_to_temporal( TimeUnit::Microseconds => 1_000_000, TimeUnit::Milliseconds => 1_000, TimeUnit::Seconds => 1, - TimeUnit::Days => vortex_panic!(InvalidArgument: "cannot decode into TimeUnit::D"), + TimeUnit::Days => vortex_panic!("cannot decode into TimeUnit::D"), }; let days = parts.days.as_::(); diff --git a/encodings/datetime-parts/src/ops.rs b/encodings/datetime-parts/src/ops.rs index 4fc0c0e3638..b75b4378b27 100644 --- a/encodings/datetime-parts/src/ops.rs +++ b/encodings/datetime-parts/src/ops.rs @@ -35,7 +35,7 @@ impl OperationsVTable for DateTimeParts { }; let Some(options) = ext.metadata_opt::() else { - vortex_panic!(Compute: "must decode TemporalMetadata from extension metadata"); + vortex_panic!("must decode TemporalMetadata from extension metadata"); }; if !state.is_valid(index, ctx)? { diff --git a/encodings/decimal-byte-parts/src/decimal_byte_parts/assemble.rs b/encodings/decimal-byte-parts/src/decimal_byte_parts/assemble.rs index df222241964..1cdf27c15bd 100644 --- a/encodings/decimal-byte-parts/src/decimal_byte_parts/assemble.rs +++ b/encodings/decimal-byte-parts/src/decimal_byte_parts/assemble.rs @@ -154,7 +154,9 @@ fn assemble_wide_decimal_from_arrays( validity, ) .into_array(), - _ => vortex_bail!("expected between one and {MAX_LOWER_PARTS} lower parts"), + _ => { + vortex_bail!("expected between one and {MAX_LOWER_PARTS} lower parts") + } } })) } diff --git a/encodings/fastlanes/src/bitpacking/array/bitpack_compress/blocked.rs b/encodings/fastlanes/src/bitpacking/array/bitpack_compress/blocked.rs index 4fbe91fedb9..7aa088f3bac 100644 --- a/encodings/fastlanes/src/bitpacking/array/bitpack_compress/blocked.rs +++ b/encodings/fastlanes/src/bitpacking/array/bitpack_compress/blocked.rs @@ -94,7 +94,7 @@ pub fn bitpack_encode_blocked( let num_blocks = array.len().div_ceil(FL_CHUNK_SIZE); vortex_ensure!( bit_widths.len() == num_blocks, - InvalidArgument: "Expected {num_blocks} bit widths, got {}", + "Expected {num_blocks} bit widths, got {}", bit_widths.len() ); validate_bit_widths(bit_widths, array.ptype())?; @@ -133,13 +133,13 @@ fn validate_bit_widths(bit_widths: &[u8], ptype: PType) -> VortexResult<()> { let bit_width = usize::from(bit_width); vortex_ensure!( bit_width <= max_bit_width, - InvalidArgument: "Bit widths must be at most {max_bit_width} for {ptype}" + "Bit widths must be at most {max_bit_width} for {ptype}" ); has_packed_block |= bit_width < max_bit_width; } vortex_ensure!( bit_widths.is_empty() || has_packed_block, - InvalidArgument: "Cannot pack: all blocks use the native bit width {max_bit_width}" + "Cannot pack: all blocks use the native bit width {max_bit_width}" ); Ok(()) } @@ -343,7 +343,6 @@ mod tests { use vortex_array::assert_arrays_eq; use vortex_array::validity::Validity; use vortex_buffer::Buffer; - use vortex_error::VortexError; use vortex_error::vortex_bail; use vortex_error::vortex_err; use vortex_session::VortexSession; @@ -542,33 +541,43 @@ mod tests { } #[rstest] - #[case::negative(PrimitiveArray::from_iter(-5i64..5))] - #[case::float(PrimitiveArray::from_iter([1.0f32, 2.0]))] - fn encode_blocked_rejects_invalid_values(#[case] array: PrimitiveArray) { + #[case::negative( + PrimitiveArray::from_iter(-5i64..5), + "cannot bitpack array containing negative integers" + )] + #[case::float(PrimitiveArray::from_iter([1.0f32, 2.0]), "cannot bitpack f32 array")] + fn encode_blocked_rejects_invalid_values( + #[case] array: PrimitiveArray, + #[case] expected: &str, + ) { let mut ctx = SESSION.create_execution_ctx(); - assert!(matches!( - bitpack_blocked_to_best_bit_widths(&array, &mut ctx).unwrap_err(), - VortexError::InvalidArgument(_, _) - )); - assert!(matches!( - BitPackedData::encode_blocked(&array.into_array(), &[1], &mut ctx).unwrap_err(), - VortexError::InvalidArgument(_, _) - )); + let err = bitpack_blocked_to_best_bit_widths(&array, &mut ctx) + .unwrap_err() + .to_string(); + assert!(err.contains(expected), "{err}"); + let err = BitPackedData::encode_blocked(&array.into_array(), &[1], &mut ctx) + .unwrap_err() + .to_string(); + assert!(err.contains(expected), "{err}"); } #[rstest] - #[case::too_few(vec![3])] - #[case::too_many(vec![3, 3, 3])] - #[case::wider_than_type(vec![3, 33])] - fn encode_blocked_rejects_invalid_bit_widths(#[case] bit_widths: Vec) { + #[case::too_few(vec![3], "Expected 2 bit widths, got 1")] + #[case::too_many(vec![3, 3, 3], "Expected 2 bit widths, got 3")] + #[case::wider_than_type(vec![3, 33], "Bit widths must be at most 32 for u32")] + fn encode_blocked_rejects_invalid_bit_widths( + #[case] bit_widths: Vec, + #[case] expected: &str, + ) { let array = PrimitiveArray::from_iter((0..2048u32).map(|i| i % 8)); let err = BitPackedData::encode_blocked( &array.into_array(), &bit_widths, &mut SESSION.create_execution_ctx(), ) - .unwrap_err(); - assert!(matches!(err, VortexError::InvalidArgument(_, _))); + .unwrap_err() + .to_string(); + assert!(err.contains(expected), "{err}"); } #[test] diff --git a/encodings/fastlanes/src/bitpacking/array/bitpack_compress/global.rs b/encodings/fastlanes/src/bitpacking/array/bitpack_compress/global.rs index 53f16a0e06a..a20a0d027de 100644 --- a/encodings/fastlanes/src/bitpacking/array/bitpack_compress/global.rs +++ b/encodings/fastlanes/src/bitpacking/array/bitpack_compress/global.rs @@ -58,7 +58,7 @@ pub fn bitpack_encode( if bit_width >= array.ptype().bit_width() as u8 { // Nothing we can do vortex_bail!( - InvalidArgument: "Cannot pack - specified bit width {bit_width} >= {}", + "Cannot pack - specified bit width {bit_width} >= {}", array.ptype().bit_width() ) } @@ -434,7 +434,6 @@ mod test { use vortex_array::builders::ArrayBuilder; use vortex_array::builders::PrimitiveBuilder; use vortex_buffer::Buffer; - use vortex_error::VortexError; use vortex_error::vortex_err; use vortex_session::VortexSession; @@ -493,8 +492,12 @@ mod test { assert!(array.ptype().is_signed_int()); let err = BitPackedData::encode(&array.into_array(), 1024u32.ilog2() as u8, &mut ctx) - .unwrap_err(); - assert!(matches!(err, VortexError::InvalidArgument(_, _))); + .unwrap_err() + .to_string(); + assert!( + err.contains("cannot bitpack array containing negative integers"), + "{err}" + ); } #[test] diff --git a/encodings/fastlanes/src/bitpacking/array/bitpack_compress/mod.rs b/encodings/fastlanes/src/bitpacking/array/bitpack_compress/mod.rs index b60bf1e6513..70a3cde910b 100644 --- a/encodings/fastlanes/src/bitpacking/array/bitpack_compress/mod.rs +++ b/encodings/fastlanes/src/bitpacking/array/bitpack_compress/mod.rs @@ -33,7 +33,7 @@ fn ensure_non_negative_integers( ) -> VortexResult<()> { vortex_ensure!( array.ptype().is_int(), - InvalidArgument: "cannot bitpack {} array", + "cannot bitpack {} array", array.ptype() ); if array.ptype().is_signed_int() { @@ -41,7 +41,7 @@ fn ensure_non_negative_integers( array.statistics().compute_min::

(ctx).unwrap_or_default() < 0 }); if has_negative_values { - vortex_bail!(InvalidArgument: "cannot bitpack array containing negative integers") + vortex_bail!("cannot bitpack array containing negative integers") } } Ok(()) diff --git a/encodings/fastlanes/src/bitpacking/array/mod.rs b/encodings/fastlanes/src/bitpacking/array/mod.rs index 323bee8c7d8..685ff42298e 100644 --- a/encodings/fastlanes/src/bitpacking/array/mod.rs +++ b/encodings/fastlanes/src/bitpacking/array/mod.rs @@ -330,7 +330,11 @@ impl BitPackedData { length: usize, offset: u16, ) -> VortexResult<()> { - vortex_ensure!(ptype.is_int(), MismatchedTypes: "integer", ptype); + vortex_ensure!( + ptype.is_int(), + "expected type: integer but instead got {}", + ptype + ); if let Some(validity_len) = validity.maybe_len() { vortex_ensure_eq!( @@ -437,10 +441,12 @@ impl BitPackedData { bit_width: u8, ctx: &mut ExecutionCtx, ) -> VortexResult { - let parray: PrimitiveArray = array - .clone() - .try_downcast::() - .map_err(|a| vortex_err!(InvalidArgument: "Bitpacking can only encode primitive arrays, got {}", a.encoding_id()))?; + let parray: PrimitiveArray = array.clone().try_downcast::().map_err(|a| { + vortex_err!( + "Bitpacking can only encode primitive arrays, got {}", + a.encoding_id() + ) + })?; bitpack_encode(&parray, bit_width, None, ctx) } @@ -460,10 +466,12 @@ impl BitPackedData { bit_widths: &[u8], ctx: &mut ExecutionCtx, ) -> VortexResult { - let parray: PrimitiveArray = array - .clone() - .try_downcast::() - .map_err(|a| vortex_err!(InvalidArgument: "Bitpacking can only encode primitive arrays, got {}", a.encoding_id()))?; + let parray: PrimitiveArray = array.clone().try_downcast::().map_err(|a| { + vortex_err!( + "Bitpacking can only encode primitive arrays, got {}", + a.encoding_id() + ) + })?; bitpack_encode_blocked(&parray, bit_widths, None, ctx) } } diff --git a/encodings/fastlanes/src/for_/array/mod.rs b/encodings/fastlanes/src/for_/array/mod.rs index 4d99943ae9a..d7edb2de513 100644 --- a/encodings/fastlanes/src/for_/array/mod.rs +++ b/encodings/fastlanes/src/for_/array/mod.rs @@ -67,7 +67,9 @@ pub trait FoRArrayExt: FoRArraySlotsExt { match (&self.global_reference, self.blocked_references()) { (Some(reference), None) => FoRReferences::Global(reference), (None, Some(references)) => FoRReferences::Blocked(references), - _ => vortex_panic!("FoR array must have either a global or blocked references"), + _ => { + vortex_panic!("FoR array must have either a global or blocked references") + } } } diff --git a/encodings/fsst/src/array.rs b/encodings/fsst/src/array.rs index c8cb108adf3..ae8d909b052 100644 --- a/encodings/fsst/src/array.rs +++ b/encodings/fsst/src/array.rs @@ -510,13 +510,13 @@ impl FSSTSymbolTable { vortex_ensure!( padded_symbols.len() == FSST_SYMBOL_TABLE_LEN && padded_symbol_lengths.len() == FSST_SYMBOL_TABLE_LEN, - InvalidArgument: "padded symbol table must have exactly {FSST_SYMBOL_TABLE_LEN} entries, found {} symbols and {} symbol lengths", + "padded symbol table must have exactly {FSST_SYMBOL_TABLE_LEN} entries, found {} symbols and {} symbol lengths", padded_symbols.len(), padded_symbol_lengths.len() ); vortex_ensure!( n_symbols <= FSST_SYMBOL_TABLE_LEN, - InvalidArgument: "n_symbols must be <= {FSST_SYMBOL_TABLE_LEN}, found {n_symbols}" + "n_symbols must be <= {FSST_SYMBOL_TABLE_LEN}, found {n_symbols}" ); Ok(Self { padded_symbols, @@ -666,7 +666,7 @@ impl FSST { ctx: &mut ExecutionCtx, ) -> VortexResult> { if children.len() != 2 { - vortex_bail!(InvalidArgument: "Expected 2 children, got {}", children.len()); + vortex_bail!("Expected 2 children, got {}", children.len()); } let codes = children.get(0, &DType::Binary(dtype.nullability()), len)?; let codes: VarBinArray = codes @@ -763,11 +763,11 @@ impl FSSTData { vortex_ensure_eq!( symbols.len(), symbol_lengths.len(), - InvalidArgument: "symbols and symbol_lengths arrays must have same length" + "symbols and symbol_lengths arrays must have same length" ); vortex_ensure!( symbols.len() <= FSST_SYMBOL_TABLE_LEN, - InvalidArgument: "symbols array must have length <= {FSST_SYMBOL_TABLE_LEN}, found {}", + "symbols array must have length <= {FSST_SYMBOL_TABLE_LEN}, found {}", symbols.len() ); let n_symbols = symbols.len(); @@ -820,11 +820,11 @@ impl FSSTData { ); if symbols.len() > FSST_SYMBOL_TABLE_LEN { - vortex_bail!(InvalidArgument: "symbols array must have length <= {FSST_SYMBOL_TABLE_LEN}"); + vortex_bail!("symbols array must have length <= {FSST_SYMBOL_TABLE_LEN}"); } if symbols.len() != symbol_lengths.len() { - vortex_bail!(InvalidArgument: "symbols and symbol_lengths arrays must have same length"); + vortex_bail!("symbols and symbol_lengths arrays must have same length"); } Self::validate_symbol_lengths(symbol_lengths)?; @@ -832,24 +832,30 @@ impl FSSTData { // codes_offsets.len() - 1 == number of elements let codes_len = codes_offsets.len().saturating_sub(1); if codes_len != len { - vortex_bail!(InvalidArgument: "codes must have same len as outer array"); + vortex_bail!("codes must have same len as outer array"); } if uncompressed_lengths.len() != len { - vortex_bail!(InvalidArgument: "uncompressed_lengths must be same len as codes"); + vortex_bail!("uncompressed_lengths must be same len as codes"); } if !uncompressed_lengths.dtype().is_int() || uncompressed_lengths.dtype().is_nullable() { - vortex_bail!(InvalidArgument: "uncompressed_lengths must have integer type and cannot be nullable, found {}", uncompressed_lengths.dtype()); + vortex_bail!( + "uncompressed_lengths must have integer type and cannot be nullable, found {}", + uncompressed_lengths.dtype() + ); } // Offsets must be non-nullable integer. if !codes_offsets.dtype().is_int() || codes_offsets.dtype().is_nullable() { - vortex_bail!(InvalidArgument: "codes offsets must be non-nullable integer type, found {}", codes_offsets.dtype()); + vortex_bail!( + "codes offsets must be non-nullable integer type, found {}", + codes_offsets.dtype() + ); } if codes_nullability != dtype.nullability() { - vortex_bail!(InvalidArgument: "codes nullability must match outer dtype nullability"); + vortex_bail!("codes nullability must match outer dtype nullability"); } Ok(()) @@ -869,7 +875,7 @@ impl FSSTData { (&codes_offsets.execute_scalar(codes_offsets.len() - 1, ctx)?).try_into()?; vortex_ensure!( last_offset <= codes_bytes.len(), - InvalidArgument: "Last codes offset {} exceeds codes bytes length {}", + "Last codes offset {} exceeds codes bytes length {}", last_offset, codes_bytes.len() ); @@ -882,12 +888,14 @@ impl FSSTData { let mut expected = 2; for (idx, &len) in symbol_lengths.iter().enumerate() { if len > 8 || len == 0 { - vortex_bail!(InvalidArgument: "symbol length at index {idx} must be between 1 and 8, found {len}"); + vortex_bail!("symbol length at index {idx} must be between 1 and 8, found {len}"); } if expected == 1 { if len != 1 { - vortex_bail!(InvalidArgument: "symbol length at index {idx} must be 1 after one-byte symbols begin, found {len}"); + vortex_bail!( + "symbol length at index {idx} must be 1 after one-byte symbols begin, found {len}" + ); } } else { if len == 1 { @@ -895,7 +903,9 @@ impl FSSTData { } if len < expected { - vortex_bail!(InvalidArgument: "symbol length at index {idx} violates FSST symbol table ordering"); + vortex_bail!( + "symbol length at index {idx} violates FSST symbol table ordering" + ); } expected = len; } diff --git a/encodings/onpair/src/array.rs b/encodings/onpair/src/array.rs index d48287fac84..a521c1bc18c 100644 --- a/encodings/onpair/src/array.rs +++ b/encodings/onpair/src/array.rs @@ -233,7 +233,7 @@ fn build_dictionary( offsets: Buffer, ) -> VortexResult> { CompactDictionary::validate_safety(OnPairDictionaryStorage { bytes, offsets }) - .map_err(|e| vortex_err!(InvalidArgument: "Unsafe OnPair dictionary: {e}")) + .map_err(|e| vortex_err!("Unsafe OnPair dictionary: {e}")) } /// A safety-validated dictionary built from host-materialized parts, owned @@ -417,19 +417,19 @@ fn validate_parts( ); if !dict_offsets.dtype().is_int() || dict_offsets.dtype().is_nullable() { - vortex_bail!(InvalidArgument: "dict_offsets must be non-nullable integer"); + vortex_bail!("dict_offsets must be non-nullable integer"); } if !codes.dtype().is_int() || codes.dtype().is_nullable() { - vortex_bail!(InvalidArgument: "codes must be non-nullable integer"); + vortex_bail!("codes must be non-nullable integer"); } if !codes_offsets.dtype().is_int() || codes_offsets.dtype().is_nullable() { - vortex_bail!(InvalidArgument: "codes_offsets must be non-nullable integer"); + vortex_bail!("codes_offsets must be non-nullable integer"); } if !uncompressed_lengths.dtype().is_int() || uncompressed_lengths.dtype().is_nullable() { - vortex_bail!(InvalidArgument: "uncompressed_lengths must be non-nullable integer"); + vortex_bail!("uncompressed_lengths must be non-nullable integer"); } if codes_offsets.len() != uncompressed_lengths.len() + 1 { - vortex_bail!(InvalidArgument: + vortex_bail!( "codes_offsets.len ({}) != uncompressed_lengths.len + 1 ({})", codes_offsets.len(), uncompressed_lengths.len() + 1 @@ -464,7 +464,7 @@ impl VTable for OnPair { s.uncompressed_lengths, )?; if s.uncompressed_lengths.len() != len { - vortex_bail!(InvalidArgument: "uncompressed_lengths must have same len as outer array"); + vortex_bail!("uncompressed_lengths must have same len as outer array"); } Ok(()) } @@ -538,7 +538,7 @@ impl VTable for OnPair { _session: &VortexSession, ) -> VortexResult> { if buffers.len() != 1 { - vortex_bail!(InvalidArgument: "Expected 1 buffer, got {}", buffers.len()); + vortex_bail!("Expected 1 buffer, got {}", buffers.len()); } let metadata = OnPairMetadata::decode(metadata)?; let uncompressed_ptype = metadata.get_uncompressed_lengths_ptype()?; @@ -585,7 +585,7 @@ impl VTable for OnPair { let validity = match children.len() { 4 => Validity::from(dtype.nullability()), 5 => Validity::Array(children.get(4, &Validity::DTYPE, len)?), - other => vortex_bail!(InvalidArgument: "Expected 4 or 5 children, got {other}"), + other => vortex_bail!("Expected 4 or 5 children, got {other}"), }; let data = OnPairData::new(buffers[0].clone()); diff --git a/encodings/parquet-variant/src/arrow.rs b/encodings/parquet-variant/src/arrow.rs index 48d2c305c94..f6eaf1d1c79 100644 --- a/encodings/parquet-variant/src/arrow.rs +++ b/encodings/parquet-variant/src/arrow.rs @@ -81,7 +81,7 @@ pub(crate) fn export_storage_to_target( }; let Some(child) = child else { vortex_bail!( - InvalidArgument: "cannot export Parquet Variant storage field '{}' because source has no {} child", + "cannot export Parquet Variant storage field '{}' because source has no {} child", field.name(), field.name() ); @@ -211,13 +211,11 @@ impl ArrowExportVTable for ParquetVariant { )?)); } if has_value && !request_has_value { - vortex_bail!( - InvalidArgument: "cannot export Parquet Variant storage without losing value child" - ); + vortex_bail!("cannot export Parquet Variant storage without losing value child"); } if has_typed_value && !request_has_typed_value { vortex_bail!( - InvalidArgument: "cannot export Parquet Variant storage without losing typed_value child" + "cannot export Parquet Variant storage without losing typed_value child" ); } diff --git a/encodings/parquet-variant/src/vtable.rs b/encodings/parquet-variant/src/vtable.rs index 34703b17c45..70de3b1d582 100644 --- a/encodings/parquet-variant/src/vtable.rs +++ b/encodings/parquet-variant/src/vtable.rs @@ -407,12 +407,10 @@ mod tests { vortex_file::register_default_encodings(&session); session.arrays().register(ParquetVariant); let editions = session.editions(); - editions - .declare_edition(Edition { - id: TEST_EDITION, - min_library_version: None, - }) - .map_err(|error| vortex_err!("{error}"))?; + editions.declare_edition(Edition { + id: TEST_EDITION, + min_library_version: None, + })?; let component_ids = [ ( ComponentKind::Array, @@ -438,9 +436,7 @@ mod tests { ]; for (kind, ids) in component_ids { for id in ids { - editions - .declare_inclusion(EditionInclusion::new(kind, &id, TEST_EDITION)) - .map_err(|error| vortex_err!("{error}"))?; + editions.declare_inclusion(EditionInclusion::new(kind, &id, TEST_EDITION))?; } } for id in [ @@ -451,17 +447,13 @@ mod tests { "vortex.nan_count", "vortex.null_count", ] { - editions - .declare_inclusion(EditionInclusion::new( - ComponentKind::Aggregate, - id, - TEST_EDITION, - )) - .map_err(|error| vortex_err!("{error}"))?; + editions.declare_inclusion(EditionInclusion::new( + ComponentKind::Aggregate, + id, + TEST_EDITION, + ))?; } - session - .enable_edition(TEST_EDITION) - .map_err(|error| vortex_err!("{error}"))?; + session.enable_edition(TEST_EDITION)?; Ok(session) } diff --git a/encodings/pco/src/array.rs b/encodings/pco/src/array.rs index c1a4347cd94..684155fa875 100644 --- a/encodings/pco/src/array.rs +++ b/encodings/pco/src/array.rs @@ -315,7 +315,10 @@ pub(crate) fn vortex_err_from_pco(err: PcoError) -> VortexError { use pco::errors::ErrorKind::*; match err.kind { Io(io_kind) => VortexError::from(std::io::Error::new(io_kind, err.message)), - InvalidArgument => vortex_err!(InvalidArgument: "{}", err.message), + InvalidArgument => vortex_err!("{}", err.message), + kind @ (Corruption | InsufficientData) => { + vortex_err!("Pco {:?} error: {}", kind, err.message) + } other => vortex_err!("Pco {:?} error: {}", other, err.message), } } diff --git a/encodings/runend/src/compute/take.rs b/encodings/runend/src/compute/take.rs index 047a072e9ab..a50787ef7fd 100644 --- a/encodings/runend/src/compute/take.rs +++ b/encodings/runend/src/compute/take.rs @@ -293,7 +293,7 @@ fn prefer_linear_scan( fn check_index(index: usize, array_len: usize) -> VortexResult<()> { if index >= array_len { - vortex_bail!(OutOfBounds: index, 0, array_len); + vortex_bail!("index {} out of bounds from {} to {}", index, 0, array_len); } Ok(()) } diff --git a/encodings/sequence/src/array.rs b/encodings/sequence/src/array.rs index f6ead2c5230..81eacb0627f 100644 --- a/encodings/sequence/src/array.rs +++ b/encodings/sequence/src/array.rs @@ -186,7 +186,9 @@ impl SequenceData { { Kind::Int64Value(_) => Ok(PType::I64), Kind::Uint64Value(_) => Ok(PType::U64), - _ => vortex_bail!("only integer ptypes are supported in SequenceArray currently"), + _ => { + vortex_bail!("only integer ptypes are supported in SequenceArray currently") + } } } diff --git a/encodings/sequence/src/compute/take.rs b/encodings/sequence/src/compute/take.rs index daf23a713ad..5089f65d486 100644 --- a/encodings/sequence/src/compute/take.rs +++ b/encodings/sequence/src/compute/take.rs @@ -47,7 +47,12 @@ fn take_inner( let validity = match indices_mask.bit_buffer() { AllOr::All => { if let Err(position) = indices.try_map_into(out, value_at) { - vortex_panic!(OutOfBounds: indices[position].as_(), 0, len); + vortex_panic!( + "index {} out of bounds from {} to {}", + indices[position].as_(), + 0, + len + ); } Validity::from(result_nullability) } @@ -60,7 +65,12 @@ fn take_inner( } AllOr::Some(bits) => { if let Err(position) = indices.try_map_masked_into(bits, out, value_at) { - vortex_panic!(OutOfBounds: indices[position].as_(), 0, len); + vortex_panic!( + "index {} out of bounds from {} to {}", + indices[position].as_(), + 0, + len + ); } Validity::from(bits.clone()) } diff --git a/encodings/sparse/src/canonical.rs b/encodings/sparse/src/canonical.rs index 7430228e931..796fda56ef5 100644 --- a/encodings/sparse/src/canonical.rs +++ b/encodings/sparse/src/canonical.rs @@ -103,7 +103,9 @@ impl SparseStringParts { .cloned() .map(BufferString::into_inner), DType::Binary(_) => fill_scalar.as_binary().value().cloned(), - dtype => vortex_bail!("Sparse string decode of non-string dtype {dtype}"), + dtype => { + vortex_bail!("Sparse string decode of non-string dtype {dtype}") + } }; Ok(Self { len: array.len(), @@ -346,7 +348,9 @@ pub(super) fn execute_sparse(parts: SparseParts, ctx: &mut ExecutionCtx) -> Vort DType::FixedSizeList(.., nullability) => { execute_sparse_fixed_size_list(&patches, &fill_value, len, *nullability, ctx)? } - DType::Map(..) => vortex_bail!("Sparse canonicalization does not support Map arrays yet"), + DType::Map(..) => { + vortex_bail!("Sparse canonicalization does not support Map arrays yet") + } DType::Struct(struct_fields, ..) => execute_sparse_struct( struct_fields, fill_value.as_struct(), @@ -356,7 +360,9 @@ pub(super) fn execute_sparse(parts: SparseParts, ctx: &mut ExecutionCtx) -> Vort ctx, )?, DType::Union(..) => vortex_bail!("TODO(connor)[Union]: unimplemented"), - DType::Variant(_) => vortex_bail!("Sparse canonicalization does not support Variant"), + DType::Variant(_) => { + vortex_bail!("Sparse canonicalization does not support Variant") + } DType::Extension(..) => { vortex_bail!("Sparse canonicalization does not support Extension arrays yet") } diff --git a/encodings/uuid/src/vtable.rs b/encodings/uuid/src/vtable.rs index f937e8fd81d..03b7400347f 100644 --- a/encodings/uuid/src/vtable.rs +++ b/encodings/uuid/src/vtable.rs @@ -44,7 +44,9 @@ impl ExtVTable for Uuid { let version = match metadata.len() { 0 => None, 1 => Some(u8_to_version(metadata[0])?), - other => vortex_bail!("UUID metadata must be 0 or 1 bytes, got {other}"), + other => { + vortex_bail!("UUID metadata must be 0 or 1 bytes, got {other}") + } }; Ok(UuidMetadata { version }) diff --git a/encodings/zigzag/src/array.rs b/encodings/zigzag/src/array.rs index 25fed1c159e..101da38b521 100644 --- a/encodings/zigzag/src/array.rs +++ b/encodings/zigzag/src/array.rs @@ -218,7 +218,10 @@ impl ZigZagData { pub fn try_new(encoded_dtype: &DType) -> VortexResult { if !encoded_dtype.is_unsigned_int() { - vortex_bail!(MismatchedTypes: "unsigned int", encoded_dtype); + vortex_bail!( + "expected type: unsigned int but instead got {}", + encoded_dtype + ); } Self::dtype_from_encoded_dtype(encoded_dtype)?; diff --git a/encodings/zstd/src/lib.rs b/encodings/zstd/src/lib.rs index e50f3cb1e21..ea08ae07402 100644 --- a/encodings/zstd/src/lib.rs +++ b/encodings/zstd/src/lib.rs @@ -52,11 +52,9 @@ pub fn initialize(session: &VortexSession) { session .editions() .declare_family(&editions::FAMILY) - .map_err(|error| vortex_err!("{error}")) .vortex_expect("Zstd edition family is valid"); session .register_edition(&editions::DECLARATION) - .map_err(|error| vortex_err!("{error}")) .vortex_expect("Zstd edition declaration is valid"); } } diff --git a/encodings/zstd/src/zstd_buffers.rs b/encodings/zstd/src/zstd_buffers.rs index c7478e79c0b..7619ce9ec6e 100644 --- a/encodings/zstd/src/zstd_buffers.rs +++ b/encodings/zstd/src/zstd_buffers.rs @@ -492,7 +492,9 @@ impl VTable for ZstdBuffers { vortex_ensure_eq!(validity.dtype(), &Validity::DTYPE); vortex_ensure_eq!(validity.len(), len); } - None => vortex_ensure_eq!(extra_slots, 0, "ZstdBuffers has unexpected extra slots"), + None => { + vortex_ensure_eq!(extra_slots, 0, "ZstdBuffers has unexpected extra slots") + } } Ok(()) } diff --git a/fuzz/src/lib.rs b/fuzz/src/lib.rs index ca6cd49535e..6747b27be17 100644 --- a/fuzz/src/lib.rs +++ b/fuzz/src/lib.rs @@ -42,11 +42,9 @@ fn enable_latest_core_edition(session: &vortex_session::VortexSession) { use vortex::editions::CORE_2026_08_3; use vortex::editions::EditionSessionExt; use vortex_error::VortexExpect; - use vortex_error::vortex_err; session .enable_edition(CORE_2026_08_3) - .map_err(|error| vortex_err!("{error}")) .vortex_expect("latest core edition is registered"); } diff --git a/lang/cpp/include/vortex/array.hpp b/lang/cpp/include/vortex/array.hpp index 88d06e51905..3da45d83a70 100644 --- a/lang/cpp/include/vortex/array.hpp +++ b/lang/cpp/include/vortex/array.hpp @@ -442,7 +442,7 @@ PrimitiveView Array::values(const Session &session) const { Array canonical = canonicalize(session); const vx_array *raw = detail::Access::c_ptr(canonical); if (!vx_array_is_primitive(raw, detail::to_ptype())) { - throw VortexException("values: T does not match the array's ptype", ErrorCode::MismatchedTypes); + throw VortexException("values: T does not match the array's ptype"); } vx_error *error = nullptr; const void *data = vx_array_data_ptr_primitive(raw, &error); diff --git a/lang/cpp/include/vortex/error.hpp b/lang/cpp/include/vortex/error.hpp index 8793be76dbb..60b8d298958 100644 --- a/lang/cpp/include/vortex/error.hpp +++ b/lang/cpp/include/vortex/error.hpp @@ -11,20 +11,13 @@ namespace vortex { enum class ErrorCode { Other = VX_ERROR_CODE_OTHER, - OutOfBounds = VX_ERROR_CODE_OUT_OF_BOUNDS, - Compute = VX_ERROR_CODE_COMPUTE, - InvalidArgument = VX_ERROR_CODE_INVALID_ARGUMENT, - Serialization = VX_ERROR_CODE_SERIALIZATION, - NotImplemented = VX_ERROR_CODE_NOT_IMPLEMENTED, - MismatchedTypes = VX_ERROR_CODE_MISMATCHED_TYPES, - AssertionFailed = VX_ERROR_CODE_ASSERTION_FAILED, - Io = VX_ERROR_CODE_IO, Panic = VX_ERROR_CODE_PANIC, }; class VortexException : public std::runtime_error { public: - VortexException(const std::string &message, ErrorCode code) : std::runtime_error(message), code_(code) { + explicit VortexException(const std::string &message, ErrorCode code = ErrorCode::Other) + : std::runtime_error(message), code_(code) { } ErrorCode code() const { diff --git a/lang/cpp/include/vortex/estimate.hpp b/lang/cpp/include/vortex/estimate.hpp index 1b412555b0f..ff6f3c332b5 100644 --- a/lang/cpp/include/vortex/estimate.hpp +++ b/lang/cpp/include/vortex/estimate.hpp @@ -31,7 +31,7 @@ class Estimate { */ inline uint64_t value() const { if (type() == EstimateType::Unknown) { - throw VortexException("estimate is unknown", ErrorCode::InvalidArgument); + throw VortexException("estimate is unknown"); } return raw_.estimate; } diff --git a/lang/cpp/include/vortex/scalar.hpp b/lang/cpp/include/vortex/scalar.hpp index 9c4b7a39dd4..4deed75be05 100644 --- a/lang/cpp/include/vortex/scalar.hpp +++ b/lang/cpp/include/vortex/scalar.hpp @@ -65,86 +65,85 @@ class Scalar { template T Scalar::get() const { using enum DataTypeVariant; - using enum ErrorCode; using enum PType; const DataType data_type = dtype(); const DataTypeVariant variant = data_type.variant(); if (is_null()) { - throw VortexException("Scalar is null", InvalidArgument); + throw VortexException("Scalar is null"); } const vx_scalar *const h = handle_.get(); if constexpr (std::is_same_v) { if (variant != Bool) { - throw VortexException("scalar get type doesn't match", InvalidArgument); + throw VortexException("scalar get type doesn't match"); } return vx_scalar_get_bool(h); } else if constexpr (std::is_same_v) { if (variant != Utf8) { - throw VortexException("scalar get type doesn't match", InvalidArgument); + throw VortexException("scalar get type doesn't match"); } vx_view v = vx_scalar_get_utf8(h); return std::string_view(v.ptr, v.len); } else if constexpr (std::is_same_v) { if (variant != Binary) { - throw VortexException("scalar get type doesn't match", InvalidArgument); + throw VortexException("scalar get type doesn't match"); } vx_view v = vx_scalar_get_binary(h); return BinaryView(reinterpret_cast(v.ptr), v.len); } else if constexpr (std::is_same_v) { if (variant != Primitive || data_type.primitive_type() != U8) { - throw VortexException("scalar get type doesn't match", ErrorCode::InvalidArgument); + throw VortexException("scalar get type doesn't match"); } return vx_scalar_get_u8(h); } else if constexpr (std::is_same_v) { if (variant != Primitive || data_type.primitive_type() != U16) { - throw VortexException("scalar get type doesn't match", ErrorCode::InvalidArgument); + throw VortexException("scalar get type doesn't match"); } return vx_scalar_get_u16(h); } else if constexpr (std::is_same_v) { if (variant != Primitive || data_type.primitive_type() != U32) { - throw VortexException("scalar get type doesn't match", ErrorCode::InvalidArgument); + throw VortexException("scalar get type doesn't match"); } return vx_scalar_get_u32(h); } else if constexpr (std::is_same_v) { if (variant != Primitive || data_type.primitive_type() != U64) { - throw VortexException("scalar get type doesn't match", ErrorCode::InvalidArgument); + throw VortexException("scalar get type doesn't match"); } return vx_scalar_get_u64(h); } else if constexpr (std::is_same_v) { if (variant != Primitive || data_type.primitive_type() != I8) { - throw VortexException("scalar get type doesn't match", ErrorCode::InvalidArgument); + throw VortexException("scalar get type doesn't match"); } return vx_scalar_get_i8(h); } else if constexpr (std::is_same_v) { if (variant != Primitive || data_type.primitive_type() != I16) { - throw VortexException("scalar get type doesn't match", ErrorCode::InvalidArgument); + throw VortexException("scalar get type doesn't match"); } return vx_scalar_get_i16(h); } else if constexpr (std::is_same_v) { if (variant != Primitive || data_type.primitive_type() != I32) { - throw VortexException("scalar get type doesn't match", ErrorCode::InvalidArgument); + throw VortexException("scalar get type doesn't match"); } return vx_scalar_get_i32(h); } else if constexpr (std::is_same_v) { if (variant != Primitive || data_type.primitive_type() != I64) { - throw VortexException("scalar get type doesn't match", ErrorCode::InvalidArgument); + throw VortexException("scalar get type doesn't match"); } return vx_scalar_get_i64(h); } else if constexpr (std::is_same_v) { if (variant != Primitive || data_type.primitive_type() != F32) { - throw VortexException("scalar get type doesn't match", ErrorCode::InvalidArgument); + throw VortexException("scalar get type doesn't match"); } return vx_scalar_get_f32(h); } else if constexpr (std::is_same_v) { if (variant != Primitive || data_type.primitive_type() != F64) { - throw VortexException("scalar get type doesn't match", ErrorCode::InvalidArgument); + throw VortexException("scalar get type doesn't match"); } return vx_scalar_get_f64(h); } else if constexpr (std::is_same_v) { if (variant != Primitive || data_type.primitive_type() != F16) { - throw VortexException("scalar get type doesn't match", ErrorCode::InvalidArgument); + throw VortexException("scalar get type doesn't match"); } const uint16_t bits = vx_scalar_get_f16_bits(h); #if __STDCPP_FLOAT16_T__ != 1 @@ -160,10 +159,10 @@ T Scalar::get() const { template T Scalar::get_decimal() const { if (is_null()) { - throw VortexException("Scalar is null", ErrorCode::InvalidArgument); + throw VortexException("Scalar is null"); } if (dtype().variant() != DataTypeVariant::Decimal) { - throw VortexException("DataType is not Decimal", ErrorCode::InvalidArgument); + throw VortexException("DataType is not Decimal"); } const vx_scalar *const h = handle_.get(); if constexpr (std::is_same_v) { diff --git a/lang/cpp/src/array.cpp b/lang/cpp/src/array.cpp index 55c4b2827dd..2e0775ce613 100644 --- a/lang/cpp/src/array.cpp +++ b/lang/cpp/src/array.cpp @@ -26,14 +26,13 @@ Validity::Validity(const Validity &other) Validity::Validity(ValidityType type) : type_(type), array_(nullptr) { if (type == ValidityType::FromArray) { - throw VortexException("Validity(ValidityType) called with ValidityType::FromArray", - ErrorCode::InvalidArgument); + throw VortexException("Validity(ValidityType) called with ValidityType::FromArray"); } } Validity Validity::from_array(const Array &bools) { if (!bools.has_dtype(DataTypeVariant::Bool)) { - throw VortexException("Validity array isn't a Bool array", ErrorCode::InvalidArgument); + throw VortexException("Validity array isn't a Bool array"); } return {ValidityType::FromArray, vx_array_clone(Access::c_ptr(bools))}; } @@ -67,7 +66,7 @@ Validity::~Validity() { Array Validity::array() const { if (type_ != ValidityType::FromArray || array_ == nullptr) { - throw VortexException("validity has no backing array", ErrorCode::InvalidArgument); + throw VortexException("validity has no backing array"); } return Access::adopt(vx_array_clone(array_)); } @@ -139,7 +138,7 @@ ValidityBits::~ValidityBits() { static const vx_struct_fields *struct_fields_or_throw(const vx_dtype *dtype) { const vx_struct_fields *fields = vx_dtype_struct_dtype(dtype); if (fields == nullptr) { - throw VortexException("dtype is not a struct", ErrorCode::MismatchedTypes); + throw VortexException("dtype is not a struct"); } return fields; } @@ -254,7 +253,7 @@ Array Array::field(std::string_view name) const { return this->field(i); } } - throw VortexException("no field named \"" + std::string(name) + "\"", ErrorCode::InvalidArgument); + throw VortexException("no field named \"" + std::string(name) + "\""); } Array Array::slice(size_t begin, size_t end) const { @@ -311,7 +310,7 @@ PrimitiveView Array::bools(const Session &session) const { Array canonical = canonicalize(session); const vx_array *raw = Access::c_ptr(canonical); if (!vx_array_has_dtype(raw, DTYPE_BOOL)) { - throw VortexException("bools(): array is not a Bool array", ErrorCode::MismatchedTypes); + throw VortexException("bools(): array is not a Bool array"); } detail::ValidityBits validity(session, raw); const size_t len = vx_array_len(raw); @@ -322,7 +321,7 @@ StringView Array::strings(const Session &session) const { Array canonical = canonicalize(session); const vx_array *raw = Access::c_ptr(canonical); if (!vx_array_has_dtype(raw, DTYPE_UTF8)) { - throw VortexException("strings(): array is not a Utf8 array", ErrorCode::MismatchedTypes); + throw VortexException("strings(): array is not a Utf8 array"); } detail::ValidityBits validity(session, raw); const size_t len = vx_array_len(raw); @@ -333,7 +332,7 @@ BytesView Array::bytes(const Session &session) const { Array canonical = canonicalize(session); const vx_array *raw = Access::c_ptr(canonical); if (!vx_array_has_dtype(raw, DTYPE_BINARY)) { - throw VortexException("bytes(): array is not a Binary array", ErrorCode::MismatchedTypes); + throw VortexException("bytes(): array is not a Binary array"); } detail::ValidityBits validity(session, raw); const size_t len = vx_array_len(raw); diff --git a/lang/cpp/src/dtype.cpp b/lang/cpp/src/dtype.cpp index af7d42ccbd4..c4157495d49 100644 --- a/lang/cpp/src/dtype.cpp +++ b/lang/cpp/src/dtype.cpp @@ -76,7 +76,7 @@ namespace { const vx_struct_fields *struct_fields_or_throw(const vx_dtype *dtype) { const vx_struct_fields *fields = vx_dtype_struct_dtype(dtype); if (fields == nullptr) { - throw VortexException("dtype is not a struct", ErrorCode::MismatchedTypes); + throw VortexException("dtype is not a struct"); } return fields; } @@ -92,12 +92,11 @@ std::vector DataType::fields() const { for (uint64_t idx = 0; idx < fields_size; ++idx) { const vx_view name = vx_struct_fields_field_name(fields.get(), idx); if (name.ptr == nullptr) { - throw VortexException("error getting field name at index "s + std::to_string(idx), - ErrorCode::Other); + throw VortexException("error getting field name at index "s + std::to_string(idx)); } const vx_dtype *dtype = vx_struct_fields_field_dtype(fields.get(), idx); if (dtype == nullptr) { - throw VortexException("error getting dtype at index "s + std::to_string(idx), ErrorCode::Other); + throw VortexException("error getting dtype at index "s + std::to_string(idx)); } out.push_back(StructField {{name.ptr, name.len}, DataType(dtype)}); } @@ -107,7 +106,7 @@ std::vector DataType::fields() const { DataType DataType::list_element() const { const vx_dtype *element = vx_dtype_list_element(handle_.get()); if (element == nullptr) { - throw VortexException("dtype is not a list", ErrorCode::MismatchedTypes); + throw VortexException("dtype is not a list"); } return DataType(element); } @@ -115,7 +114,7 @@ DataType DataType::list_element() const { DataType DataType::fixed_size_list_element() const { const vx_dtype *element = vx_dtype_fixed_size_list_element(handle_.get()); if (element == nullptr) { - throw VortexException("dtype is not a fixed-size list", ErrorCode::MismatchedTypes); + throw VortexException("dtype is not a fixed-size list"); } return DataType(element); } diff --git a/lang/cpp/src/expression.cpp b/lang/cpp/src/expression.cpp index d314ae26293..93ee6fe075a 100644 --- a/lang/cpp/src/expression.cpp +++ b/lang/cpp/src/expression.cpp @@ -35,7 +35,7 @@ Expression &Expression::operator=(const Expression &other) { Expression Expression::operator[](std::string_view field) const { vx_expression *out = vx_expression_get_item(to_view(field), handle_.get()); if (out == nullptr) { - throw VortexException("get_item: field name is not valid UTF-8", ErrorCode::InvalidArgument); + throw VortexException("get_item: field name is not valid UTF-8"); } return Access::adopt(out); } @@ -129,7 +129,7 @@ static Expression combine(vx_expression *(*combiner)(const vx_expression *const } vx_expression *out = combiner(raw.data(), raw.size()); if (out == nullptr) { - throw VortexException("empty expression list", ErrorCode::InvalidArgument); + throw VortexException("empty expression list"); } return Access::adopt(out); } diff --git a/lang/cpp/src/scalar.cpp b/lang/cpp/src/scalar.cpp index f0427aec647..85988c4db25 100644 --- a/lang/cpp/src/scalar.cpp +++ b/lang/cpp/src/scalar.cpp @@ -80,7 +80,7 @@ vx_scalar *make_primitive(vx_ptype ptype, const void *value, bool nullable) { case PTYPE_F64: return vx_scalar_new_f64(*static_cast(value), nullable); } - throw VortexException("unsupported ptype", ErrorCode::InvalidArgument); + throw VortexException("unsupported ptype"); } vx_scalar *make_utf8(std::string_view value, bool nullable) { diff --git a/lang/cpp/src/writer.cpp b/lang/cpp/src/writer.cpp index 57035171a14..ef2c790866d 100644 --- a/lang/cpp/src/writer.cpp +++ b/lang/cpp/src/writer.cpp @@ -33,7 +33,7 @@ Writer Writer::open(const Session &session, std::string_view path, const DataTyp void Writer::push(std::span arrays) { if (handle_ == nullptr) { - throw VortexException("null handle_", ErrorCode::InvalidArgument); + throw VortexException("null handle_"); } vx_error *error = nullptr; for (const Array &array : arrays) { @@ -52,7 +52,7 @@ void Writer::push(std::initializer_list arrays) { void Writer::finish() { if (handle_ == nullptr) { - throw VortexException("finish() called twice", ErrorCode::InvalidArgument); + throw VortexException("finish() called twice"); } vx_error *error = nullptr; vx_array_sink_close(handle_.release(), &error); diff --git a/lang/cpp/tests/array.cpp b/lang/cpp/tests/array.cpp index 3b0f5b842ff..6fa75f94072 100644 --- a/lang/cpp/tests/array.cpp +++ b/lang/cpp/tests/array.cpp @@ -220,8 +220,8 @@ TEST_CASE("Error with a code", "[array]") { (void)a.scalar_at(Session {}, 2); FAIL("expected exception"); } catch (const VortexException &e) { - REQUIRE_FALSE(std::string(e.what()).empty()); - REQUIRE(e.code() == ErrorCode::OutOfBounds); + REQUIRE(std::string(e.what()).find("out of bounds") != std::string::npos); + REQUIRE(e.code() == ErrorCode::Other); } } } // namespace diff --git a/vortex-array/src/aggregate_fn/accumulator_grouped.rs b/vortex-array/src/aggregate_fn/accumulator_grouped.rs index 6405d993f40..832f4a06f78 100644 --- a/vortex-array/src/aggregate_fn/accumulator_grouped.rs +++ b/vortex-array/src/aggregate_fn/accumulator_grouped.rs @@ -253,7 +253,9 @@ impl DynGroupedAccumulator for GroupedAccumulator { match canonical { Canonical::List(groups) => self.accumulate_grouped_array(groups.into(), ctx), Canonical::FixedSizeList(groups) => self.accumulate_grouped_array(groups.into(), ctx), - _ => vortex_panic!("We checked the DType above, so this should never happen"), + _ => { + vortex_panic!("We checked the DType above, so this should never happen") + } } } diff --git a/vortex-array/src/aggregate_fn/fns/nan_count/mod.rs b/vortex-array/src/aggregate_fn/fns/nan_count/mod.rs index d77d6dc5a26..1251db1616b 100644 --- a/vortex-array/src/aggregate_fn/fns/nan_count/mod.rs +++ b/vortex-array/src/aggregate_fn/fns/nan_count/mod.rs @@ -6,7 +6,6 @@ mod primitive; use vortex_error::VortexExpect; use vortex_error::VortexResult; use vortex_error::vortex_bail; -use vortex_error::vortex_err; use vortex_session::VortexSession; use vortex_session::registry::CachedId; @@ -39,7 +38,7 @@ pub fn nan_count(array: &ArrayRef, ctx: &mut ExecutionCtx) -> VortexResult VortexResult { if let Precision::Exact(null_count_scalar) = array.statistics().get(Stat::NullCount) { return usize::try_from(&null_count_scalar) - .map_err(|e| vortex_err!("Failed to convert null count stat to usize: {e}")); + .map_err(|e| e.with_context("Failed to convert null count stat to usize")); } let mut acc = Accumulator::try_new(NullCount, EmptyOptions, array.dtype().clone())?; diff --git a/vortex-array/src/aggregate_fn/fns/sum/mod.rs b/vortex-array/src/aggregate_fn/fns/sum/mod.rs index 94c5fa7643d..98596975f93 100644 --- a/vortex-array/src/aggregate_fn/fns/sum/mod.rs +++ b/vortex-array/src/aggregate_fn/fns/sum/mod.rs @@ -283,7 +283,9 @@ impl AggregateFnVTable for Sum { } Canonical::Bool(b) => accumulate_bool(&mut inner, b, ctx), Canonical::Decimal(d) => accumulate_decimal(&mut inner, args.return_dtype, d, ctx), - _ => vortex_bail!("Unsupported canonical type for sum: {}", batch.dtype()), + _ => { + vortex_bail!("Unsupported canonical type for sum: {}", batch.dtype()) + } }, Columnar::Constant(_) => unreachable!(), }; diff --git a/vortex-array/src/aggregate_fn/fns/sum/primitive.rs b/vortex-array/src/aggregate_fn/fns/sum/primitive.rs index fe61454a914..a63edacf6b3 100644 --- a/vortex-array/src/aggregate_fn/fns/sum/primitive.rs +++ b/vortex-array/src/aggregate_fn/fns/sum/primitive.rs @@ -59,7 +59,9 @@ fn accumulate_primitive_all( Ok(false) } ), - SumState::Decimal(_) => vortex_panic!("decimal sum state with primitive input"), + SumState::Decimal(_) => { + vortex_panic!("decimal sum state with primitive input") + } } } @@ -174,7 +176,9 @@ fn accumulate_primitive_valid( Ok(false) } ), - SumState::Decimal(_) => vortex_panic!("decimal sum state with primitive input"), + SumState::Decimal(_) => { + vortex_panic!("decimal sum state with primitive input") + } } } diff --git a/vortex-array/src/aggregate_fn/fns/sum_v2/mod.rs b/vortex-array/src/aggregate_fn/fns/sum_v2/mod.rs index 46fe681ab0a..b8f0e731597 100644 --- a/vortex-array/src/aggregate_fn/fns/sum_v2/mod.rs +++ b/vortex-array/src/aggregate_fn/fns/sum_v2/mod.rs @@ -291,7 +291,9 @@ impl AggregateFnVTable for SumV2 { Canonical::Decimal(array) => { accumulate_decimal(&mut partial.sum, args.return_dtype, array, ctx) } - _ => vortex_bail!("Unsupported canonical type for sum_v2: {}", batch.dtype()), + _ => { + vortex_bail!("Unsupported canonical type for sum_v2: {}", batch.dtype()) + } }, Columnar::Constant(_) => unreachable!(), }; @@ -419,7 +421,9 @@ fn decode_partial_scalar<'a>( let bool_field = |name: &str| -> VortexResult { match &values[field_index(name)?] { Some(ScalarValue::Bool(value)) => Ok(*value), - _ => vortex_bail!("SumV2 partial field {name} must be a non-null bool"), + _ => { + vortex_bail!("SumV2 partial field {name} must be a non-null bool") + } } }; @@ -491,7 +495,9 @@ fn checked_add_sum_value( Some(_) | None => true, } } - (_, other) => vortex_bail!("SumV2 cannot add {other} to a {return_dtype} sum"), + (_, other) => { + vortex_bail!("SumV2 cannot add {other} to a {return_dtype} sum") + } }) } diff --git a/vortex-array/src/aggregate_fn/fns/uncompressed_size_in_bytes/mod.rs b/vortex-array/src/aggregate_fn/fns/uncompressed_size_in_bytes/mod.rs index 285d1948960..eca323d70a1 100644 --- a/vortex-array/src/aggregate_fn/fns/uncompressed_size_in_bytes/mod.rs +++ b/vortex-array/src/aggregate_fn/fns/uncompressed_size_in_bytes/mod.rs @@ -160,7 +160,7 @@ fn uncompressed_size_in_bytes_u64( }; if let Some(size_scalar) = cached { return u64::try_from(&size_scalar) - .map_err(|e| vortex_err!("Failed to convert uncompressed size stat to u64: {e}")); + .map_err(|e| e.with_context("Failed to convert uncompressed size stat to u64")); } // The accumulator caches an exact size on `array` through the stat slot. diff --git a/vortex-array/src/array/probe/array.rs b/vortex-array/src/array/probe/array.rs index 57a908905a4..99661ccf584 100644 --- a/vortex-array/src/array/probe/array.rs +++ b/vortex-array/src/array/probe/array.rs @@ -138,7 +138,7 @@ pub(super) fn check_bounds(array: &ArrayRef, index: usize) -> VortexResult<()> { #[cold] #[inline(never)] fn out_of_bounds(index: usize, len: usize) -> VortexError { - vortex_err!(OutOfBounds: index, 0, len) + vortex_err!("index {index} out of bounds from 0 to {len}") } /// The encoding state type of `V`'s operations vtable. diff --git a/vortex-array/src/arrays/chunked/array.rs b/vortex-array/src/arrays/chunked/array.rs index 6c0169d7a7c..98ace53eadf 100644 --- a/vortex-array/src/arrays/chunked/array.rs +++ b/vortex-array/src/arrays/chunked/array.rs @@ -149,7 +149,7 @@ impl ChunkedData { pub fn validate(chunks: &[ArrayRef], dtype: &DType) -> VortexResult<()> { for chunk in chunks { if chunk.dtype() != dtype { - vortex_bail!(MismatchedTypes: dtype, chunk.dtype()); + vortex_bail!("expected type: {} but instead got {}", dtype, chunk.dtype()); } } @@ -172,7 +172,11 @@ impl Array { for chunk in chunks { if VALIDATE && chunk.dtype() != &dtype { - vortex_bail!(MismatchedTypes: &dtype, chunk.dtype()); + vortex_bail!( + "expected type: {} but instead got {}", + &dtype, + chunk.dtype() + ); } len += chunk.len(); chunk_offsets.push(len); diff --git a/vortex-array/src/arrays/chunked/compute/take.rs b/vortex-array/src/arrays/chunked/compute/take.rs index 15511c5a8cc..5602d051b23 100644 --- a/vortex-array/src/arrays/chunked/compute/take.rs +++ b/vortex-array/src/arrays/chunked/compute/take.rs @@ -97,7 +97,12 @@ fn take_chunked_via_sort( if let Some(&(index, _)) = pairs.last() { let index = usize::try_from(index)?; if index >= array.len() { - vortex_bail!(OutOfBounds: index, 0, array.len()); + vortex_bail!( + "index {} out of bounds from {} to {}", + index, + 0, + array.len() + ); } } @@ -209,7 +214,12 @@ fn take_chunked( let index = usize::try_from(index)?; if index >= array.len() { - vortex_bail!(OutOfBounds: index, 0, array.len()); + vortex_bail!( + "index {} out of bounds from {} to {}", + index, + 0, + array.len() + ); } let still_sorted = last_index.is_none_or(|last_index| index >= last_index); @@ -314,7 +324,10 @@ fn take_chunked_sorted( } if cursor != indices_values.len() { vortex_bail!( - OutOfBounds: usize::try_from(indices_values[cursor])?, 0, array.as_ref().len() + "index {} out of bounds from {} to {}", + usize::try_from(indices_values[cursor])?, + 0, + array.as_ref().len() ); } @@ -434,7 +447,12 @@ fn take_piecewise_chunked( .checked_add(length) .ok_or_else(|| vortex_err!("PiecewiseSequenceArray range overflows usize"))?; if end > array_len { - vortex_bail!(OutOfBounds: end - 1, 0, array_len); + vortex_bail!( + "index {} out of bounds from {} to {}", + end - 1, + 0, + array_len + ); } total_len = total_len .checked_add(length) diff --git a/vortex-array/src/arrays/chunked/vtable/canonical.rs b/vortex-array/src/arrays/chunked/vtable/canonical.rs index b0c935fcedb..5d6d35d0dae 100644 --- a/vortex-array/src/arrays/chunked/vtable/canonical.rs +++ b/vortex-array/src/arrays/chunked/vtable/canonical.rs @@ -366,7 +366,9 @@ mod tests { fn into_variant(canonical: Canonical) -> VortexResult { match canonical { Canonical::Variant(array) => Ok(array), - other => vortex_bail!("expected Variant canonical array, got {other:?}"), + other => { + vortex_bail!("expected Variant canonical array, got {other:?}") + } } } @@ -549,7 +551,9 @@ mod tests { let canonical = chunked.clone().execute::(&mut ctx)?; let fsl = match canonical { Canonical::FixedSizeList(fsl) => fsl, - other => vortex_bail!("expected FixedSizeList canonical array, got {other:?}"), + other => { + vortex_bail!("expected FixedSizeList canonical array, got {other:?}") + } }; assert_eq!(fsl.len(), 5); diff --git a/vortex-array/src/arrays/decimal/array.rs b/vortex-array/src/arrays/decimal/array.rs index eff2e5c8688..9269e7c83ea 100644 --- a/vortex-array/src/arrays/decimal/array.rs +++ b/vortex-array/src/arrays/decimal/array.rs @@ -302,14 +302,14 @@ impl DecimalData { let byte_width = values_type.byte_width(); vortex_ensure!( buffer.len().is_multiple_of(byte_width), - InvalidArgument: "decimal buffer size {} is not divisible by element width {}", + "decimal buffer size {} is not divisible by element width {}", buffer.len(), byte_width, ); match_each_decimal_value_type!(values_type, |D| { vortex_ensure!( buffer.is_aligned_to(Alignment::of::()), - InvalidArgument: "decimal buffer alignment {:?} is invalid for values type {:?}", + "decimal buffer alignment {:?} is invalid for values type {:?}", buffer.alignment(), D::DECIMAL_TYPE, ); diff --git a/vortex-array/src/arrays/decimal/vtable/mod.rs b/vortex-array/src/arrays/decimal/vtable/mod.rs index fe4d8ddeea7..f3f0c9494a3 100644 --- a/vortex-array/src/arrays/decimal/vtable/mod.rs +++ b/vortex-array/src/arrays/decimal/vtable/mod.rs @@ -133,7 +133,7 @@ impl VTable for Decimal { vortex_ensure_eq!( data.len(), len, - InvalidArgument: "DecimalArray length does not match outer length", + "DecimalArray length does not match outer length", ); let validity = crate::array::child_to_validity(slots[DecimalSlots::VALIDITY].as_ref(), *nullability); @@ -141,7 +141,7 @@ impl VTable for Decimal { vortex_ensure_eq!( validity_len, len, - InvalidArgument: "DecimalArray validity len does not match outer length", + "DecimalArray validity len does not match outer length", ); } diff --git a/vortex-array/src/arrays/dict/array.rs b/vortex-array/src/arrays/dict/array.rs index 79f5ac8be33..69d5be3a39e 100644 --- a/vortex-array/src/arrays/dict/array.rs +++ b/vortex-array/src/arrays/dict/array.rs @@ -116,7 +116,7 @@ impl DictData { /// It is an error to provide a nullable `codes` with non-nullable `values`. pub(crate) fn try_new(codes_dtype: &DType) -> VortexResult { if !codes_dtype.is_int() { - vortex_bail!(MismatchedTypes: "int", codes_dtype); + vortex_bail!("expected type: int but instead got {}", codes_dtype); } Ok(unsafe { Self::new_unchecked() }) diff --git a/vortex-array/src/arrays/filter/execute/take/rank.rs b/vortex-array/src/arrays/filter/execute/take/rank.rs index 7bfb9b24773..7743c1df02c 100644 --- a/vortex-array/src/arrays/filter/execute/take/rank.rs +++ b/vortex-array/src/arrays/filter/execute/take/rank.rs @@ -60,7 +60,12 @@ fn sequential_take_len_typed( } if ranks.len() > filtered_len { - vortex_bail!(OutOfBounds: ranks.len() - 1, 0, filtered_len); + vortex_bail!( + "index {} out of bounds from {} to {}", + ranks.len() - 1, + 0, + filtered_len + ); } Ok(Some(ranks.len())) @@ -141,7 +146,12 @@ pub(in crate::arrays::filter) fn validate_rank( ) -> VortexResult { let rank: usize = rank.as_(); if rank >= filtered_len { - vortex_bail!(OutOfBounds: rank, 0, filtered_len); + vortex_bail!( + "index {} out of bounds from {} to {}", + rank, + 0, + filtered_len + ); } Ok(rank) } diff --git a/vortex-array/src/arrays/fixed_size_list/array.rs b/vortex-array/src/arrays/fixed_size_list/array.rs index 6f35e5c91a4..bae1829f67a 100644 --- a/vortex-array/src/arrays/fixed_size_list/array.rs +++ b/vortex-array/src/arrays/fixed_size_list/array.rs @@ -188,7 +188,7 @@ impl FixedSizeListData { vortex_ensure_eq!( len, validity_len, - InvalidArgument: "validity length does not match fixed-size list array size", + "validity length does not match fixed-size list array size", ); } @@ -197,7 +197,7 @@ impl FixedSizeListData { if list_size == 0 { vortex_ensure!( elements.is_empty(), - InvalidArgument: "a degenerate (`list_size == 0`) `FixedSizeList` should have no underlying elements" + "a degenerate (`list_size == 0`) `FixedSizeList` should have no underlying elements" ); return Ok(()); } @@ -205,7 +205,7 @@ impl FixedSizeListData { vortex_ensure_eq!( len * list_size as usize, elements.len(), - InvalidArgument: "the `elements` array has the incorrect number of elements to construct a \ + "the `elements` array has the incorrect number of elements to construct a \ `FixedSizeList[{list_size}] array of length {len}", ); diff --git a/vortex-array/src/arrays/fixed_size_list/compute/take.rs b/vortex-array/src/arrays/fixed_size_list/compute/take.rs index acc05cca73d..7fb7d6c0da8 100644 --- a/vortex-array/src/arrays/fixed_size_list/compute/take.rs +++ b/vortex-array/src/arrays/fixed_size_list/compute/take.rs @@ -177,7 +177,12 @@ fn take_piecewise_fsl( .checked_add(length) .ok_or_else(|| vortex_err!("PiecewiseSequenceArray range overflows usize"))?; if end > array_len { - vortex_bail!(OutOfBounds: end - 1, 0, array_len); + vortex_bail!( + "index {} out of bounds from {} to {}", + end - 1, + 0, + array_len + ); } total_len = total_len .checked_add(length) @@ -308,7 +313,12 @@ fn take_non_empty_non_degenerate_elements( let data_idx: usize = data_idx.as_(); if data_idx >= array_len { - vortex_bail!(OutOfBounds: data_idx, 0, array_len); + vortex_bail!( + "index {} out of bounds from {} to {}", + data_idx, + 0, + array_len + ); } Ok((data_idx * list_size) as u64) }) @@ -332,7 +342,12 @@ fn bounds_check_valid_indices( if is_index_valid { let data_idx = data_idx.as_(); if data_idx >= array_len { - vortex_bail!(OutOfBounds: data_idx, 0, array_len); + vortex_bail!( + "index {} out of bounds from {} to {}", + data_idx, + 0, + array_len + ); } } } diff --git a/vortex-array/src/arrays/list/array.rs b/vortex-array/src/arrays/list/array.rs index 377499faea6..e4b6e7efa54 100644 --- a/vortex-array/src/arrays/list/array.rs +++ b/vortex-array/src/arrays/list/array.rs @@ -205,13 +205,13 @@ impl ListData { // Offsets must have at least one element vortex_ensure!( !offsets.is_empty(), - InvalidArgument: "Offsets must have at least one element, [0] for an empty list" + "Offsets must have at least one element, [0] for an empty list" ); // Offsets must be of integer type, and cannot go lower than 0. vortex_ensure!( offsets.dtype().is_int() && !offsets.dtype().is_nullable(), - InvalidArgument: "offsets have invalid type {}", + "offsets have invalid type {}", offsets.dtype() ); @@ -220,9 +220,9 @@ impl ListData { // Offsets must be sorted (but not strictly sorted, zero-length lists are allowed) if let Some(is_sorted) = offsets.statistics().compute_is_sorted(ctx) { - vortex_ensure!(is_sorted, InvalidArgument: "offsets must be sorted"); + vortex_ensure!(is_sorted, "offsets must be sorted"); } else { - vortex_bail!(InvalidArgument: "offsets must report is_sorted statistic"); + vortex_bail!("offsets must report is_sorted statistic"); } // Validate that offsets min is non-negative, and max does not exceed the length of @@ -244,7 +244,7 @@ impl ListData { vortex_ensure!( min >= 0, - InvalidArgument: "offsets minimum {min} outside valid range [0, {max}]" + "offsets minimum {min} outside valid range [0, {max}]" ); vortex_ensure!( @@ -253,7 +253,7 @@ impl ListData {

::PTYPE, elements.len() )), - InvalidArgument: "Max offset {max} is beyond the length of the elements array {}", + "Max offset {max} is beyond the length of the elements array {}", elements.len() ); } @@ -261,7 +261,7 @@ impl ListData { } else { // TODO(aduffy): fallback to slower validation pathway? vortex_bail!( - InvalidArgument: "offsets array with encoding {} must support min_max compute function", + "offsets array with encoding {} must support min_max compute function", offsets.encoding_id() ); }; @@ -271,7 +271,7 @@ impl ListData { vortex_ensure_eq!( validity_len, offsets.len() - 1, - InvalidArgument: "validity length does not match array size", + "validity length does not match array size", ); } diff --git a/vortex-array/src/arrays/null/compute/take.rs b/vortex-array/src/arrays/null/compute/take.rs index 6a60c94a54c..d006e168c0b 100644 --- a/vortex-array/src/arrays/null/compute/take.rs +++ b/vortex-array/src/arrays/null/compute/take.rs @@ -27,7 +27,12 @@ impl TakeExecute for Null { match_each_integer_ptype!(indices.ptype(), |T| { for index in indices.as_slice::() { if (*index as usize) >= array.len() { - vortex_bail!(OutOfBounds: *index as usize, 0, array.len()); + vortex_bail!( + "index {} out of bounds from {} to {}", + *index as usize, + 0, + array.len() + ); } } }); diff --git a/vortex-array/src/arrays/primitive/array/mod.rs b/vortex-array/src/arrays/primitive/array/mod.rs index 417f4e184a6..28cee4d21d6 100644 --- a/vortex-array/src/arrays/primitive/array/mod.rs +++ b/vortex-array/src/arrays/primitive/array/mod.rs @@ -327,7 +327,6 @@ impl PrimitiveData { && buffer.len() != len { return Err(vortex_err!( - InvalidArgument: "Buffer and validity length mismatch: buffer={}, validity={}", buffer.len(), len diff --git a/vortex-array/src/arrays/primitive/compute/cast.rs b/vortex-array/src/arrays/primitive/compute/cast.rs index c937d68d611..022d2216d2c 100644 --- a/vortex-array/src/arrays/primitive/compute/cast.rs +++ b/vortex-array/src/arrays/primitive/compute/cast.rs @@ -107,8 +107,9 @@ impl CastKernel for Primitive { if same_rep { if !values_fit_in(array, new_ptype, ctx, true) { vortex_bail!( - Compute: "Cannot cast {} to {} — values exceed target range", - src_ptype, new_ptype, + "Cannot cast {} to {} — values exceed target range", + src_ptype, + new_ptype, ); } return Ok(Some(reinterpret(array, new_ptype, new_validity))); @@ -132,8 +133,9 @@ fn cast_to_decimal( ) -> VortexResult { if !array.ptype().is_int() { vortex_bail!( - Compute: "Cannot cast floating primitive {} to decimal {}", - array.ptype(), decimal_dtype + "Cannot cast floating primitive {} to decimal {}", + array.ptype(), + decimal_dtype ); } @@ -399,7 +401,7 @@ where #[cold] #[inline(never)] fn scale_factor_error(scale: i8) -> VortexError { - vortex_err!(Compute: "Cannot create decimal scale factor for scale {scale}") + vortex_err!("Cannot create decimal scale factor for scale {scale}") } fn decimal_value_fits_precision( @@ -450,7 +452,7 @@ where { let Some(value) = ::from(value) else { return vortex_err!( - Compute: "primitive value cannot be represented while casting to {}", + "primitive value cannot be represented while casting to {}", decimal_dtype ); }; @@ -465,7 +467,7 @@ where "primitive-to-decimal fast path rejected a value that the scalar cast accepts" ); vortex_err!( - Compute: "primitive value cannot be represented while casting to {}", + "primitive value cannot be represented while casting to {}", decimal_dtype ) } @@ -484,8 +486,9 @@ where { let overflow = || { vortex_err!( - Compute: "Cannot cast {} to {} — value exceeds target range", - F::PTYPE, T::PTYPE, + "Cannot cast {} to {} — value exceeds target range", + F::PTYPE, + T::PTYPE, ) }; @@ -664,7 +667,6 @@ mod test { use rstest::rstest; use vortex_buffer::BitBuffer; use vortex_buffer::buffer; - use vortex_error::VortexError; use vortex_error::VortexResult; use vortex_mask::Mask; use vortex_session::VortexSession; @@ -1093,7 +1095,6 @@ mod test { .map(|c| c.into_array()) }) .unwrap_err(); - assert!(matches!(error, VortexError::Compute(..))); assert!(error.to_string().contains("values exceed target range")); } @@ -1109,7 +1110,6 @@ mod test { }) .unwrap_err(); - assert!(matches!(err, VortexError::InvalidArgument(..))); assert!( err.to_string() .contains("Cannot cast array with invalid values to non-nullable type.") @@ -1173,7 +1173,10 @@ mod test { .map(|c| c.into_array()) }) .unwrap_err(); - assert!(matches!(err, VortexError::Compute(..))); + assert!( + err.to_string().contains("values exceed target range"), + "{err}" + ); } /// All-null array cast between same-width types should succeed without diff --git a/vortex-array/src/arrays/struct_/vtable/mod.rs b/vortex-array/src/arrays/struct_/vtable/mod.rs index 15cf128b8e9..d258d82ad6e 100644 --- a/vortex-array/src/arrays/struct_/vtable/mod.rs +++ b/vortex-array/src/arrays/struct_/vtable/mod.rs @@ -69,7 +69,7 @@ impl VTable for Struct { let expected_slots = struct_dtype.nfields() + 1; if slots.len() != expected_slots { vortex_bail!( - InvalidArgument: "StructArray has {} slots but expected {}", + "StructArray has {} slots but expected {}", slots.len(), expected_slots ); @@ -80,7 +80,7 @@ impl VTable for Struct { && validity_len != len { vortex_bail!( - InvalidArgument: "StructArray validity length {} does not match outer length {}", + "StructArray validity length {} does not match outer length {}", validity_len, len ); @@ -98,14 +98,14 @@ impl VTable for Struct { .ok_or_else(|| vortex_error::vortex_err!("StructArray missing field slot {idx}"))?; if field.len() != len { vortex_bail!( - InvalidArgument: "StructArray field {idx} has length {} but expected {}", + "StructArray field {idx} has length {} but expected {}", field.len(), len ); } if field.dtype() != &field_dtype { vortex_bail!( - InvalidArgument: "StructArray field {idx} has dtype {} but expected {}", + "StructArray field {idx} has dtype {} but expected {}", field.dtype(), field_dtype ); diff --git a/vortex-array/src/arrays/validation_tests.rs b/vortex-array/src/arrays/validation_tests.rs index 938c0e8a709..3c59d1a02c2 100644 --- a/vortex-array/src/arrays/validation_tests.rs +++ b/vortex-array/src/arrays/validation_tests.rs @@ -14,7 +14,6 @@ mod tests { use vortex_buffer::Buffer; use vortex_buffer::ByteBuffer; use vortex_buffer::buffer; - use vortex_error::VortexError; use vortex_session::VortexSession; use crate::IntoArray; @@ -42,7 +41,11 @@ mod tests { let chunk2 = buffer![4i64, 5, 6].into_array(); let result = ChunkedArray::try_new(vec![chunk1, chunk2], PType::I32.into()); - assert!(matches!(result, Err(VortexError::MismatchedTypes(_, _, _)))); + let err = result.unwrap_err().to_string(); + assert!( + err.contains("expected type: i32 but instead got i64"), + "{err}" + ); } #[test] @@ -53,7 +56,11 @@ mod tests { let decimal_dtype = crate::dtype::DecimalDType::new(10, 2); let result = DecimalArray::try_new(buffer, decimal_dtype, validity); - assert!(matches!(result, Err(VortexError::InvalidArgument(_, _)))); + let err = result.unwrap_err().to_string(); + assert!( + err.contains("DecimalArray validity len does not match outer length"), + "{err}" + ); } #[test] @@ -63,7 +70,11 @@ mod tests { let validity = Validity::from_iter([true, false]); // Length 2, buffer is length 3. let result = PrimitiveArray::try_new(buffer, validity); - assert!(matches!(result, Err(VortexError::InvalidArgument(_, _)))); + let err = result.unwrap_err().to_string(); + assert!( + err.contains("Buffer and validity length mismatch: buffer=3, validity=2"), + "{err}" + ); } #[test] @@ -74,7 +85,11 @@ mod tests { let offsets = buffer![0i64, 2, 5].into_array(); // 5 > 3. let result = ListArray::try_new(elements, offsets, Validity::NonNullable, &mut ctx); - assert!(matches!(result, Err(VortexError::InvalidArgument(_, _)))); + let err = result.unwrap_err().to_string(); + assert!( + err.contains("Max offset 5 is beyond the length of the elements array 3"), + "{err}" + ); } #[test] @@ -83,7 +98,11 @@ mod tests { let elements = buffer![1i32, 2, 3, 4, 5].into_array(); // 5 elements. let result = FixedSizeListArray::try_new(elements, 2, Validity::NonNullable, 3); // Expects 2 * 3 = 6. - assert!(matches!(result, Err(VortexError::InvalidArgument(_, _)))); + let err = result.unwrap_err().to_string(); + assert!( + err.contains("the `elements` array has the incorrect number of elements"), + "{err}" + ); } #[test] @@ -122,7 +141,11 @@ mod tests { &mut SESSION.create_execution_ctx(), ); - assert!(matches!(result, Err(VortexError::InvalidArgument(_, _)))); + let err = result.unwrap_err().to_string(); + assert!( + err.contains("view at index 0 references invalid buffer: 1 out of bounds"), + "{err}" + ); } #[test] @@ -134,7 +157,11 @@ mod tests { let names = ["a", "b"]; let result = StructArray::try_new(names.into(), fields, 3, Validity::NonNullable); - assert!(matches!(result, Err(VortexError::InvalidArgument(_, _)))); + let err = result.unwrap_err().to_string(); + assert!( + err.contains("StructArray field 1 has length 2 but expected 3"), + "{err}" + ); } #[rstest::rstest] diff --git a/vortex-array/src/arrays/varbin/array.rs b/vortex-array/src/arrays/varbin/array.rs index e9f4e913955..768e1790e23 100644 --- a/vortex-array/src/arrays/varbin/array.rs +++ b/vortex-array/src/arrays/varbin/array.rs @@ -236,19 +236,21 @@ impl VarBinData { // Check offsets are non-nullable integer vortex_ensure!( offsets.dtype().is_int() && !offsets.dtype().is_nullable(), - MismatchedTypes: "non nullable int", offsets.dtype() + "expected type: non nullable int but instead got {}", + offsets.dtype() ); // Check dtype is Binary or Utf8 vortex_ensure!( matches!(dtype, DType::Binary(_) | DType::Utf8(_)), - MismatchedTypes: "utf8 or binary", dtype + "expected type: utf8 or binary but instead got {}", + dtype ); // Check nullability matches vortex_ensure!( dtype.is_nullable() != matches!(validity, Validity::NonNullable), - InvalidArgument: "incorrect validity {:?} for dtype {}", + "incorrect validity {:?} for dtype {}", validity, dtype ); @@ -256,7 +258,7 @@ impl VarBinData { // Check offsets has at least one element vortex_ensure!( !offsets.is_empty(), - InvalidArgument: "Offsets must have at least one element" + "Offsets must have at least one element" ); // Check validity length @@ -291,15 +293,15 @@ impl VarBinData { ) -> VortexResult<()> { // Offsets must be sorted (but not strictly sorted, empty values are allowed) let Some(is_sorted) = offsets.statistics().compute_is_sorted(ctx) else { - vortex_bail!(InvalidArgument: "offsets must report is_sorted statistic"); + vortex_bail!("offsets must report is_sorted statistic"); }; - vortex_ensure!(is_sorted, InvalidArgument: "offsets must be sorted"); + vortex_ensure!(is_sorted, "offsets must be sorted"); // Validate that offsets min is non-negative, and max does not exceed the length of // the bytes buffer. let Some(min_max) = min_max(offsets, ctx, NumericalAggregateOpts::default())? else { vortex_bail!( - InvalidArgument: "offsets array with encoding {} must support min_max compute function", + "offsets array with encoding {} must support min_max compute function", offsets.encoding_id() ); }; @@ -320,13 +322,13 @@ impl VarBinData { vortex_ensure!( min >= 0, - InvalidArgument: "offsets minimum {min} outside valid range [0, {max}]" + "offsets minimum {min} outside valid range [0, {max}]" ); // An offset type too narrow for the bytes length cannot exceed it. vortex_ensure!( P::try_from(bytes_len).ok().is_none_or(|len| max <= len), - InvalidArgument: "Max offset {max} is beyond the length of the bytes buffer {bytes_len}" + "Max offset {max} is beyond the length of the bytes buffer {bytes_len}" ); } }); @@ -344,7 +346,7 @@ impl VarBinData { let validate_at = |i: usize, start: usize, end: usize| -> VortexResult<()> { let string_bytes = bytes.get(start..end).ok_or_else(|| { vortex_err!( - InvalidArgument: "offsets {start}..{end} at index {i} are out of order or out of bounds for bytes of length {}", + "offsets {start}..{end} at index {i} are out of order or out of bounds for bytes of length {}", bytes.len() ) })?; @@ -377,7 +379,7 @@ impl VarBinData { let last_offset: usize = offsets_slice[offsets_slice.len() - 1].as_(); vortex_ensure!( last_offset <= bytes.len(), - InvalidArgument: "Last offset {} exceeds bytes length {}", + "Last offset {} exceeds bytes length {}", last_offset, bytes.len() ); diff --git a/vortex-array/src/arrays/varbin/builder.rs b/vortex-array/src/arrays/varbin/builder.rs index 47e7e4e025d..d929c97ca58 100644 --- a/vortex-array/src/arrays/varbin/builder.rs +++ b/vortex-array/src/arrays/varbin/builder.rs @@ -208,7 +208,9 @@ impl VarBinBuilder { Some(value) => self.append_n_values(value, n)?, None => self.push_nulls(n), }, - dtype => vortex_bail!("VarBinBuilder cannot append scalar of dtype {dtype}"), + dtype => { + vortex_bail!("VarBinBuilder cannot append scalar of dtype {dtype}") + } } Ok(()) } diff --git a/vortex-array/src/arrays/varbin/mod.rs b/vortex-array/src/arrays/varbin/mod.rs index 6bc9a4eda82..2f3f3cdc2c3 100644 --- a/vortex-array/src/arrays/varbin/mod.rs +++ b/vortex-array/src/arrays/varbin/mod.rs @@ -26,7 +26,6 @@ pub mod builder; use vortex_buffer::BufferString; use vortex_buffer::ByteBuffer; use vortex_error::VortexExpect; -use vortex_error::vortex_err; use crate::dtype::DType; use crate::scalar::Scalar; @@ -35,7 +34,7 @@ use crate::scalar::ScalarValue; pub fn varbin_scalar(value: ByteBuffer, dtype: &DType) -> Scalar { if matches!(dtype, DType::Utf8(_)) { Scalar::try_utf8(value, dtype.nullability()) - .map_err(|err| vortex_err!("Failed to create scalar from utf8 buffer: {}", err)) + .map_err(|err| err.with_context("Failed to create scalar from utf8 buffer")) .vortex_expect("UTF-8 scalar creation should succeed") } else { Scalar::binary(value, dtype.nullability()) diff --git a/vortex-array/src/arrays/varbinview/array.rs b/vortex-array/src/arrays/varbinview/array.rs index 32ee555ceca..7ecf9163cbf 100644 --- a/vortex-array/src/arrays/varbinview/array.rs +++ b/vortex-array/src/arrays/varbinview/array.rs @@ -177,7 +177,7 @@ impl VarBinViewData { match dtype { DType::Utf8(nullability) => Ok((true, *nullability)), DType::Binary(nullability) => Ok((false, *nullability)), - _ => vortex_bail!(InvalidArgument: "invalid DType {dtype} for `VarBinViewArray`"), + _ => vortex_bail!("invalid DType {dtype} for `VarBinViewArray`"), } } @@ -392,7 +392,7 @@ impl VarBinViewData { vortex_ensure_eq!( validity.nullability(), nullability, - InvalidArgument: "validity nullability is incompatible with dtype nullability", + "validity nullability is incompatible with dtype nullability", ); Ok(is_utf8) } @@ -496,7 +496,7 @@ impl VarBinViewData { let bytes = view.as_inlined().value(); vortex_ensure!( simdutf8::basic::from_utf8(bytes).is_ok(), - InvalidArgument: "view at index {idx}: inlined bytes failed utf-8 validation" + "view at index {idx}: inlined bytes failed utf-8 validation" ); } return Ok(()); @@ -508,25 +508,25 @@ impl VarBinViewData { let end_offset = start_offset.saturating_add(view.size as usize); let buf = buffers.get(buf_index).ok_or_else(|| - vortex_err!(InvalidArgument: "view at index {idx} references invalid buffer: {buf_index} out of bounds for VarBinViewData with {} buffers", + vortex_err!("view at index {idx} references invalid buffer: {buf_index} out of bounds for VarBinViewData with {} buffers", buffers.len()))?; vortex_ensure!( start_offset < buf.len(), - InvalidArgument: "start offset {start_offset} out of bounds for buffer {buf_index} with size {}", + "start offset {start_offset} out of bounds for buffer {buf_index} with size {}", buf.len(), ); vortex_ensure!( end_offset <= buf.len(), - InvalidArgument: "end offset {end_offset} out of bounds for buffer {buf_index} with size {}", + "end offset {end_offset} out of bounds for buffer {buf_index} with size {}", buf.len(), ); // Make sure the prefix data matches the buffer data. vortex_ensure!( view.prefix == buf[start_offset..start_offset + 4], - InvalidArgument: "VarBinView prefix does not match full string" + "VarBinView prefix does not match full string" ); let Some(buffers_utf8) = buffers_utf8 else { @@ -542,7 +542,7 @@ impl VarBinViewData { vortex_ensure!( valid_utf8, - InvalidArgument: "view at index {idx}: outlined bytes fails utf-8 validation" + "view at index {idx}: outlined bytes fails utf-8 validation" ); Ok(()) diff --git a/vortex-array/src/canonical.rs b/vortex-array/src/canonical.rs index 4b437da716a..d154bbc3954 100644 --- a/vortex-array/src/canonical.rs +++ b/vortex-array/src/canonical.rs @@ -256,7 +256,7 @@ impl Canonical { Canonical::Union(UnionArray::empty(variants.clone(), *nullability)) } DType::Variant(_) => { - vortex_panic!(InvalidArgument: "Canonical empty is not supported for Variant") + vortex_panic!("Canonical empty is not supported for Variant") } DType::Extension(ext_dtype) => Canonical::Extension(ExtensionArray::new( ext_dtype.clone(), @@ -1050,7 +1050,9 @@ impl Executable for VariantArray { Ok(variant_array) => Ok(variant_array), Err(array) => match Canonical::execute(array, ctx)? { Canonical::Variant(variant_array) => Ok(variant_array), - canonical => vortex_panic!("Cannot unwrap VariantArray from {:?}", canonical), + canonical => { + vortex_panic!("Cannot unwrap VariantArray from {:?}", canonical) + } }, } } diff --git a/vortex-array/src/compute/conformance/binary_numeric.rs b/vortex-array/src/compute/conformance/binary_numeric.rs index 29f3c3a6531..9b87f5fea2c 100644 --- a/vortex-array/src/compute/conformance/binary_numeric.rs +++ b/vortex-array/src/compute/conformance/binary_numeric.rs @@ -327,8 +327,9 @@ fn assert_decimal_results( let result = result.unwrap_or_else(|err| { vortex_panic!( + err, "Decimal binary numeric operation unexpectedly failed for encoding {}: \ - {operator:?} {scalar} (lhs_is_array: {lhs_is_array}): {err}", + {operator:?} {scalar} (lhs_is_array: {lhs_is_array})", array.encoding_id(), ) }); diff --git a/vortex-array/src/compute/conformance/cast.rs b/vortex-array/src/compute/conformance/cast.rs index 32d194edf41..175ece9e524 100644 --- a/vortex-array/src/compute/conformance/cast.rs +++ b/vortex-array/src/compute/conformance/cast.rs @@ -288,7 +288,8 @@ fn test_cast_to_primitive( let target_dtype = DType::Primitive(target_ptype, array.dtype().nullability()); let casted = cast_and_execute(array, target_dtype.clone(), ctx).unwrap_or_else(|e| { vortex_panic!( - "Cast must succeed because all values are within bounds. {} {}: {e}", + e, + "Cast must succeed because all values are within bounds. {} {}", target_ptype, array.display_values(ctx), ) diff --git a/vortex-array/src/compute/conformance/consistency.rs b/vortex-array/src/compute/conformance/consistency.rs index 6d1b919e51d..e83b4867eb9 100644 --- a/vortex-array/src/compute/conformance/consistency.rs +++ b/vortex-array/src/compute/conformance/consistency.rs @@ -1047,7 +1047,9 @@ fn slice_cast_targets(dtype: &DType) -> Vec { vec![DType::Struct(fields.clone(), opposite)] } DType::Union(..) => vortex_panic!("TODO(connor)[Union]: unimplemented"), - DType::Variant(_) => vortex_panic!("Variant conformance casting is not implemented"), + DType::Variant(_) => { + vortex_panic!("Variant conformance casting is not implemented") + } DType::Extension(_) => vec![], // Extension types typically only cast to themselves } } diff --git a/vortex-array/src/dtype/decimal/types.rs b/vortex-array/src/dtype/decimal/types.rs index 4aec9a8fb5b..f83a3918a86 100644 --- a/vortex-array/src/dtype/decimal/types.rs +++ b/vortex-array/src/dtype/decimal/types.rs @@ -256,7 +256,9 @@ impl TryFrom for DecimalType { | PType::U64 | PType::F16 | PType::F32 - | PType::F64) => vortex_bail!("cannot convert ptype {p} to DecimalType"), + | PType::F64) => { + vortex_bail!("cannot convert ptype {p} to DecimalType") + } }) } } diff --git a/vortex-array/src/dtype/field_mask.rs b/vortex-array/src/dtype/field_mask.rs index 7920746f830..006768379d9 100644 --- a/vortex-array/src/dtype/field_mask.rs +++ b/vortex-array/src/dtype/field_mask.rs @@ -49,7 +49,9 @@ impl FieldMask { /// Returns the first field explicit select mask, if there is one, failing if mask = `All`. pub fn starting_field(&self) -> VortexResult> { match self { - FieldMask::All => vortex_bail!("Cannot get starting field from All mask"), + FieldMask::All => { + vortex_bail!("Cannot get starting field from All mask") + } // We know that fp is non-empty FieldMask::Prefix(fp) | FieldMask::Exact(fp) => Ok(fp.parts().first()), } diff --git a/vortex-array/src/dtype/serde/flatbuffers.rs b/vortex-array/src/dtype/serde/flatbuffers.rs index baabcf10ce4..1f4b36acf23 100644 --- a/vortex-array/src/dtype/serde/flatbuffers.rs +++ b/vortex-array/src/dtype/serde/flatbuffers.rs @@ -296,8 +296,7 @@ impl TryFrom for DType { vortex_err!("failed to parse extension id from flatbuffer") })?); let storage_dtype = fb_ext.storage_dtype().ok_or_else(|| { - vortex_err!( - Serde: "storage_dtype must be present on DType fbs message") + vortex_err!("storage_dtype must be present on DType fbs message") })?; let storage_view = ViewedDType::from_fb_loc( storage_dtype._tab.loc(), @@ -305,7 +304,7 @@ impl TryFrom for DType { vfdt.session.clone(), ); let storage_dtype = DType::try_from(storage_view) - .map_err(|e| vortex_err!("failed to create DType from fbs message: {e}"))?; + .map_err(|e| e.with_context("failed to create DType from fbs message"))?; let metadata = fb_ext .metadata() @@ -556,7 +555,7 @@ impl TryFrom for PType { fb::PType::F16 => Self::F16, fb::PType::F32 => Self::F32, fb::PType::F64 => Self::F64, - _ => vortex_bail!(Serde: "Unknown PType variant"), + _ => vortex_bail!("Unknown PType variant"), }) } } diff --git a/vortex-array/src/dtype/serde/proto.rs b/vortex-array/src/dtype/serde/proto.rs index 99a3fd7801f..d6d4d31e0c8 100644 --- a/vortex-array/src/dtype/serde/proto.rs +++ b/vortex-array/src/dtype/serde/proto.rs @@ -29,7 +29,7 @@ impl DType { match value .dtype_type .as_ref() - .ok_or_else(|| vortex_err!(Serde: "Unrecognized DType"))? + .ok_or_else(|| vortex_err!("Unrecognized DType"))? { DtypeType::Null(_) => Ok(Self::Null), DtypeType::Bool(b) => Ok(Self::Bool(b.nullable.into())), @@ -53,7 +53,7 @@ impl DType { DType::from_proto( l.element_type .as_ref() - .ok_or_else(|| vortex_err!(Serde: "Invalid list element type"))? + .ok_or_else(|| vortex_err!("Invalid list element type"))? .as_ref(), session, ) @@ -67,9 +67,7 @@ impl DType { DType::from_proto( fsl.element_type .as_ref() - .ok_or_else( - || vortex_err!(Serde: "Invalid fixed-size list element type"), - )? + .ok_or_else(|| vortex_err!("Invalid fixed-size list element type"))? .as_ref(), session, ) @@ -83,14 +81,14 @@ impl DType { DType::from_proto( map.key_type .as_ref() - .ok_or_else(|| vortex_err!(Serde: "Invalid map key type"))? + .ok_or_else(|| vortex_err!("Invalid map key type"))? .as_ref(), session, )?, DType::from_proto( map.value_type .as_ref() - .ok_or_else(|| vortex_err!(Serde: "Invalid map value type"))? + .ok_or_else(|| vortex_err!("Invalid map value type"))? .as_ref(), session, )?, @@ -142,7 +140,7 @@ impl DType { } else if session.allows_unknown() { ForeignExtDType::from_parts(id, e.metadata().to_vec(), storage_dtype)? } else { - return Err(vortex_err!(Serde: "Unregistered extension type ID: {}", e.id)); + return Err(vortex_err!("Unregistered extension type ID: {}", e.id)); }; Ok(Self::Extension(ext_dtype)) } @@ -273,7 +271,7 @@ impl TryFrom<&pb::FieldPath> for FieldPath { match field .field_type .as_ref() - .ok_or_else(|| vortex_err!(Serde: "FieldPath part missing type"))? + .ok_or_else(|| vortex_err!("FieldPath part missing type"))? { FieldType::Name(name) => path.push(Field::from(name.as_str())), } diff --git a/vortex-array/src/executor.rs b/vortex-array/src/executor.rs index afc5464b03e..b9e22a41e62 100644 --- a/vortex-array/src/executor.rs +++ b/vortex-array/src/executor.rs @@ -53,8 +53,8 @@ use crate::trace_op; /// Returns the maximum number of iterations to attempt when executing an array before giving up and returning /// an error, can be by the `VORTEX_MAX_ITERATIONS` env variables, otherwise defaults to 2^22. pub(crate) fn max_iterations() -> usize { - static MAX_ITERATIONS: LazyLock = - LazyLock::new(|| match std::env::var("VORTEX_MAX_ITERATIONS") { + static MAX_ITERATIONS: LazyLock = LazyLock::new(|| { + match std::env::var("VORTEX_MAX_ITERATIONS") { Ok(val) => val.parse::().unwrap_or_else(|e| { vortex_panic!("VORTEX_MAX_ITERATIONS is not a valid usize: {e}") }), @@ -62,7 +62,8 @@ pub(crate) fn max_iterations() -> usize { Err(VarError::NotUnicode(_)) => { vortex_panic!("VORTEX_MAX_ITERATIONS is not a valid unicode string") } - }); + } + }); *MAX_ITERATIONS } diff --git a/vortex-array/src/extension/datetime/time.rs b/vortex-array/src/extension/datetime/time.rs index 750a1a183f7..2645f9fd2e9 100644 --- a/vortex-array/src/extension/datetime/time.rs +++ b/vortex-array/src/extension/datetime/time.rs @@ -136,7 +136,9 @@ impl ExtVTable for Time { Span::new().nanoseconds(length_of_time), TimeValue::Nanoseconds(length_of_time), ), - d @ TimeUnit::Days => vortex_bail!("Time type does not support time unit {d}"), + d @ TimeUnit::Days => { + vortex_bail!("Time type does not support time unit {d}") + } }; // Validate the storage value is within the valid range for Time. diff --git a/vortex-array/src/extension/datetime/timestamp.rs b/vortex-array/src/extension/datetime/timestamp.rs index 7c0cdaeb1bb..42f00776216 100644 --- a/vortex-array/src/extension/datetime/timestamp.rs +++ b/vortex-array/src/extension/datetime/timestamp.rs @@ -228,7 +228,9 @@ impl ExtVTable for Timestamp { Span::new().seconds(ts_value), TimestampValue::Seconds(ts_value, tz), ), - TimeUnit::Days => vortex_bail!("Timestamp does not support Days time unit"), + TimeUnit::Days => { + vortex_bail!("Timestamp does not support Days time unit") + } }; // Validate the storage value is within the valid range for Timestamp. @@ -238,7 +240,7 @@ impl ExtVTable for Timestamp { if let Some(tz) = tz { resolve_time_zone(Some(tz)) - .map_err(|e| vortex_err!("Invalid timezone for timestamp scalar: {}", e))?; + .map_err(|e| e.with_context("Invalid timezone for timestamp scalar"))?; } Ok(value) diff --git a/vortex-array/src/normalize.rs b/vortex-array/src/normalize.rs index 302ae3d1f47..1204f466c20 100644 --- a/vortex-array/src/normalize.rs +++ b/vortex-array/src/normalize.rs @@ -43,7 +43,7 @@ impl ArrayRef { fn normalize_with_error(&self, allowed: &HashSet) -> VortexResult<()> { if !self.is_allowed_encoding(allowed) { - vortex_bail!(AssertionFailed: "normalize forbids encoding ({})", self.encoding_id()) + vortex_bail!("normalize forbids encoding ({})", self.encoding_id()) } for child in self.children() { diff --git a/vortex-array/src/scalar/cast.rs b/vortex-array/src/scalar/cast.rs index 1a2fd1b1021..8e57cb68fff 100644 --- a/vortex-array/src/scalar/cast.rs +++ b/vortex-array/src/scalar/cast.rs @@ -80,7 +80,9 @@ impl Scalar { "union scalar cast from {} to {target_dtype} is not supported (yet)", self.dtype() ), - DType::Variant(_) => vortex_bail!("Variant scalars can't be cast to {target_dtype}"), + DType::Variant(_) => { + vortex_bail!("Variant scalars can't be cast to {target_dtype}") + } DType::Extension(..) => self.as_extension().cast(target_dtype), } } diff --git a/vortex-array/src/scalar/proto.rs b/vortex-array/src/scalar/proto.rs index fa9e60812fe..2c29e08a37f 100644 --- a/vortex-array/src/scalar/proto.rs +++ b/vortex-array/src/scalar/proto.rs @@ -199,14 +199,14 @@ impl Scalar { value .dtype .as_ref() - .ok_or_else(|| vortex_err!(Serde: "Scalar missing dtype"))?, + .ok_or_else(|| vortex_err!("Scalar missing dtype"))?, session, )?; let pb_scalar_value: &pb::ScalarValue = value .value .as_ref() - .ok_or_else(|| vortex_err!(Serde: "Scalar missing value"))?; + .ok_or_else(|| vortex_err!("Scalar missing value"))?; let value: Option = ScalarValue::from_proto(pb_scalar_value, &dtype, session)?; @@ -248,7 +248,7 @@ impl ScalarValue { let kind = value .kind .as_ref() - .ok_or_else(|| vortex_err!(Serde: "Scalar value missing kind"))?; + .ok_or_else(|| vortex_err!("Scalar value missing kind"))?; // `DType::Extension` store their serialized values using the storage `DType`. let dtype = match dtype { @@ -272,7 +272,7 @@ impl ScalarValue { DType::Variant(_) => Some(ScalarValue::Variant(Box::new(Scalar::from_proto( v, session, )?))), - _ => vortex_bail!(Serde: "expected non-Variant scalar proto for dtype {dtype}"), + _ => vortex_bail!("expected non-Variant scalar proto for dtype {dtype}"), }, }) } @@ -282,7 +282,7 @@ impl ScalarValue { fn bool_from_proto(v: bool, dtype: &DType) -> VortexResult { vortex_ensure!( dtype.is_boolean(), - Serde: "expected Bool dtype for BoolValue, got {dtype}" + "expected Bool dtype for BoolValue, got {dtype}" ); Ok(ScalarValue::Bool(v)) @@ -295,7 +295,7 @@ fn bool_from_proto(v: bool, dtype: &DType) -> VortexResult { fn int64_from_proto(v: i64, dtype: &DType) -> VortexResult { vortex_ensure!( dtype.is_primitive(), - Serde: "expected Primitive dtype for Int64Value, got {dtype}" + "expected Primitive dtype for Int64Value, got {dtype}" ); let pvalue = match dtype.as_ptype() { @@ -310,10 +310,10 @@ fn int64_from_proto(v: i64, dtype: &DType) -> VortexResult { PType::U32 => v.to_u32().map(PValue::U32), PType::U64 => v.to_u64().map(PValue::U64), ftype @ (PType::F16 | PType::F32 | PType::F64) => vortex_bail!( - Serde: "expected signed integer ptype for serialized Int64Value, got float {ftype}" + "expected signed integer ptype for serialized Int64Value, got float {ftype}" ), } - .ok_or_else(|| vortex_err!(Serde: "Int64 value {v} out of range for dtype {dtype}"))?; + .ok_or_else(|| vortex_err!("Int64 value {v} out of range for dtype {dtype}"))?; Ok(ScalarValue::Primitive(pvalue)) } @@ -326,7 +326,7 @@ fn int64_from_proto(v: i64, dtype: &DType) -> VortexResult { fn uint64_from_proto(v: u64, dtype: &DType) -> VortexResult { vortex_ensure!( dtype.is_primitive(), - Serde: "expected Primitive dtype for Uint64Value, got {dtype}" + "expected Primitive dtype for Uint64Value, got {dtype}" ); let pvalue = match dtype.as_ptype() { @@ -342,11 +342,11 @@ fn uint64_from_proto(v: u64, dtype: &DType) -> VortexResult { PType::I64 => v.to_i64().map(PValue::I64), // f16 values used to be serialized as u64, so we need to be able to read an f16 from a u64. PType::F16 => v.to_u16().map(f16::from_bits).map(PValue::F16), - ftype @ (PType::F32 | PType::F64) => vortex_bail!( - Serde: "expected unsigned integer ptype for serialized Uint64Value, got {ftype}" - ), + ftype @ (PType::F32 | PType::F64) => { + vortex_bail!("expected unsigned integer ptype for serialized Uint64Value, got {ftype}") + } } - .ok_or_else(|| vortex_err!(Serde: "Uint64 value {v} out of range for dtype {dtype}"))?; + .ok_or_else(|| vortex_err!("Uint64 value {v} out of range for dtype {dtype}"))?; Ok(ScalarValue::Primitive(pvalue)) } @@ -355,11 +355,11 @@ fn uint64_from_proto(v: u64, dtype: &DType) -> VortexResult { fn f16_from_proto(v: u64, dtype: &DType) -> VortexResult { vortex_ensure!( matches!(dtype, DType::Primitive(PType::F16, _)), - Serde: "expected F16 dtype for F16Value, got {dtype}" + "expected F16 dtype for F16Value, got {dtype}" ); let bits = u16::try_from(v) - .map_err(|_| vortex_err!(Serde: "f16 bitwise representation has more than 16 bits: {v}"))?; + .map_err(|_| vortex_err!("f16 bitwise representation has more than 16 bits: {v}"))?; Ok(ScalarValue::Primitive(PValue::F16(f16::from_bits(bits)))) } @@ -368,7 +368,7 @@ fn f16_from_proto(v: u64, dtype: &DType) -> VortexResult { fn f32_from_proto(v: f32, dtype: &DType) -> VortexResult { vortex_ensure!( matches!(dtype, DType::Primitive(PType::F32, _)), - Serde: "expected F32 dtype for F32Value, got {dtype}" + "expected F32 dtype for F32Value, got {dtype}" ); Ok(ScalarValue::Primitive(PValue::F32(v))) @@ -378,7 +378,7 @@ fn f32_from_proto(v: f32, dtype: &DType) -> VortexResult { fn f64_from_proto(v: f64, dtype: &DType) -> VortexResult { vortex_ensure!( matches!(dtype, DType::Primitive(PType::F64, _)), - Serde: "expected F64 dtype for F64Value, got {dtype}" + "expected F64 dtype for F64Value, got {dtype}" ); Ok(ScalarValue::Primitive(PValue::F64(v))) @@ -390,9 +390,7 @@ fn string_from_proto(s: &str, dtype: &DType) -> VortexResult { match dtype { DType::Utf8(_) => Ok(ScalarValue::Utf8(BufferString::from(s))), DType::Binary(_) => Ok(ScalarValue::Binary(ByteBuffer::copy_from(s.as_bytes()))), - _ => vortex_bail!( - Serde: "expected Utf8 or Binary dtype for StringValue, got {dtype}" - ), + _ => vortex_bail!("expected Utf8 or Binary dtype for StringValue, got {dtype}"), } } @@ -437,11 +435,9 @@ fn bytes_from_proto(bytes: &[u8], dtype: &DType) -> VortexResult { .ok() .vortex_expect("Buffer has invalid number of bytes"), )), - l => vortex_bail!(Serde: "invalid decimal byte length: {l}"), + l => vortex_bail!("invalid decimal byte length: {l}"), })), - _ => vortex_bail!( - Serde: "expected Utf8, Binary, or Decimal dtype for BytesValue, got {dtype}" - ), + _ => vortex_bail!("expected Utf8, Binary, or Decimal dtype for BytesValue, got {dtype}"), } } @@ -461,7 +457,7 @@ fn list_from_proto( vortex_ensure_eq!( v.values.len(), fields.nfields(), - Serde: "ListValue has the wrong number of struct fields" + "ListValue has the wrong number of struct fields" ); v.values @@ -477,9 +473,7 @@ fn list_from_proto( .map(|entry| ScalarValue::from_proto(entry, &entry_dtype, session)) .collect::>>()? } - _ => vortex_bail!( - Serde: "expected a tuple-backed dtype for ListValue, got {dtype}" - ), + _ => vortex_bail!("expected a tuple-backed dtype for ListValue, got {dtype}"), }; Ok(ScalarValue::Tuple(values)) } @@ -491,34 +485,33 @@ fn union_from_proto( session: &VortexSession, ) -> VortexResult { let DType::Union(variants, _) = dtype else { - vortex_bail!(Serde: "expected Union dtype for UnionValue, got {dtype}"); + vortex_bail!("expected Union dtype for UnionValue, got {dtype}"); }; - let type_id = u8::try_from(value.type_id).map_err( - |_| vortex_err!(Serde: "union type ID {} is outside the u8 range", value.type_id), - )?; + let type_id = u8::try_from(value.type_id) + .map_err(|_| vortex_err!("union type ID {} is outside the u8 range", value.type_id))?; let child_index = variants.tag_to_child_index(type_id).ok_or_else(|| { vortex_err!( - Serde: "union type ID {type_id} is not present in {:?}", + "union type ID {type_id} is not present in {:?}", variants.type_ids() ) })?; let child_dtype = variants .variant_by_index(child_index) - .ok_or_else(|| vortex_err!(Serde: "union type ID {type_id} resolved out of bounds"))?; + .ok_or_else(|| vortex_err!("union type ID {type_id} resolved out of bounds"))?; let child_proto = value .value .as_deref() - .ok_or_else(|| vortex_err!(Serde: "UnionValue missing child value"))?; + .ok_or_else(|| vortex_err!("UnionValue missing child value"))?; let child_value = ScalarValue::from_proto(child_proto, &child_dtype, session)?; Scalar::validate(&child_dtype, child_value.as_ref()).map_err(|error| { - vortex_err!( - Serde: "union type ID {type_id} has invalid child for dtype {child_dtype}: {error}" - ) + error.with_context(format!( + "union type ID {type_id} has invalid child for dtype {child_dtype}" + )) })?; Ok(ScalarValue::Union(UnionValue::new(type_id, child_value))) @@ -531,7 +524,6 @@ mod tests { use std::sync::Arc; use vortex_buffer::BufferString; - use vortex_error::VortexError; use vortex_error::vortex_panic; use vortex_session::VortexSession; @@ -865,10 +857,10 @@ mod tests { }))), }; - assert!(matches!( - ScalarValue::from_proto(&missing_child, &dtype, &session()), - Err(VortexError::Serde(..)) - )); + let err = ScalarValue::from_proto(&missing_child, &dtype, &session()) + .unwrap_err() + .to_string(); + assert!(err.contains("UnionValue missing child value"), "{err}"); let wrong_child_value = pb::ScalarValue { kind: Some(Kind::UnionValue(Box::new(PbUnionValue { @@ -879,10 +871,13 @@ mod tests { }))), }; - assert!(matches!( - ScalarValue::from_proto(&wrong_child_value, &dtype, &session()), - Err(VortexError::Serde(..)) - )); + let err = ScalarValue::from_proto(&wrong_child_value, &dtype, &session()) + .unwrap_err() + .to_string(); + assert!( + err.contains("expected Utf8 or Binary dtype for StringValue, got i32?"), + "{err}" + ); Ok(()) } @@ -1087,14 +1082,18 @@ mod tests { PValue::U16(v) => *v as u64, PValue::U32(v) => *v as u64, PValue::U64(v) => *v, - _ => vortex_panic!("Unexpected primitive type for {name}: {pv:?}"), + _ => { + vortex_panic!("Unexpected primitive type for {name}: {pv:?}") + } }; assert_eq!( v, expected, "ScalarValue {name} value not preserved: expected {expected}, got {v}" ); } - _ => vortex_panic!("Unexpected type after roundtrip for {name}: {read_back:?}"), + _ => { + vortex_panic!("Unexpected type after roundtrip for {name}: {read_back:?}") + } } } @@ -1132,14 +1131,18 @@ mod tests { PValue::I16(v) => *v as i64, PValue::I32(v) => *v as i64, PValue::I64(v) => *v, - _ => vortex_panic!("Unexpected primitive type for {name}: {pv:?}"), + _ => { + vortex_panic!("Unexpected primitive type for {name}: {pv:?}") + } }; assert_eq!( v, expected, "ScalarValue {name} value not preserved: expected {expected}, got {v}" ); } - _ => vortex_panic!("Unexpected type after roundtrip for {name}: {read_back:?}"), + _ => { + vortex_panic!("Unexpected type after roundtrip for {name}: {read_back:?}") + } } } } diff --git a/vortex-array/src/scalar/scalar_impl.rs b/vortex-array/src/scalar/scalar_impl.rs index 4b9b63a2713..8f3e58bd0e8 100644 --- a/vortex-array/src/scalar/scalar_impl.rs +++ b/vortex-array/src/scalar/scalar_impl.rs @@ -219,7 +219,9 @@ impl Scalar { let value = self.value()?; let is_zero = match self.dtype() { - DType::Null => vortex_panic!("non-null value somehow had `DType::Null`"), + DType::Null => { + vortex_panic!("non-null value somehow had `DType::Null`") + } DType::Bool(_) => !value.as_bool(), DType::Primitive(..) => value.as_primitive().is_zero(), DType::Decimal(..) => value.as_decimal().is_zero(), diff --git a/vortex-array/src/scalar/typed_view/decimal/arithmetic.rs b/vortex-array/src/scalar/typed_view/decimal/arithmetic.rs index 2a47c9cf977..ec7b57f538c 100644 --- a/vortex-array/src/scalar/typed_view/decimal/arithmetic.rs +++ b/vortex-array/src/scalar/typed_view/decimal/arithmetic.rs @@ -88,10 +88,7 @@ pub(crate) fn decimal_numeric_result_dtype( let result_precision = >::from(input.precision()) + >::from(result_scale); let result_precision = u8::try_from(result_precision).map_err(|_| { - vortex_err!( - InvalidArgument: - "decimal division result precision {result_precision} is invalid" - ) + vortex_err!("decimal division result precision {result_precision} is invalid") })?; DecimalDType::try_new(result_precision.min(MAX_PRECISION), result_scale) } diff --git a/vortex-array/src/scalar/typed_view/primitive/numeric_operator.rs b/vortex-array/src/scalar/typed_view/primitive/numeric_operator.rs index d7bcc8d0b4c..7a52a8ce485 100644 --- a/vortex-array/src/scalar/typed_view/primitive/numeric_operator.rs +++ b/vortex-array/src/scalar/typed_view/primitive/numeric_operator.rs @@ -51,7 +51,7 @@ impl TryFrom for NumericOperator { Operator::Sub => Ok(NumericOperator::Sub), Operator::Mul => Ok(NumericOperator::Mul), Operator::Div => Ok(NumericOperator::Div), - _ => Err(vortex_err!(InvalidArgument: "{op} is not a numeric operator")), + _ => Err(vortex_err!("{op} is not a numeric operator")), } } } diff --git a/vortex-array/src/scalar/validate.rs b/vortex-array/src/scalar/validate.rs index 1c9cf50e558..274adc4ca66 100644 --- a/vortex-array/src/scalar/validate.rs +++ b/vortex-array/src/scalar/validate.rs @@ -185,7 +185,7 @@ impl Scalar { for (i, element) in elements.iter().enumerate() { Self::validate_detailed(elem_dtype.as_ref(), element.as_ref()) - .map_err(|e| vortex_error::vortex_err!("list element at index {i}: {e}"))?; + .map_err(|e| e.with_context(format!("list element at index {i}")))?; } } DType::FixedSizeList(elem_dtype, size, _) => { @@ -202,7 +202,7 @@ impl Scalar { for (i, element) in elements.iter().enumerate() { Self::validate_detailed(elem_dtype.as_ref(), element.as_ref()).map_err( - |e| vortex_error::vortex_err!("fixed-size list element at index {i}: {e}",), + |e| e.with_context(format!("fixed-size list element at index {i}")), )?; } } @@ -228,11 +228,10 @@ impl Scalar { "map entry at index {index} has the wrong number of values", ); - Self::validate_detailed(&key_dtype, values[0].as_ref()).map_err(|error| { - vortex_error::vortex_err!("map key at entry {index}: {error}") - })?; + Self::validate_detailed(&key_dtype, values[0].as_ref()) + .map_err(|error| error.with_context(format!("map key at entry {index}")))?; Self::validate_detailed(&value_dtype, values[1].as_ref()).map_err(|error| { - vortex_error::vortex_err!("map value at entry {index}: {error}") + error.with_context(format!("map value at entry {index}")) })?; } } @@ -272,10 +271,9 @@ impl Scalar { Self::validate_detailed(&child_dtype, union_value.child_value()).map_err( |error| { - vortex_error::vortex_err!( - "union value for type ID {type_id} is invalid for dtype {child_dtype}: \ - {error}" - ) + error.with_context(format!( + "union value for type ID {type_id} is invalid for dtype {child_dtype}" + )) }, )?; } diff --git a/vortex-array/src/scalar_fn/fns/binary/compare/bytes.rs b/vortex-array/src/scalar_fn/fns/binary/compare/bytes.rs index ef833adf6c4..7dc45fac2c2 100644 --- a/vortex-array/src/scalar_fn/fns/binary/compare/bytes.rs +++ b/vortex-array/src/scalar_fn/fns/binary/compare/bytes.rs @@ -76,7 +76,9 @@ fn constant_bytes(scalar: &Scalar) -> VortexResult> { .value() .map(|s| s.as_str().as_bytes().to_vec()), DType::Binary(_) => scalar.as_binary().value().map(|b| b.to_vec()), - _ => vortex_bail!("expected utf8 or binary scalar, got {}", scalar.dtype()), + _ => { + vortex_bail!("expected utf8 or binary scalar, got {}", scalar.dtype()) + } }; value.ok_or_else(|| vortex_err!("null constant handled by execute_compare")) } diff --git a/vortex-array/src/scalar_fn/fns/binary/numeric/decimal.rs b/vortex-array/src/scalar_fn/fns/binary/numeric/decimal.rs index 86b3891c50b..8742ea66a93 100644 --- a/vortex-array/src/scalar_fn/fns/binary/numeric/decimal.rs +++ b/vortex-array/src/scalar_fn/fns/binary/numeric/decimal.rs @@ -255,10 +255,7 @@ where ::from(1_i8).vortex_expect("one fits every decimal working width"); for _ in 0..exp { factor = factor.checked_mul(&ten).ok_or_else(|| { - vortex_err!( - InvalidArgument: - "decimal scale factor 10^{exp} cannot be represented at the working width" - ) + vortex_err!("decimal scale factor 10^{exp} cannot be represented at the working width") })?; } Ok(factor) @@ -382,8 +379,8 @@ where ) => { let lhs = typed_constant::(lhs); let rhs = typed_constant::(rhs); - let value = Op::apply(lhs, rhs, constants) - .ok_or_else(|| vortex_err!(InvalidArgument: "{}", Op::ERROR))?; + let value = + Op::apply(lhs, rhs, constants).ok_or_else(|| vortex_err!("{}", Op::ERROR))?; let value = DecimalValue::from(value) .normalize(result_decimal_dtype) .vortex_expect("bounds-checked result fits the result precision"); @@ -394,7 +391,7 @@ where .into_array()); } } - .map_err(|_lane| vortex_err!(InvalidArgument: "{}", Op::ERROR))?; + .map_err(|_lane| vortex_err!("{}", Op::ERROR))?; Ok(decimal_array_narrowed( values, diff --git a/vortex-array/src/scalar_fn/fns/binary/numeric/row.rs b/vortex-array/src/scalar_fn/fns/binary/numeric/row.rs index ff7fbe7ab81..e750276e543 100644 --- a/vortex-array/src/scalar_fn/fns/binary/numeric/row.rs +++ b/vortex-array/src/scalar_fn/fns/binary/numeric/row.rs @@ -131,5 +131,5 @@ where #[cold] #[inline(never)] fn numeric_error(message: &'static str) -> VortexError { - vortex_err!(InvalidArgument: "{message}") + vortex_err!("{message}") } diff --git a/vortex-array/src/scalar_fn/fns/byte_length.rs b/vortex-array/src/scalar_fn/fns/byte_length.rs index afdf77d118d..4a81f68681b 100644 --- a/vortex-array/src/scalar_fn/fns/byte_length.rs +++ b/vortex-array/src/scalar_fn/fns/byte_length.rs @@ -98,7 +98,9 @@ impl ScalarFnVTable for ByteLength { DType::Utf8(nullable) | DType::Binary(nullable) => { Ok(DType::Primitive(PType::U64, *nullable)) } - other => vortex_bail!("byte_length() requires Utf8 or Binary, got {other}"), + other => { + vortex_bail!("byte_length() requires Utf8 or Binary, got {other}") + } } } @@ -118,7 +120,9 @@ impl ScalarFnVTable for ByteLength { match input.dtype() { DType::Utf8(_) | DType::Binary(_) => byte_length(&input, nullability, ctx), - other => vortex_bail!("byte_length() requires Utf8 or Binary, got {other}"), + other => { + vortex_bail!("byte_length() requires Utf8 or Binary, got {other}") + } } } @@ -147,7 +151,9 @@ fn scalar_byte_length(scalar: &Scalar, nullability: Nullability) -> VortexResult .value() .vortex_expect("null binary scalar") .len(), - other => vortex_bail!("byte_length() requires Utf8 or Binary, got {other}"), + other => { + vortex_bail!("byte_length() requires Utf8 or Binary, got {other}") + } }; let len: u64 = len.as_(); Ok(Scalar::primitive(len, nullability)) diff --git a/vortex-array/src/scalar_fn/fns/list_length.rs b/vortex-array/src/scalar_fn/fns/list_length.rs index f88e2baf80d..a456511de14 100644 --- a/vortex-array/src/scalar_fn/fns/list_length.rs +++ b/vortex-array/src/scalar_fn/fns/list_length.rs @@ -78,7 +78,9 @@ impl ScalarFnVTable for ListLength { DType::List(_, nullable) | DType::FixedSizeList(_, _, nullable) => { Ok(DType::Primitive(PType::U64, *nullable)) } - other => vortex_bail!("list_length() requires List or FixedSizeList, got {other}"), + other => { + vortex_bail!("list_length() requires List or FixedSizeList, got {other}") + } } } diff --git a/vortex-array/src/scalar_fn/fns/list_sum.rs b/vortex-array/src/scalar_fn/fns/list_sum.rs index 0e83a22ed3b..c5818ef012e 100644 --- a/vortex-array/src/scalar_fn/fns/list_sum.rs +++ b/vortex-array/src/scalar_fn/fns/list_sum.rs @@ -72,7 +72,9 @@ impl ScalarFnVTable for ListSum { fn return_dtype(&self, options: &Self::Options, arg_dtypes: &[DType]) -> VortexResult { let elem_dtype = match &arg_dtypes[0] { DType::List(elem, _) | DType::FixedSizeList(elem, ..) => elem.as_ref(), - other => vortex_bail!("list_sum() requires List or FixedSizeList, got {other}"), + other => { + vortex_bail!("list_sum() requires List or FixedSizeList, got {other}") + } }; SumV2 .return_dtype(options, elem_dtype) @@ -89,7 +91,9 @@ impl ScalarFnVTable for ListSum { let elem_dtype = match input.dtype() { DType::List(elem, _) | DType::FixedSizeList(elem, ..) => elem.as_ref().clone(), - other => vortex_bail!("list_sum() requires List or FixedSizeList, got {other}"), + other => { + vortex_bail!("list_sum() requires List or FixedSizeList, got {other}") + } }; let columnar = input.execute::(ctx)?; diff --git a/vortex-array/src/scalar_fn/fns/operators.rs b/vortex-array/src/scalar_fn/fns/operators.rs index aa209531e46..3fdc921e39d 100644 --- a/vortex-array/src/scalar_fn/fns/operators.rs +++ b/vortex-array/src/scalar_fn/fns/operators.rs @@ -273,9 +273,9 @@ impl TryFrom for CompareOperator { type Error = VortexError; fn try_from(value: Operator) -> Result { - value.to_compare_operator().ok_or_else( - || vortex_error::vortex_err!(InvalidArgument: "{value} is not a comparison operator"), - ) + value + .to_compare_operator() + .ok_or_else(|| vortex_error::vortex_err!("{value} is not a comparison operator")) } } diff --git a/vortex-array/src/scalar_fn/unstable/row/batch/tests.rs b/vortex-array/src/scalar_fn/unstable/row/batch/tests.rs index 532bd30a6f7..22309a0bd9c 100644 --- a/vortex-array/src/scalar_fn/unstable/row/batch/tests.rs +++ b/vortex-array/src/scalar_fn/unstable/row/batch/tests.rs @@ -347,7 +347,6 @@ impl RowFn for RepeatValue { ) -> VortexResult { vortex_ensure!( u32::try_from(*width).is_ok(), - InvalidArgument: "test.repeat_value width must fit in the fixed-size-list u32 list size, got {width}", ); @@ -384,7 +383,7 @@ impl RowFn for DeferredAdd { |&(), (lhs, rhs)| lhs.overflowing_add(rhs), |overflowed| { if overflowed { - vortex_bail!(InvalidArgument: "deferred addition overflowed"); + vortex_bail!("deferred addition overflowed"); } Ok(()) @@ -459,7 +458,7 @@ impl RowFn for DenseRetryIncrement { |(value,)| value.overflowing_add(1), |overflowed| { if overflowed { - vortex_bail!(InvalidArgument: "deferred increment overflowed"); + vortex_bail!("deferred increment overflowed"); } Ok(()) @@ -487,7 +486,6 @@ impl RowFn for FilteredRepeat { ) -> VortexResult { vortex_ensure!( u32::try_from(*width).is_ok(), - InvalidArgument: "test.filtered_repeat width must fit in u32, got {width}", ); @@ -563,8 +561,7 @@ fn test_fixed_size_list_sink_uses_runtime_width( let actual = execute_rows(&RepeatValue, &width, &args, &mut ctx)?; let expected = FixedSizeListArray::new( PrimitiveArray::from_iter(expected_elements).into_array(), - u32::try_from(width) - .map_err(|_| vortex_err!(InvalidArgument: "test width must fit in u32, got {width}"))?, + u32::try_from(width).map_err(|_| vortex_err!("test width must fit in u32, got {width}"))?, validity, row_count, ) @@ -637,7 +634,7 @@ impl RowFn for DeferredGreaterThan { |(lhs, rhs)| (lhs > rhs, lhs == i64::MIN), |failed| { if failed { - vortex_bail!(InvalidArgument: "deferred comparison failed"); + vortex_bail!("deferred comparison failed"); } Ok(()) @@ -728,7 +725,9 @@ fn test_kernel_output_rejects_nulls_at_function_boundary() -> VortexResult<()> { Err(error) => error, Ok(output) => match output.execute::(&mut ctx) { Err(error) => error, - Ok(_) => vortex_bail!("an invalid row kernel output passed boundary validation"), + Ok(_) => { + vortex_bail!("an invalid row kernel output passed boundary validation") + } }, }; let error = error.to_string(); @@ -880,8 +879,7 @@ fn test_filtered_execution_preserves_runtime_sink_params( let actual = execute_rows(&FilteredRepeat, &width, &args, &mut ctx)?; let expected = FixedSizeListArray::new( PrimitiveArray::from_iter(expected_elements).into_array(), - u32::try_from(width) - .map_err(|_| vortex_err!(InvalidArgument: "test width must fit in u32, got {width}"))?, + u32::try_from(width).map_err(|_| vortex_err!("test width must fit in u32, got {width}"))?, validity, 2, ) diff --git a/vortex-array/src/scalar_fn/unstable/row/execute/sink.rs b/vortex-array/src/scalar_fn/unstable/row/execute/sink.rs index b964cbb737e..46474590f78 100644 --- a/vortex-array/src/scalar_fn/unstable/row/execute/sink.rs +++ b/vortex-array/src/scalar_fn/unstable/row/execute/sink.rs @@ -418,7 +418,9 @@ mod tests { let error = match result { Err(error) => error, - Ok(_) => vortex_bail!("the sink must reject rows changed by its initializer"), + Ok(_) => { + vortex_bail!("the sink must reject rows changed by its initializer") + } }; assert!( error diff --git a/vortex-array/src/scalar_fn/unstable/row/types/sink/fixed_size_list.rs b/vortex-array/src/scalar_fn/unstable/row/types/sink/fixed_size_list.rs index 5a2f86580e3..e74bc7ca98f 100644 --- a/vortex-array/src/scalar_fn/unstable/row/types/sink/fixed_size_list.rs +++ b/vortex-array/src/scalar_fn/unstable/row/types/sink/fixed_size_list.rs @@ -126,7 +126,6 @@ unsafe impl OutputSink for FixedSizeListSink< let width = *params; let element_capacity = rows.checked_mul(width).ok_or_else(|| { vortex_err!( - InvalidArgument: "fixed-size-list sink capacity must fit in usize, got {rows} rows with width {width}" ) })?; diff --git a/vortex-array/src/scalar_fn/unstable/row/visitor/row_visitor.rs b/vortex-array/src/scalar_fn/unstable/row/visitor/row_visitor.rs index 952d0b478ea..22385bda489 100644 --- a/vortex-array/src/scalar_fn/unstable/row/visitor/row_visitor.rs +++ b/vortex-array/src/scalar_fn/unstable/row/visitor/row_visitor.rs @@ -183,7 +183,7 @@ pub trait RowVisitor: private::Sealed + Sized { /// #[cold] /// #[inline(never)] /// fn integer_division_error() -> VortexError { - /// vortex_err!(InvalidArgument: "integer division by zero or overflow") + /// vortex_err!("integer division by zero or overflow") /// } /// /// visitor.visit_into::<(i64, i64), UninitElementSink, _>( @@ -281,7 +281,7 @@ pub trait RowVisitor: private::Sealed + Sized { /// #[cold] /// #[inline(never)] /// fn integer_addition_error() -> VortexError { - /// vortex_err!(InvalidArgument: "integer overflow in checked add") + /// vortex_err!("integer overflow in checked add") /// } /// /// visitor.visit_deferred::<(i64, i64), i64, bool>( @@ -350,7 +350,7 @@ pub trait RowVisitor: private::Sealed + Sized { /// #[cold] /// #[inline(never)] /// fn decimal_rescaling_overflow() -> VortexError { - /// vortex_err!(InvalidArgument: "decimal rescaling overflowed") + /// vortex_err!("decimal rescaling overflowed") /// } /// /// visitor.visit_prepared_deferred::< diff --git a/vortex-array/src/scalar_fn/unstable/row/vtable.rs b/vortex-array/src/scalar_fn/unstable/row/vtable.rs index 32212d367e0..bc4d793404f 100644 --- a/vortex-array/src/scalar_fn/unstable/row/vtable.rs +++ b/vortex-array/src/scalar_fn/unstable/row/vtable.rs @@ -393,7 +393,9 @@ mod tests { let error = match execute_rows(&function, &EmptyOptions, &args, &mut ctx) { Err(error) => error, - Ok(_) => vortex_error::vortex_bail!("dispatch must not change after planning"), + Ok(_) => { + vortex_error::vortex_bail!("dispatch must not change after planning") + } }; let message = error.to_string(); @@ -420,7 +422,9 @@ mod tests { let error = match execute_rows(&function, &EmptyOptions, &args, &mut ctx) { Err(error) => error, - Ok(_) => vortex_error::vortex_bail!("dispatch must preserve its planned element types"), + Ok(_) => { + vortex_error::vortex_bail!("dispatch must preserve its planned element types") + } }; assert!( diff --git a/vortex-array/src/stats/flatbuffers.rs b/vortex-array/src/stats/flatbuffers.rs index 2e385885e5a..4d78c5bfe85 100644 --- a/vortex-array/src/stats/flatbuffers.rs +++ b/vortex-array/src/stats/flatbuffers.rs @@ -165,7 +165,9 @@ impl StatsSet { match fb.max_precision() { fba::Precision::Exact => Precision::Exact(value), fba::Precision::Inexact => Precision::Inexact(value), - other => vortex_bail!("Corrupted max_precision field: {other:?}"), + other => { + vortex_bail!("Corrupted max_precision field: {other:?}") + } }, ); } @@ -185,7 +187,9 @@ impl StatsSet { match fb.min_precision() { fba::Precision::Exact => Precision::Exact(value), fba::Precision::Inexact => Precision::Inexact(value), - other => vortex_bail!("Corrupted min_precision field: {other:?}"), + other => { + vortex_bail!("Corrupted min_precision field: {other:?}") + } }, ); } diff --git a/vortex-array/src/validity.rs b/vortex-array/src/validity.rs index 449f66b07d9..763325257c0 100644 --- a/vortex-array/src/validity.rs +++ b/vortex-array/src/validity.rs @@ -420,9 +420,7 @@ impl Validity { _ if len == 0 => Ok(Some(Validity::NonNullable)), Self::NonNullable => Ok(Some(Self::NonNullable)), Self::AllValid => Ok(Some(Self::NonNullable)), - Self::AllInvalid => { - Err(vortex_err!(InvalidArgument: "Cannot cast AllInvalid to NonNullable")) - } + Self::AllInvalid => Err(vortex_err!("Cannot cast AllInvalid to NonNullable")), Self::Array(_) => Ok(None), } } @@ -451,7 +449,7 @@ impl Validity { ) -> VortexResult { match nullability { Nullability::NonNullable => self.into_non_nullable(len, ctx).ok_or_else(|| { - vortex_err!(InvalidArgument: "Cannot cast array with invalid values to non-nullable type.") + vortex_err!("Cannot cast array with invalid values to non-nullable type.") }), Nullability::Nullable => Ok(self.into_nullable()), } diff --git a/vortex-arrow/src/convert.rs b/vortex-arrow/src/convert.rs index a2760a82883..0dbdc822a2d 100644 --- a/vortex-arrow/src/convert.rs +++ b/vortex-arrow/src/convert.rs @@ -733,7 +733,9 @@ pub fn from_arrow_dyn(array: &dyn ArrowArray, nullable: bool) -> VortexResult vortex_bail!("Unsupported dictionary key type: {key_dt}"), }, - dt => vortex_bail!("Array encoding not implemented for Arrow data type {dt}"), + dt => { + vortex_bail!("Array encoding not implemented for Arrow data type {dt}") + } } } diff --git a/vortex-arrow/src/dtype.rs b/vortex-arrow/src/dtype.rs index 9fb33b8ebba..0e35c05dc69 100644 --- a/vortex-arrow/src/dtype.rs +++ b/vortex-arrow/src/dtype.rs @@ -149,7 +149,7 @@ pub(crate) fn from_arrow_data_type( vortex_ensure_eq!( fields.len(), 2, - InvalidArgument: "Arrow map entries struct must contain exactly two fields" + "Arrow map entries struct must contain exactly two fields" ); let key = &fields[0]; let value = &fields[1]; @@ -292,7 +292,7 @@ pub(crate) fn to_data_type_naive(dtype: &DType) -> VortexResult { TimeUnit::Days => DataType::Date32, TimeUnit::Milliseconds => DataType::Date64, TimeUnit::Nanoseconds | TimeUnit::Microseconds | TimeUnit::Seconds => { - vortex_panic!(InvalidArgument: "Invalid TimeUnit {} for {}", unit, ext_dtype.id()) + vortex_panic!("Invalid TimeUnit {} for {}", unit, ext_dtype.id()) } }, TemporalMetadata::Time(unit) => match unit { @@ -301,7 +301,7 @@ pub(crate) fn to_data_type_naive(dtype: &DType) -> VortexResult { TimeUnit::Microseconds => DataType::Time64(ArrowTimeUnit::Microsecond), TimeUnit::Nanoseconds => DataType::Time64(ArrowTimeUnit::Nanosecond), TimeUnit::Days => { - vortex_panic!(InvalidArgument: "Invalid TimeUnit {} for {}", unit, ext_dtype.id()) + vortex_panic!("Invalid TimeUnit {} for {}", unit, ext_dtype.id()) } }, }); diff --git a/vortex-arrow/src/scalar.rs b/vortex-arrow/src/scalar.rs index 5039e2d0907..aa3cb6356bd 100644 --- a/vortex-arrow/src/scalar.rs +++ b/vortex-arrow/src/scalar.rs @@ -75,14 +75,22 @@ impl ToArrowDatum for Scalar { DType::Decimal(..) => decimal_to_arrow(value.as_decimal()), DType::Utf8(_) => utf8_to_arrow(value.as_utf8()), DType::Binary(_) => binary_to_arrow(value.as_binary()), - DType::List(..) => vortex_bail!("list scalar conversion is not supported"), + DType::List(..) => { + vortex_bail!("list scalar conversion is not supported") + } DType::FixedSizeList(..) => { vortex_bail!("fixed-size list scalar conversion is not supported") } DType::Map(..) => map_to_arrow(value.as_map()), - DType::Struct(..) => vortex_bail!("struct scalar conversion is not supported"), - DType::Union(..) => vortex_bail!("union scalar conversion is not supported"), - DType::Variant(_) => vortex_bail!("Variant scalar conversion is not supported"), + DType::Struct(..) => { + vortex_bail!("struct scalar conversion is not supported") + } + DType::Union(..) => { + vortex_bail!("union scalar conversion is not supported") + } + DType::Variant(_) => { + vortex_bail!("Variant scalar conversion is not supported") + } DType::Extension(..) => extension_to_arrow(value.as_extension()), } } diff --git a/vortex-bench/src/measurements.rs b/vortex-bench/src/measurements.rs index 6eeceda6dbb..3403a7051cb 100644 --- a/vortex-bench/src/measurements.rs +++ b/vortex-bench/src/measurements.rs @@ -74,7 +74,9 @@ impl Div for MeasurementValue { (MeasurementValue::Int(a), MeasurementValue::Int(b)) => { MeasurementValue::Float(a as f64 / b as f64) } - _ => vortex_panic!("Can't divide two measurement values of different kinds"), + _ => { + vortex_panic!("Can't divide two measurement values of different kinds") + } } } } @@ -99,7 +101,9 @@ impl Add for MeasurementValue { MeasurementValue::Float(a + b) } (MeasurementValue::Int(a), MeasurementValue::Int(b)) => MeasurementValue::Int(a + b), - _ => vortex_panic!("Can't subtract two measurement values of different kinds"), + _ => { + vortex_panic!("Can't subtract two measurement values of different kinds") + } } } } @@ -113,7 +117,9 @@ impl Sub for MeasurementValue { MeasurementValue::Float(a - b) } (MeasurementValue::Int(a), MeasurementValue::Int(b)) => MeasurementValue::Int(a - b), - _ => vortex_panic!("Can't subtract two measurement values of different kinds"), + _ => { + vortex_panic!("Can't subtract two measurement values of different kinds") + } } } } diff --git a/vortex-bench/src/statpopgen/vcf_conversion.rs b/vortex-bench/src/statpopgen/vcf_conversion.rs index 8df82ee26f7..eaa8a1a81c6 100644 --- a/vortex-bench/src/statpopgen/vcf_conversion.rs +++ b/vortex-bench/src/statpopgen/vcf_conversion.rs @@ -34,8 +34,12 @@ pub fn builder_from_info(info: &Map) -> InfoArrayBuilder { (Number::Count(1), Type::String) => InfoArrayBuilder::String(Default::default()), (_, Type::Integer) => InfoArrayBuilder::ListInteger(Default::default()), (_, Type::Float) => InfoArrayBuilder::ListFloat(Default::default()), - (_, Type::Flag) => vortex_panic!("Flag INFO fields with a count are not supported"), - (_, Type::Character) => vortex_panic!("Character INFO fields are not supported"), + (_, Type::Flag) => { + vortex_panic!("Flag INFO fields with a count are not supported") + } + (_, Type::Character) => { + vortex_panic!("Character INFO fields are not supported") + } (_, Type::String) => InfoArrayBuilder::ListString(Default::default()), } } @@ -55,8 +59,12 @@ pub fn data_type_from_info(info: &Map) -> DataType { (Number::Count(1), Type::String) => Utf8, (_, Type::Integer) => list(Int32), (_, Type::Float) => list(Float32), - (_, Type::Flag) => vortex_panic!("Flag INFO fields with a count are not supported"), - (_, Type::Character) => vortex_panic!("Character INFO fields are not supported"), + (_, Type::Flag) => { + vortex_panic!("Flag INFO fields with a count are not supported") + } + (_, Type::Character) => { + vortex_panic!("Character INFO fields are not supported") + } (_, Type::String) => list(Utf8), } } diff --git a/vortex-cloud/src/registry/tests.rs b/vortex-cloud/src/registry/tests.rs index 0566e0386de..ca47328dd07 100644 --- a/vortex-cloud/src/registry/tests.rs +++ b/vortex-cloud/src/registry/tests.rs @@ -265,7 +265,6 @@ async fn test_resolved_key_reaches_the_object( #[case] key: &str, ) -> Result<(), Box> { use object_store::ObjectStoreExt; - use vortex_error::VortexError; use vortex_file::OpenOptionsSessionExt; let session = VortexSession::empty() @@ -295,11 +294,8 @@ async fn test_resolved_key_reaches_the_object( panic!("the payload is not a Vortex file, so opening it must fail") }; assert!( - matches!( - &err, - VortexError::Other(message, _) - if message.as_ref() == "Malformed file, invalid magic bytes, got [0, 0, 0, 0]" - ), + err.to_string() + .contains("Malformed file, invalid magic bytes, got [0, 0, 0, 0]"), "opening {key:?} must read the stored payload and reject its footer: {err}" ); Ok(()) diff --git a/vortex-compressor/src/stats/float.rs b/vortex-compressor/src/stats/float.rs index 5b243f38dca..3f4088f4059 100644 --- a/vortex-compressor/src/stats/float.rs +++ b/vortex-compressor/src/stats/float.rs @@ -114,7 +114,9 @@ impl FloatStats { PType::F16 => typed_float_stats::(input, opts.count_distinct_values, ctx), PType::F32 => typed_float_stats::(input, opts.count_distinct_values, ctx), PType::F64 => typed_float_stats::(input, opts.count_distinct_values, ctx), - _ => vortex_panic!("cannot generate FloatStats from ptype {}", input.ptype()), + _ => { + vortex_panic!("cannot generate FloatStats from ptype {}", input.ptype()) + } } } diff --git a/vortex-cuda/benches/dynamic_dispatch_cuda.rs b/vortex-cuda/benches/dynamic_dispatch_cuda.rs index e0d5b694788..ef9d9aa9fee 100644 --- a/vortex-cuda/benches/dynamic_dispatch_cuda.rs +++ b/vortex-cuda/benches/dynamic_dispatch_cuda.rs @@ -99,7 +99,9 @@ static GLOBAL: MiMalloc = MiMalloc; fn global_reference(array: &FoRArray) -> Scalar { match array.references() { FoRReferences::Global(reference) => reference.clone(), - FoRReferences::Blocked(_) => vortex_panic!("FoR::encode uses a global reference"), + FoRReferences::Blocked(_) => { + vortex_panic!("FoR::encode uses a global reference") + } } } diff --git a/vortex-cuda/ffi/src/lib.rs b/vortex-cuda/ffi/src/lib.rs index f34e4408ef4..02fb74c1e80 100644 --- a/vortex-cuda/ffi/src/lib.rs +++ b/vortex-cuda/ffi/src/lib.rs @@ -18,7 +18,6 @@ use vortex::dtype::FieldName; use vortex::dtype::FieldNames; use vortex::error::VortexResult; use vortex::error::vortex_ensure; -use vortex::error::vortex_err; use vortex::expr::root; use vortex::expr::select; use vortex::file::OpenOptionsSessionExt; @@ -338,7 +337,7 @@ unsafe fn scan_columns(columns: *const vx_view, ncolumns: usize) -> VortexResult ); // SAFETY: The caller guarantees readable name bytes. as_str checks null and UTF-8. let name = unsafe { column.as_str() } - .map_err(|error| vortex_err!("invalid CUDA scan column {index}: {error}"))?; + .map_err(|error| error.with_context(format!("invalid CUDA scan column {index}")))?; vortex_ensure!(seen.insert(name), "duplicate CUDA scan column: {name:?}"); names.push(FieldName::from(name)); } @@ -531,6 +530,7 @@ mod tests { use vortex::dtype::Nullability; use vortex::error::VortexResult; use vortex::error::vortex_ensure_eq; + use vortex::error::vortex_err; use vortex::file::WriteOptionsSessionExt; use vortex::io::session::RuntimeSessionExt; use vortex::layout::LayoutStrategy; diff --git a/vortex-cuda/src/arrow/canonical.rs b/vortex-cuda/src/arrow/canonical.rs index 44e4b11f0b0..f04295e04e1 100644 --- a/vortex-cuda/src/arrow/canonical.rs +++ b/vortex-cuda/src/arrow/canonical.rs @@ -336,7 +336,9 @@ fn export_canonical( } export_varbinview(varbinview, ctx).await } - c => vortex_bail!("unsupported Arrow Device export for {} array", c.dtype()), + c => { + vortex_bail!("unsupported Arrow Device export for {} array", c.dtype()) + } } }) } @@ -1776,7 +1778,9 @@ mod tests { PType::I16 => primitive_on_device(values.iter().map(|&value| value as i16), ctx).await, PType::I32 => primitive_on_device(values.iter().map(|&value| value as i32), ctx).await, PType::I64 => primitive_on_device(values.iter().copied(), ctx).await, - ptype => vortex_bail!("test helper only supports integer PTypes, got {ptype}"), + ptype => { + vortex_bail!("test helper only supports integer PTypes, got {ptype}") + } } } diff --git a/vortex-cuda/src/arrow/list_view.rs b/vortex-cuda/src/arrow/list_view.rs index 2ce3b3b5669..9bff2a8d476 100644 --- a/vortex-cuda/src/arrow/list_view.rs +++ b/vortex-cuda/src/arrow/list_view.rs @@ -548,6 +548,8 @@ where 2 => vortex_bail!( "cannot export device-resident ListViewArray as Arrow List: offsets exceed i32 range required by cuDF" ), - status => vortex_bail!("unexpected list-view offsets kernel status {status}"), + status => { + vortex_bail!("unexpected list-view offsets kernel status {status}") + } } } diff --git a/vortex-cuda/src/arrow/mod.rs b/vortex-cuda/src/arrow/mod.rs index 3baa16b8853..a412f9f82aa 100644 --- a/vortex-cuda/src/arrow/mod.rs +++ b/vortex-cuda/src/arrow/mod.rs @@ -780,7 +780,9 @@ fn arrow_device_export_dictionary_codes_dtype(codes_dtype: &DType) -> VortexResu PType::U16 => PType::I32, PType::U32 | PType::U64 => PType::I64, ptype @ (PType::I8 | PType::I16 | PType::I32 | PType::I64) => ptype, - ptype => return Err(vortex_err!("dictionary codes must be integer, got {ptype}")), + ptype => { + return Err(vortex_err!("dictionary codes must be integer, got {ptype}")); + } }; Ok(DType::Primitive(ptype, codes_dtype.nullability())) diff --git a/vortex-cuda/src/dynamic_dispatch/mod.rs b/vortex-cuda/src/dynamic_dispatch/mod.rs index 0a031dc95ef..494bd90862b 100644 --- a/vortex-cuda/src/dynamic_dispatch/mod.rs +++ b/vortex-cuda/src/dynamic_dispatch/mod.rs @@ -391,7 +391,9 @@ impl MaterializedPlan { PType::U16 | PType::I16 => PType::U16, PType::U32 | PType::I32 | PType::F32 => PType::U32, PType::U64 | PType::I64 | PType::F64 => PType::U64, - other => vortex_bail!("dynamic dispatch does not support PType {:?}", other), + other => { + vortex_bail!("dynamic dispatch does not support PType {:?}", other) + } }; match_each_unsigned_integer_ptype!(unsigned_ptype, |T| { self.execute_typed::(output_ptype, len, ctx) @@ -538,7 +540,9 @@ mod tests { fn global_reference(array: &FoRArray) -> VortexResult { match array.references() { FoRReferences::Global(reference) => Ok(reference.clone()), - FoRReferences::Blocked(_) => vortex_bail!("FoR::encode uses a global reference"), + FoRReferences::Blocked(_) => { + vortex_bail!("FoR::encode uses a global reference") + } } } diff --git a/vortex-cuda/src/kernel/encodings/date_time_parts.rs b/vortex-cuda/src/kernel/encodings/date_time_parts.rs index 9a1a339aa67..35be0f0dd36 100644 --- a/vortex-cuda/src/kernel/encodings/date_time_parts.rs +++ b/vortex-cuda/src/kernel/encodings/date_time_parts.rs @@ -93,7 +93,9 @@ impl CudaExecute for DateTimePartsExecutor { TimeUnit::Microseconds => 1_000_000, TimeUnit::Milliseconds => 1_000, TimeUnit::Seconds => 1, - TimeUnit::Days => vortex_bail!("Cannot decode DateTimeParts with TimeUnit::Days"), + TimeUnit::Days => { + vortex_bail!("Cannot decode DateTimeParts with TimeUnit::Days") + } }; let days_canonical = array.days().clone().execute_cuda(ctx).await?; diff --git a/vortex-cuda/src/kernel/encodings/onpair.rs b/vortex-cuda/src/kernel/encodings/onpair.rs index a97aca0941b..6fe8152a19e 100644 --- a/vortex-cuda/src/kernel/encodings/onpair.rs +++ b/vortex-cuda/src/kernel/encodings/onpair.rs @@ -309,7 +309,9 @@ async fn decode_onpair_bytes( match codes.ptype() { PType::U8 => decode_window::(codes, codes_offsets, lengths, dict, ctx).await, PType::U16 => decode_window::(codes, codes_offsets, lengths, dict, ctx).await, - other => vortex_bail!("OnPair codes must decompress to u8 or u16, got {other}"), + other => { + vortex_bail!("OnPair codes must decompress to u8 or u16, got {other}") + } } } diff --git a/vortex-cuda/src/kernel/filter/mod.rs b/vortex-cuda/src/kernel/filter/mod.rs index 3bb02d7f8db..1c925fa9526 100644 --- a/vortex-cuda/src/kernel/filter/mod.rs +++ b/vortex-cuda/src/kernel/filter/mod.rs @@ -83,7 +83,9 @@ impl CudaExecute for FilterExecutor { Canonical::VarBinView(varbinview) => { filter_varbinview(varbinview, m, ctx).await } - c => vortex_bail!("CUDA filter kernel is not implemented for {}", c.dtype()), + c => { + vortex_bail!("CUDA filter kernel is not implemented for {}", c.dtype()) + } } } } diff --git a/vortex-datafusion/src/convert/scalars.rs b/vortex-datafusion/src/convert/scalars.rs index 1b5a2d8e95f..08d403af301 100644 --- a/vortex-datafusion/src/convert/scalars.rs +++ b/vortex-datafusion/src/convert/scalars.rs @@ -134,7 +134,9 @@ impl TryToDataFusion for Scalar { dtype @ DType::Union(..) => vortex_bail!( "cannot convert Vortex scalar dtype {dtype} to DataFusion ScalarValue: unsupported scalar type" ), - DType::Variant(_) => vortex_bail!("Variant scalars aren't supported with DF"), + DType::Variant(_) => { + vortex_bail!("Variant scalars aren't supported with DF") + } DType::Extension(ext) => { let storage_scalar = self.as_extension().to_storage_scalar(); @@ -385,7 +387,7 @@ fn struct_from_df(array: &StructArray, session: &VortexSession) -> Scalar { .from_arrow_array(ArrowArrayRef::clone(column), field) .and_then(|column| column.execute_scalar(0, &mut ctx)) .unwrap_or_else(|e| { - vortex_panic!("cannot convert struct field to a Vortex scalar: {e}") + vortex_panic!(e, "cannot convert struct field to a Vortex scalar") }) }) .collect::>(); diff --git a/vortex-duckdb/src/convert/dtype.rs b/vortex-duckdb/src/convert/dtype.rs index e80542e9327..28b6a841b4b 100644 --- a/vortex-duckdb/src/convert/dtype.rs +++ b/vortex-duckdb/src/convert/dtype.rs @@ -88,7 +88,9 @@ impl FromLogicalType for DType { nullability: Nullability, ) -> VortexResult { Ok(match logical_type.as_type_id() { - DUCKDB_TYPE::DUCKDB_TYPE_INVALID => vortex_bail!("invalid duckdb type"), + DUCKDB_TYPE::DUCKDB_TYPE_INVALID => { + vortex_bail!("invalid duckdb type") + } DUCKDB_TYPE::DUCKDB_TYPE_SQLNULL => DType::Null, DUCKDB_TYPE::DUCKDB_TYPE_BOOLEAN => DType::Bool(nullability), DUCKDB_TYPE::DUCKDB_TYPE_TINYINT => DType::Primitive(I8, nullability), @@ -99,8 +101,12 @@ impl FromLogicalType for DType { DUCKDB_TYPE::DUCKDB_TYPE_USMALLINT => DType::Primitive(U16, nullability), DUCKDB_TYPE::DUCKDB_TYPE_UINTEGER => DType::Primitive(U32, nullability), DUCKDB_TYPE::DUCKDB_TYPE_UBIGINT => DType::Primitive(U64, nullability), - DUCKDB_TYPE::DUCKDB_TYPE_HUGEINT => vortex_bail!("I128 is not in Vortex type system"), - DUCKDB_TYPE::DUCKDB_TYPE_UHUGEINT => vortex_bail!("U128 is not in Vortex type system"), + DUCKDB_TYPE::DUCKDB_TYPE_HUGEINT => { + vortex_bail!("I128 is not in Vortex type system") + } + DUCKDB_TYPE::DUCKDB_TYPE_UHUGEINT => { + vortex_bail!("U128 is not in Vortex type system") + } DUCKDB_TYPE::DUCKDB_TYPE_FLOAT => DType::Primitive(F32, nullability), DUCKDB_TYPE::DUCKDB_TYPE_DOUBLE => DType::Primitive(F64, nullability), DUCKDB_TYPE::DUCKDB_TYPE_VARCHAR => DType::Utf8(nullability), @@ -266,7 +272,9 @@ impl TryFrom<&DType> for LogicalType { } // TODO(connor): Union DType::Union(..) => vortex_bail!("Vortex Union isn't supported"), - DType::Variant(_) => vortex_bail!("Vortex Variant array aren't supported"), + DType::Variant(_) => { + vortex_bail!("Vortex Variant array aren't supported") + } DType::Extension(ext_dtype) => { // Handle first-party extension types that have DuckDB equivalents. if let Some(temporal) = ext_dtype.metadata_opt::() { @@ -389,7 +397,9 @@ impl TryFrom for LogicalType { U64 => LogicalType::new(DUCKDB_TYPE::DUCKDB_TYPE_UBIGINT), F32 => LogicalType::new(DUCKDB_TYPE::DUCKDB_TYPE_FLOAT), F64 => LogicalType::new(DUCKDB_TYPE::DUCKDB_TYPE_DOUBLE), - PType::F16 => return Err(vortex_err!("F16 type not supported in DuckDB")), + PType::F16 => { + return Err(vortex_err!("F16 type not supported in DuckDB")); + } }) } } diff --git a/vortex-duckdb/src/convert/expr.rs b/vortex-duckdb/src/convert/expr.rs index 7d7cf0e6aa8..b0ec984b7cc 100644 --- a/vortex-duckdb/src/convert/expr.rs +++ b/vortex-duckdb/src/convert/expr.rs @@ -85,7 +85,9 @@ use crate::projection::DuckdbField; fn from_bound_str(value: &ExpressionRef) -> VortexResult { match value.as_class().vortex_expect("unknown class") { BoundConstant(constant) => Ok(constant.value.as_string().as_str().to_owned()), - _ => vortex_bail!("Expected string expression, got {:?}", value.as_class_id()), + _ => { + vortex_bail!("Expected string expression, got {:?}", value.as_class_id()) + } } } @@ -718,7 +720,9 @@ fn try_from_expression_inner( DUCKDB_VX_EXPR_TYPE::DUCKDB_VX_EXPR_TYPE_CONJUNCTION_OR => { or_collect(children).vortex_expect("cannot be empty") } - _ => vortex_bail!("unexpected operator {:?} in bound conjunction", conj.op), + _ => { + vortex_bail!("unexpected operator {:?} in bound conjunction", conj.op) + } } } ExpressionClass::BoundAggregate(_) => return Ok(None), @@ -760,7 +764,9 @@ impl TryFrom for Operator { fn try_from(value: DUCKDB_VX_EXPR_TYPE) -> VortexResult { Ok(match value { - DUCKDB_VX_EXPR_TYPE::DUCKDB_VX_EXPR_TYPE_INVALID => vortex_bail!("invalid expression"), + DUCKDB_VX_EXPR_TYPE::DUCKDB_VX_EXPR_TYPE_INVALID => { + vortex_bail!("invalid expression") + } DUCKDB_VX_EXPR_TYPE::DUCKDB_VX_EXPR_TYPE_COMPARE_EQUAL => Operator::Eq, DUCKDB_VX_EXPR_TYPE::DUCKDB_VX_EXPR_TYPE_COMPARE_NOTEQUAL => Operator::NotEq, DUCKDB_VX_EXPR_TYPE::DUCKDB_VX_EXPR_TYPE_COMPARE_LESSTHAN => Operator::Lt, diff --git a/vortex-duckdb/src/convert/scalar.rs b/vortex-duckdb/src/convert/scalar.rs index f4478091b31..0b8b79dc1cb 100644 --- a/vortex-duckdb/src/convert/scalar.rs +++ b/vortex-duckdb/src/convert/scalar.rs @@ -86,10 +86,14 @@ impl ToDuckDBScalar for Scalar { DType::Binary(_) => self.as_binary().try_to_duckdb_scalar(), DType::List(..) | DType::FixedSizeList(..) => self.as_list().try_to_duckdb_scalar(), DType::Map(..) => self.as_map().try_to_duckdb_scalar(), - DType::Variant(_) => vortex_bail!("Vortex Variant scalars aren't supported"), + DType::Variant(_) => { + vortex_bail!("Vortex Variant scalars aren't supported") + } DType::Struct(..) => self.as_struct().try_to_duckdb_scalar(), // TODO(connor): Union - DType::Union(..) => vortex_bail!("Vortex Union scalars aren't supported"), + DType::Union(..) => { + vortex_bail!("Vortex Union scalars aren't supported") + } DType::Extension(..) => self.as_extension().try_to_duckdb_scalar(), } } @@ -139,7 +143,9 @@ impl ToDuckDBScalar for DecimalScalar<'_> { DecimalValue::I32(v) => v as i128, DecimalValue::I64(v) => v as i128, DecimalValue::I128(v) => v, - DecimalValue::I256(_) => vortex_bail!("cannot handle a i256 decimal in duckdb"), + DecimalValue::I256(_) => { + vortex_bail!("cannot handle a i256 decimal in duckdb") + } }; Ok(Value::new_decimal( @@ -283,7 +289,9 @@ impl ToDuckDBScalar for ExtScalar<'_> { Some(days) => Value::new_date(days), None => Value::null(&*ext_logical_type(self)?), }, - _ => vortex_bail!("cannot have TimeUnit {unit}, so represent a day"), + _ => { + vortex_bail!("cannot have TimeUnit {unit}, so represent a day") + } }, TemporalMetadata::Time(unit) => match unit { TimeUnit::Microseconds => Value::new_time(value()?), @@ -374,7 +382,9 @@ impl<'a> TryFrom<&'a ValueRef> for Scalar { ); Ok(Scalar::extension_ref(ext.clone(), storage)) } - _ => vortex_bail!("Cannot convert DuckDB blob to Vortex scalar of dtype {dtype}"), + _ => { + vortex_bail!("Cannot convert DuckDB blob to Vortex scalar of dtype {dtype}") + } }, ExtractedValue::Date(days) => Ok(Scalar::extension::( TimeUnit::Days, diff --git a/vortex-duckdb/src/convert/table_filter.rs b/vortex-duckdb/src/convert/table_filter.rs index 00d3decaf5e..2dce92d3d04 100644 --- a/vortex-duckdb/src/convert/table_filter.rs +++ b/vortex-duckdb/src/convert/table_filter.rs @@ -122,7 +122,9 @@ pub(super) fn dynamic_filter_expr( DUCKDB_VX_EXPR_TYPE::DUCKDB_VX_EXPR_TYPE_COMPARE_GREATERTHANOREQUALTO => { CompareOperator::Gte } - _ => vortex_bail!("unsupported dynamic filter operator {:?}", dynamic.operator), + _ => { + vortex_bail!("unsupported dynamic filter operator {:?}", dynamic.operator) + } }; let data = dynamic.data; diff --git a/vortex-duckdb/src/convert/vector.rs b/vortex-duckdb/src/convert/vector.rs index 77e57226928..f3da09ceeec 100644 --- a/vortex-duckdb/src/convert/vector.rs +++ b/vortex-duckdb/src/convert/vector.rs @@ -406,7 +406,9 @@ pub fn flat_vector_to_vortex(vector: &VectorRef, len: usize) -> VortexResult vortex_bail!("{type_id:?} flat Vector to Vortex array not supported"), + type_id => { + vortex_bail!("{type_id:?} flat Vector to Vortex array not supported") + } } } diff --git a/vortex-duckdb/src/duckdb/macro_.rs b/vortex-duckdb/src/duckdb/macro_.rs index 56d3aa273a4..83e79ada5e5 100644 --- a/vortex-duckdb/src/duckdb/macro_.rs +++ b/vortex-duckdb/src/duckdb/macro_.rs @@ -11,14 +11,14 @@ macro_rules! duckdb_try { }; // Pattern: duckdb_try!(function_call, "error message") - ($call:expr, $msg:expr) => { + ($call:expr, $msg:literal) => { if $call != $crate::cpp::duckdb_state::DuckDBSuccess { vortex::error::vortex_bail!($msg); } }; // Pattern: duckdb_try!(function_call, "error message with {}", args...) - ($call:expr, $msg:expr, $($args:expr),+) => { + ($call:expr, $msg:literal, $($args:expr),+) => { if $call != $crate::cpp::duckdb_state::DuckDBSuccess { vortex::error::vortex_bail!($msg, $($args),+); } diff --git a/vortex-duckdb/src/duckdb/value.rs b/vortex-duckdb/src/duckdb/value.rs index 506f047cf0e..bd14635f18a 100644 --- a/vortex-duckdb/src/duckdb/value.rs +++ b/vortex-duckdb/src/duckdb/value.rs @@ -47,7 +47,9 @@ impl ValueRef { return ExtractedValue::Null; } match self.logical_type().as_type_id() { - DUCKDB_TYPE::DUCKDB_TYPE_INVALID => vortex_panic!("Invalid type for DuckDB value"), + DUCKDB_TYPE::DUCKDB_TYPE_INVALID => { + vortex_panic!("Invalid type for DuckDB value") + } DUCKDB_TYPE::DUCKDB_TYPE_SQLNULL => ExtractedValue::Null, DUCKDB_TYPE::DUCKDB_TYPE_BOOLEAN => { ExtractedValue::Boolean(unsafe { cpp::duckdb_get_bool(self.as_ptr()) }) diff --git a/vortex-duckdb/src/exporter/decimal.rs b/vortex-duckdb/src/exporter/decimal.rs index 8d2052a3215..2330bc6a2b6 100644 --- a/vortex-duckdb/src/exporter/decimal.rs +++ b/vortex-duckdb/src/exporter/decimal.rs @@ -123,7 +123,9 @@ pub fn precision_to_duckdb_storage_size(decimal_dtype: &DecimalDType) -> VortexR 5..=9 => DecimalType::I32, 10..=18 => DecimalType::I64, 19..=38 => DecimalType::I128, - decimal_dtype => vortex_bail!("cannot represent decimal in ducdkb {decimal_dtype}"), + decimal_dtype => { + vortex_bail!("cannot represent decimal in ducdkb {decimal_dtype}") + } }) } diff --git a/vortex-duckdb/src/lib.rs b/vortex-duckdb/src/lib.rs index 1209213135f..0201079ba40 100644 --- a/vortex-duckdb/src/lib.rs +++ b/vortex-duckdb/src/lib.rs @@ -14,7 +14,6 @@ use vortex::editions::CORE_2026_08_3; use vortex::editions::EditionSessionExt; use vortex::error::VortexExpect; use vortex::error::VortexResult; -use vortex::error::vortex_err; use vortex::io::runtime::BlockingRuntime; use vortex::io::runtime::current::CurrentThreadRuntime; use vortex::io::session::RuntimeSessionExt; @@ -51,7 +50,6 @@ static SESSION: LazyLock = LazyLock::new(|| { let session = VortexSession::default().with_handle(RUNTIME.handle()); session .enable_edition(CORE_2026_08_3) - .map_err(|error| vortex_err!("{error}")) .vortex_expect("DuckDB-supported draft core edition is registered"); vortex_spatial::initialize(&session); session diff --git a/vortex-duckdb/src/table_function.rs b/vortex-duckdb/src/table_function.rs index ed751b1d64b..7c7d172b38c 100644 --- a/vortex-duckdb/src/table_function.rs +++ b/vortex-duckdb/src/table_function.rs @@ -394,7 +394,9 @@ fn aggregate_output_value(scalar: Scalar, expected: &LogicalTypeRef) -> VortexRe Ok(match primitive.ptype() { PType::I64 => primitive.typed_value::().map(i128::from), PType::U64 => primitive.typed_value::().map(i128::from), - other => vortex_bail!("expected {expected:?} output type, got {other}"), + other => { + vortex_bail!("expected {expected:?} output type, got {other}") + } }) }; match expected.as_type_id() { diff --git a/vortex-error/src/lib.rs b/vortex-error/src/lib.rs index f8ebd0ecec6..0d90340ebe2 100644 --- a/vortex-error/src/lib.rs +++ b/vortex-error/src/lib.rs @@ -5,6 +5,10 @@ //! This crate defines error & result types for Vortex. //! It also contains a variety of useful macros for error handling. +//! +//! A [`VortexError`] is a human-readable message, not one enum variant per library that Vortex +//! happens to depend on. An error from a dependency is attached as the [`Error::source`] of a +//! [`VortexError`] and is recovered by downcasting that source. use std::backtrace::Backtrace; use std::backtrace::BacktraceStatus; @@ -23,7 +27,7 @@ use std::sync::Arc; use std::sync::LazyLock; /// A string that can be used as an error message. -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct ErrString(Cow<'static, str>); #[expect( @@ -73,194 +77,122 @@ impl Display for ErrString { } } -impl From for VortexError { - fn from(_: Infallible) -> Self { - unreachable!() - } -} - -const _: () = assert!(size_of::() < 128); - /// The top-level error type for Vortex. -#[non_exhaustive] -pub enum VortexError { - /// A catch-all error variant - Other(ErrString, Box), - /// A wrapped external error - External(Box, Box), - /// An index is out of bounds. - OutOfBounds(usize, usize, usize, Box), - /// An error occurred while executing a compute kernel. - Compute(ErrString, Box), - /// An invalid argument was provided. - InvalidArgument(ErrString, Box), - /// An error occurred while serializing or deserializing. - Serde(ErrString, Box), - /// An unimplemented function was called. - NotImplemented(ErrString, ErrString, Box), - /// A type mismatch occurred. - MismatchedTypes(ErrString, ErrString, Box), - /// An assertion failed. - AssertionFailed(ErrString, Box), - /// A wrapper for other errors, carrying additional context. - Context(ErrString, Box), - /// A wrapper for shared errors that require cloning. - Shared(Arc), - /// A wrapper for errors from the Arrow library. - Arrow(arrow_schema::ArrowError, Box), - /// A wrapper for errors from the FlatBuffers library. - #[cfg(feature = "flatbuffers")] - FlatBuffers(flatbuffers::InvalidFlatbuffer, Box), - /// A wrapper for formatting errors. - Fmt(fmt::Error, Box), - /// A wrapper for IO errors. - Io(io::Error, Box), - /// A wrapper for errors from the Object Store library. - #[cfg(feature = "object_store")] - ObjectStore(object_store::Error, Box), - /// A wrapper for errors from the Jiff library. - Jiff(jiff::Error, Box), - /// A wrapper for Tokio join error. - #[cfg(feature = "tokio")] - Join(tokio::task::JoinError, Box), - /// Wrap errors for fallible integer casting. - TryFromInt(TryFromIntError, Box), - /// Wrap protobuf-related errors - Prost(Box, Box), +/// +/// An error is a message, an optional underlying error, and a backtrace captured at +/// construction. Cloning is cheap: the payload is shared, never copied. +/// +/// The payload sits behind one pointer, so a `VortexResult` costs the same as `T` plus at +/// most a word on the success path. Most results fit in two registers and are returned in them, +/// rather than through a stack slot the caller must reserve and read back. Constructing an error +/// is no more expensive for it: the payload's allocation is the one the backtrace needed anyway. +#[derive(Clone)] +pub struct VortexError(Arc); + +const _: () = assert!(size_of::() == size_of::()); +const _: () = assert!(size_of::>() == size_of::()); + +struct ErrorInner { + /// `None` defers the message to the source, keeping the `?` conversion path allocation-free. + message: Option, + origin: Origin, +} + +/// Where an error's source and backtrace come from. +enum Origin { + /// Captured when this error was created. + Own { + source: Option>, + backtrace: Backtrace, + }, + /// Shared with the error this one adds context to. + /// + /// Context is normally added in place. This is only for an error that has been cloned, whose + /// payload the other clones still see. + Context(VortexError), } impl VortexError { - /// Adds additional context to an error. - pub fn with_context>(self, msg: T) -> Self { - VortexError::Context(msg.into(), Box::new(self)) + /// Creates an error carrying `message`. + pub fn new>(message: T) -> Self { + Self::from_parts(Some(message.into()), None) } - /// Error prefix by variant - fn variant_prefix(&self) -> &'static str { - use VortexError::*; - - match self { - Other(..) => "Other error: ", - External(..) => "External error: ", - OutOfBounds(..) => "Out of bounds error: ", - Compute(..) => "Compute error: ", - InvalidArgument(..) => "Invalid argument error: ", - Serde(..) => "Serde error: ", - NotImplemented(..) => "Not implemented error: ", - MismatchedTypes(..) => "Mismatched types error: ", - AssertionFailed(..) => "Assertion failed error: ", - Context(..) | Shared(..) => "", // basically delegate to the underlying one - Arrow(..) => "Arrow error: ", - #[cfg(feature = "flatbuffers")] - FlatBuffers(..) => "Flat buffers error: ", - Fmt(..) => "Fmt: ", - Io(..) => "Io: ", - #[cfg(feature = "object_store")] - ObjectStore(..) => "Object store error: ", - Jiff(..) => "Jiff error: ", - #[cfg(feature = "tokio")] - Join(..) => "Tokio join error:", - TryFromInt(..) => "Try from int error:", - Prost(..) => "Prost error:", + /// Wraps an underlying error as a Vortex error. + /// + /// The wrapped error is preserved as the [`Error::source`], so callers that care about the + /// concrete type can recover it with [`Error::source`] and `downcast_ref`. + pub fn external(source: E) -> Self + where + E: Into>, + { + Self::from_parts(None, Some(source.into())) + } + + fn from_parts( + message: Option, + source: Option>, + ) -> Self { + Self(Arc::new(ErrorInner { + message, + origin: Origin::Own { + source, + backtrace: Backtrace::capture(), + }, + })) + } + + /// Adds additional context to an error, preserving its source and backtrace. + pub fn with_context>(mut self, msg: T) -> Self { + let msg: ErrString = msg.into(); + // Build the combined string directly: `msg` has already been through the + // `VORTEX_PANIC_ON_ERR` check and must not trip it a second time. + let message = ErrString(Cow::Owned(format!("{msg}:\n {}", self.message_body()))); + if let Some(inner) = Arc::get_mut(&mut self.0) { + inner.message = Some(message); + return self; + } + Self(Arc::new(ErrorInner { + message: Some(message), + origin: Origin::Context(self), + })) + } + + /// The error message, falling back to the underlying error when none was provided. + fn message_body(&self) -> Cow<'_, str> { + match (&self.0.message, self.source_ref()) { + (Some(message), _) => Cow::Borrowed(message.as_ref()), + (None, Some(source)) => Cow::Owned(source.to_string()), + (None, None) => Cow::Borrowed(""), } } - fn backtrace(&self) -> Option<&Backtrace> { - use VortexError::*; - - match self { - Other(.., bt) => Some(bt.as_ref()), - External(.., bt) => Some(bt.as_ref()), - OutOfBounds(.., bt) => Some(bt.as_ref()), - Compute(.., bt) => Some(bt.as_ref()), - InvalidArgument(.., bt) => Some(bt.as_ref()), - Serde(.., bt) => Some(bt.as_ref()), - NotImplemented(.., bt) => Some(bt.as_ref()), - MismatchedTypes(.., bt) => Some(bt.as_ref()), - AssertionFailed(.., bt) => Some(bt.as_ref()), - Arrow(.., bt) => Some(bt.as_ref()), - #[cfg(feature = "flatbuffers")] - FlatBuffers(.., bt) => Some(bt.as_ref()), - Fmt(.., bt) => Some(bt.as_ref()), - Io(.., bt) => Some(bt.as_ref()), - #[cfg(feature = "object_store")] - ObjectStore(.., bt) => Some(bt.as_ref()), - Jiff(.., bt) => Some(bt.as_ref()), - #[cfg(feature = "tokio")] - Join(.., bt) => Some(bt.as_ref()), - TryFromInt(.., bt) => Some(bt.as_ref()), - Prost(.., bt) => Some(bt.as_ref()), - Context(_, inner) => inner.backtrace(), - Shared(inner) => inner.backtrace(), + fn source_ref(&self) -> Option<&(dyn Error + Send + Sync + 'static)> { + match &self.0.origin { + Origin::Own { source, .. } => source.as_deref(), + Origin::Context(error) => error.source_ref(), } } - fn message(&self) -> String { - use VortexError::*; - - match self { - Other(msg, _) => msg.to_string(), - External(err, _) => err.to_string(), - OutOfBounds(idx, start, stop, _) => { - format!("index {idx} out of bounds from {start} to {stop}") - } - Compute(msg, _) | InvalidArgument(msg, _) | Serde(msg, _) | AssertionFailed(msg, _) => { - format!("{msg}") - } - NotImplemented(func, by_whom, _) => { - format!("function {func} not implemented for {by_whom}") - } - MismatchedTypes(expected, actual, _) => { - format!("expected type: {expected} but instead got {actual}") - } - Context(msg, inner) => { - format!("{msg}:\n {inner}") - } - Shared(inner) => inner.message(), - Arrow(err, _) => { - format!("{err}") - } - #[cfg(feature = "flatbuffers")] - FlatBuffers(err, _) => { - format!("{err}") - } - Fmt(err, _) => { - format!("{err}") - } - Io(err, _) => { - format!("{err}") - } - #[cfg(feature = "object_store")] - ObjectStore(err, _) => { - format!("{err}") - } - Jiff(err, _) => { - format!("{err}") - } - #[cfg(feature = "tokio")] - Join(err, _) => { - format!("{err}") - } - TryFromInt(err, _) => { - format!("{err}") - } - Prost(err, _) => { - format!("{err}") - } + fn backtrace(&self) -> &Backtrace { + match &self.0.origin { + Origin::Own { backtrace, .. } => backtrace, + Origin::Context(error) => error.backtrace(), } } } impl Display for VortexError { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.variant_prefix())?; - write!(f, "{}", self.message())?; - if let Some(backtrace) = self.backtrace() - && backtrace.status() == BacktraceStatus::Captured - { + match (&self.0.message, self.source_ref()) { + (Some(message), _) => Display::fmt(message, f)?, + (None, Some(source)) => Display::fmt(source, f)?, + (None, None) => {} + } + let backtrace = self.backtrace(); + if backtrace.status() == BacktraceStatus::Captured { write!(f, "\nBacktrace:\n{backtrace}")?; } - Ok(()) } } @@ -273,24 +205,8 @@ impl Debug for VortexError { impl Error for VortexError { fn source(&self) -> Option<&(dyn Error + 'static)> { - use VortexError::*; - - match self { - External(err, _) => Some(err.as_ref()), - Context(_, inner) => inner.source(), - Shared(inner) => inner.source(), - Arrow(err, _) => Some(err), - #[cfg(feature = "flatbuffers")] - FlatBuffers(err, _) => Some(err), - Io(err, _) => Some(err), - #[cfg(feature = "object_store")] - ObjectStore(err, _) => Some(err), - Jiff(err, _) => Some(err), - #[cfg(feature = "tokio")] - Join(err, _) => Some(err), - Prost(err, _) => Some(err.as_ref()), - _ => None, - } + self.source_ref() + .map(|source| source as &(dyn Error + 'static)) } } @@ -302,18 +218,13 @@ pub type SharedVortexResult = Result>; impl From> for VortexError { fn from(value: Arc) -> Self { - Self::from(&value) + Arc::try_unwrap(value).unwrap_or_else(|value| (*value).clone()) } } impl From<&Arc> for VortexError { - fn from(e: &Arc) -> Self { - if let VortexError::Shared(e_inner) = e.as_ref() { - // don't re-wrap - VortexError::Shared(Arc::clone(e_inner)) - } else { - VortexError::Shared(Arc::clone(e)) - } + fn from(value: &Arc) -> Self { + (**value).clone() } } @@ -363,45 +274,26 @@ impl VortexExpect for Option { } /// A convenient macro for creating a VortexError. +/// +/// ``` +/// use vortex_error::vortex_err; +/// +/// let err = vortex_err!("negative length {}", -1); +/// assert!(err.to_string().starts_with("negative length -1")); +/// ``` #[macro_export] macro_rules! vortex_err { - (Other: $($tts:tt)*) => { - $crate::__private::fmt_err($crate::VortexError::Other, &format_args!($($tts)*)) - }; - (AssertionFailed: $($tts:tt)*) => { - $crate::__private::fmt_err($crate::VortexError::AssertionFailed, &format_args!($($tts)*)) - }; - (IOError: $($tts:tt)*) => {{ - use std::backtrace::Backtrace; - $crate::__private::must_use( - $crate::VortexError::IOError(err_string.into(), Box::new(Backtrace::capture())) - ) - }}; - (OutOfBounds: $idx:expr, $start:expr, $stop:expr) => { - $crate::__private::out_of_bounds($idx, $start, $stop) - }; - (NotImplemented: $func:expr, $by_whom:expr) => { - $crate::__private::not_implemented($func, &format_args!("{}", $by_whom)) - }; - (MismatchedTypes: $expected:expr, $actual:expr) => { - $crate::__private::mismatched_types(&$expected, &$actual) - }; (Context: $msg:literal, $err:expr) => { $crate::__private::context($msg, $err) }; - (External: $err:expr) => { + (External: $err:expr $(,)?) => { $crate::__private::external($err) }; - ($variant:ident: $fmt:literal $(, $arg:expr)* $(,)?) => { - $crate::__private::fmt_err($crate::VortexError::$variant, &format_args!($fmt $(, $arg)*)) - }; - ($variant:ident: $err:expr $(,)?) => { - $crate::__private::must_use( - $crate::VortexError::$variant($err) - ) - }; ($fmt:literal $(, $arg:expr)* $(,)?) => { - $crate::vortex_err!(Other: $fmt, $($arg),*) + $crate::__private::fmt_err(&format_args!($fmt $(, $arg)*)) + }; + ($err:expr $(,)?) => { + $crate::__private::fmt_err(&format_args!("{}", $err)) }; } @@ -415,10 +307,12 @@ macro_rules! vortex_bail { /// A macro that mirrors `assert!` but instead of panicking on a failed condition, /// it will immediately return an erroneous `VortexResult` to the calling context. +/// +/// Without a message the error names the condition. #[macro_export] macro_rules! vortex_ensure { ($cond:expr) => { - $crate::vortex_ensure!($cond, AssertionFailed: "{}", stringify!($cond)); + $crate::vortex_ensure!($cond, "{}", stringify!($cond)); }; ($cond:expr, $($tt:tt)*) => { if !$cond { @@ -433,8 +327,8 @@ macro_rules! vortex_ensure { /// Both values must implement [`PartialEq`] and [`Display`], and each is /// evaluated exactly once. Use [`vortex_ensure!`] for values that do not implement `Display`. /// -/// By default this returns an [`AssertionFailed`](VortexError::AssertionFailed) error that -/// shows both expressions and their values, so most callers do not need a message: +/// By default the error shows both expressions and their values, so most callers do not need a +/// message: /// /// ``` /// # use vortex_error::{VortexResult, vortex_ensure_eq}; @@ -444,23 +338,21 @@ macro_rules! vortex_ensure { /// } /// /// let err = check(3).unwrap_err().to_string(); -/// assert!(err.starts_with("Assertion failed error: `len == 2`\n left: 3\n right: 2")); +/// assert!(err.starts_with("`len == 2`\n left: 3\n right: 2")); /// ``` /// -/// A custom message, optionally prefixed with an error variant, replaces the expressions. The -/// values are still appended, so the message does not need to repeat them: +/// A message replaces the expressions. The values are still appended, so the message does not +/// need to repeat them: /// /// ``` /// # use vortex_error::{VortexResult, vortex_ensure_eq}; /// fn check(len: usize) -> VortexResult<()> { -/// vortex_ensure_eq!(len, 2, InvalidArgument: "map entries must have {} fields", 2); +/// vortex_ensure_eq!(len, 2, "map entries must have {} fields", 2); /// Ok(()) /// } /// /// let err = check(3).unwrap_err().to_string(); -/// assert!(err.starts_with( -/// "Invalid argument error: map entries must have 2 fields\n left: 3\n right: 2" -/// )); +/// assert!(err.starts_with("map entries must have 2 fields\n left: 3\n right: 2")); /// ``` #[macro_export] macro_rules! vortex_ensure_eq { @@ -469,7 +361,7 @@ macro_rules! vortex_ensure_eq { (left_val, right_val) => { if *left_val != *right_val { $crate::vortex_bail!( - AssertionFailed: "`{} == {}`\n left: {}\n right: {}", + "`{} == {}`\n left: {}\n right: {}", stringify!($left), stringify!($right), left_val, @@ -479,12 +371,12 @@ macro_rules! vortex_ensure_eq { } } }; - ($left:expr, $right:expr, $variant:ident: $fmt:literal $(, $arg:expr)* $(,)?) => { + ($left:expr, $right:expr, $fmt:literal $(, $arg:expr)* $(,)?) => { match (&$left, &$right) { (left_val, right_val) => { if *left_val != *right_val { $crate::vortex_bail!( - $variant: "{}\n left: {}\n right: {}", + "{}\n left: {}\n right: {}", format_args!($fmt $(, $arg)*), left_val, right_val @@ -493,73 +385,65 @@ macro_rules! vortex_ensure_eq { } } }; - ($left:expr, $right:expr, $fmt:literal $(, $arg:expr)* $(,)?) => { - $crate::vortex_ensure_eq!($left, $right, Other: $fmt $(, $arg)*) - }; } /// A convenient macro for panicking with a VortexError in the presence of a programmer error /// (e.g., an invariant has been violated). +/// +/// It takes a message like [`vortex_err!`], or an existing error with an optional message to add +/// as context. #[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),*)) + // Before the `$err:expr` arms, which a bare message would otherwise match. + ($fmt:literal $(, $arg:expr)* $(,)?) => { + $crate::vortex_panic!($crate::vortex_err!($fmt $(, $arg)*)) }; ($err:expr, $fmt:literal $(, $arg:expr)* $(,)?) => { $crate::__private::panic_err_ctx($err, &format_args!($fmt $(, $arg)*)) }; - ($fmt:literal $(, $arg:expr)* $(,)?) => { - $crate::__private::panic_fmt(&format_args!($fmt $(, $arg)*)) - }; ($err:expr) => { $crate::__private::panic_err($err) }; } +impl From for VortexError { + fn from(_: Infallible) -> Self { + unreachable!() + } +} + impl From for VortexError { fn from(value: arrow_schema::ArrowError) -> Self { - VortexError::Arrow(value, Box::new(Backtrace::capture())) + VortexError::external(value) } } #[cfg(feature = "flatbuffers")] impl From for VortexError { fn from(value: flatbuffers::InvalidFlatbuffer) -> Self { - VortexError::FlatBuffers(value, Box::new(Backtrace::capture())) + VortexError::external(value) } } impl From for VortexError { fn from(value: io::Error) -> Self { - VortexError::Io(value, Box::new(Backtrace::capture())) + VortexError::external(value) } } #[cfg(feature = "object_store")] impl From for VortexError { fn from(value: object_store::Error) -> Self { - VortexError::ObjectStore(value, Box::new(Backtrace::capture())) + VortexError::external(value) } } impl From for VortexError { fn from(value: jiff::Error) -> Self { - VortexError::Jiff(value, Box::new(Backtrace::capture())) + VortexError::external(value) } } @@ -569,32 +453,32 @@ impl From for VortexError { if value.is_panic() { std::panic::resume_unwind(value.into_panic()) } else { - VortexError::Join(value, Box::new(Backtrace::capture())) + VortexError::external(value) } } } impl From for VortexError { fn from(value: TryFromIntError) -> Self { - VortexError::TryFromInt(value, Box::new(Backtrace::capture())) + VortexError::external(value) } } impl From for VortexError { fn from(value: prost::EncodeError) -> Self { - Self::Prost(Box::new(value), Box::new(Backtrace::capture())) + VortexError::external(value) } } impl From for VortexError { fn from(value: prost::DecodeError) -> Self { - Self::Prost(Box::new(value), Box::new(Backtrace::capture())) + VortexError::external(value) } } impl From for VortexError { fn from(value: prost::UnknownEnumValue) -> Self { - Self::Prost(Box::new(value), Box::new(Backtrace::capture())) + VortexError::external(value) } } @@ -610,12 +494,10 @@ impl From for VortexError { reason = "cold handlers for vortex_panic!/vortex_expect intentionally panic" )] pub mod __private { - use std::backtrace::Backtrace; + use std::error::Error; - use std::fmt; use std::fmt::Arguments; - use crate::ErrString; use crate::VortexError; #[doc(hidden)] @@ -630,43 +512,8 @@ pub mod __private { #[cold] #[inline(never)] #[must_use] - pub fn fmt_err( - variant: fn(ErrString, Box) -> VortexError, - args: &Arguments<'_>, - ) -> VortexError { - variant(args.to_string().into(), Box::new(Backtrace::capture())) - } - - #[doc(hidden)] - #[cold] - #[inline(never)] - #[must_use] - pub fn out_of_bounds(idx: usize, start: usize, stop: usize) -> VortexError { - VortexError::OutOfBounds(idx, start, stop, Box::new(Backtrace::capture())) - } - - #[doc(hidden)] - #[cold] - #[inline(never)] - #[must_use] - pub fn not_implemented(func: impl Into, by_whom: &Arguments<'_>) -> VortexError { - VortexError::NotImplemented( - func.into(), - by_whom.to_string().into(), - Box::new(Backtrace::capture()), - ) - } - - #[doc(hidden)] - #[cold] - #[inline(never)] - #[must_use] - pub fn mismatched_types(expected: &dyn fmt::Display, actual: &dyn fmt::Display) -> VortexError { - VortexError::MismatchedTypes( - expected.to_string().into(), - actual.to_string().into(), - Box::new(Backtrace::capture()), - ) + pub fn fmt_err(args: &Arguments<'_>) -> VortexError { + VortexError::new(args.to_string()) } #[doc(hidden)] @@ -674,7 +521,7 @@ pub mod __private { #[inline(never)] #[must_use] pub fn context(msg: &'static str, err: VortexError) -> VortexError { - VortexError::Context(msg.into(), Box::new(err)) + err.with_context(msg) } #[doc(hidden)] @@ -682,7 +529,7 @@ pub mod __private { #[inline(never)] #[must_use] pub fn external(err: impl Into>) -> VortexError { - VortexError::External(err.into(), Box::new(Backtrace::capture())) + VortexError::external(err) } #[doc(hidden)] @@ -692,16 +539,6 @@ pub mod __private { panic!("{err}") } - #[doc(hidden)] - #[cold] - #[inline(never)] - pub fn panic_fmt(args: &Arguments<'_>) -> ! { - panic!( - "{}", - VortexError::Other(args.to_string().into(), Box::new(Backtrace::capture())) - ) - } - #[doc(hidden)] #[cold] #[inline(never)] @@ -720,9 +557,6 @@ pub mod __private { #[cold] #[inline(never)] pub fn option_expect_failed(msg: &'static str) -> ! { - panic!( - "{}", - VortexError::AssertionFailed(msg.into(), Box::new(Backtrace::capture())) - ) + panic!("{}", VortexError::new(msg)) } } diff --git a/vortex-error/tests/ensure.rs b/vortex-error/tests/ensure.rs index ac13077be49..ba0f046670b 100644 --- a/vortex-error/tests/ensure.rs +++ b/vortex-error/tests/ensure.rs @@ -10,6 +10,15 @@ mod tests { use vortex_error::VortexError; use vortex_error::VortexResult; + /// Asserts the message shown, ignoring a captured backtrace. + fn assert_error(error: &VortexError, message: &str) { + let shown = error.to_string(); + assert!( + shown == message || shown.starts_with(&format!("{message}\nBacktrace:")), + "{shown:?}" + ); + } + fn check_default(left: &Cell, right: &Cell) -> VortexResult<()> { vortex_error::vortex_ensure_eq!( left.replace(left.get() + 1), @@ -39,12 +48,9 @@ mod tests { assert_eq!(left.get(), 2); assert_eq!(right.get(), 3); - let VortexError::AssertionFailed(message, _) = error else { - panic!("expected AssertionFailed, got {error}"); - }; - assert_eq!( - message.as_ref(), - "`left.replace(left.get() + 1) == right.replace(right.get() + 1)`\n left: 1\n right: 2" + assert_error( + &error, + "`left.replace(left.get() + 1) == right.replace(right.get() + 1)`\n left: 1\n right: 2", ); } @@ -70,31 +76,22 @@ mod tests { let error = check(&right).unwrap_err(); assert_eq!(message_calls.get(), 1); - let VortexError::Other(message, _) = error else { - panic!("expected Other, got {error}"); - }; - assert_eq!( - message.as_ref(), - "field mismatch 0\n left: left\n right: right" - ); + assert_error(&error, "field mismatch 0\n left: left\n right: right"); assert_eq!(left, "left"); assert_eq!(right, "right"); Ok(()) } #[test] - fn explicit_variant_keeps_context_and_values() { + fn message_keeps_context_and_values() { let check = || -> VortexResult<()> { - vortex_error::vortex_ensure_eq!(1, 2, InvalidArgument: "expected {} fields", 2,); + vortex_error::vortex_ensure_eq!(1, 2, "expected {} fields", 2,); Ok(()) }; let error = check().unwrap_err(); - let VortexError::InvalidArgument(message, _) = error else { - panic!("expected InvalidArgument, got {error}"); - }; - assert_eq!(message.as_ref(), "expected 2 fields\n left: 1\n right: 2"); + assert_error(&error, "expected 2 fields\n left: 1\n right: 2"); } #[test] @@ -106,9 +103,6 @@ mod tests { let error = check().unwrap_err(); - let VortexError::AssertionFailed(message, _) = error else { - panic!("expected AssertionFailed, got {error}"); - }; - assert_eq!(message.as_ref(), "false"); + assert_error(&error, "false"); } } diff --git a/vortex-error/tests/error.rs b/vortex-error/tests/error.rs new file mode 100644 index 00000000000..3592bb87ff4 --- /dev/null +++ b/vortex-error/tests/error.rs @@ -0,0 +1,117 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright the Vortex contributors + +// This seems to be a bug in the lint - https://github.com/rust-lang/rust-clippy/issues/11024 +#![expect(clippy::tests_outside_test_module)] + +use std::error::Error; +use std::io; +use std::sync::Arc; + +use vortex_error::VortexError; +use vortex_error::vortex_err; + +#[test] +fn test_message_is_displayed_as_written() { + assert!( + vortex_err!("index {} past {}", 5, 3) + .to_string() + .starts_with("index 5 past 3") + ); +} + +#[test] +fn test_with_context_preserves_source() { + let err = VortexError::from(io::Error::other("disk on fire")).with_context("while reading"); + + assert!( + err.source() + .and_then(|source| source.downcast_ref::()) + .is_some(), + "context must not hide the underlying error" + ); + + let display = err.to_string(); + assert!( + display.starts_with("while reading:\n disk on fire"), + "{display}" + ); +} + +#[test] +fn test_nested_context() { + let err = vortex_err!("bad footer") + .with_context("reading layout") + .with_context("opening file"); + + let display = err.to_string(); + assert!( + display.starts_with("opening file:\n reading layout:\n bad footer"), + "{display}" + ); +} + +#[test] +fn test_external_preserves_concrete_source() { + let err = vortex_err!(External: io::Error::from(io::ErrorKind::NotFound)); + + assert_eq!( + err.source() + .and_then(|source| source.downcast_ref::()) + .map(io::Error::kind), + Some(io::ErrorKind::NotFound) + ); +} + +#[test] +fn test_clone_and_shared_round_trip() { + let err = vortex_err!("device disappeared"); + let clone = err.clone(); + + assert_eq!(clone.to_string(), err.to_string()); + + // An `Arc` is unwrapped when uniquely held and cloned otherwise; both keep the message. + let shared = Arc::new(err); + let borrowed = VortexError::from(&shared); + let owned = VortexError::from(shared); + + assert_eq!(borrowed.to_string(), owned.to_string()); +} + +#[test] +fn test_context_on_a_clone_leaves_the_other_clone_alone() { + let err = VortexError::from(io::Error::other("disk on fire")); + let clone = err.clone(); + + // The payload is shared, so this context cannot be written into it in place. + let with_context = clone.with_context("while reading"); + + let original = err.to_string(); + assert!( + original.starts_with("disk on fire") && !original.contains("while reading"), + "{original}" + ); + let display = with_context.to_string(); + assert!( + display.starts_with("while reading:\n disk on fire"), + "{display}" + ); + assert!( + with_context + .source() + .and_then(|source| source.downcast_ref::()) + .is_some(), + "context on a shared error must still expose the underlying error" + ); +} + +#[test] +fn test_arrow_error_is_kept_as_the_source() { + let err = VortexError::from(arrow_schema::ArrowError::ComputeError("x".into())); + assert!( + err.source() + .and_then(|source| source.downcast_ref::()) + .is_some(), + "the Arrow error is kept as the source" + ); +} diff --git a/vortex-error/tests/fmt.rs b/vortex-error/tests/fmt.rs index 8aba428a3ff..c09458493aa 100644 --- a/vortex-error/tests/fmt.rs +++ b/vortex-error/tests/fmt.rs @@ -17,7 +17,7 @@ fn test_basic_display() { let display = err.to_string(); assert!( - display.contains("Other error: this is bad"), + display.contains("this is bad"), "should contain error message" ); assert!( @@ -45,8 +45,7 @@ fn test_from_arrow_with_backtrace() { let display = vx_error.to_string(); assert!( - display - .contains("Arrow error: Not yet implemented: This feature isn't implemented yet"), + display.contains("Not yet implemented: This feature isn't implemented yet"), "should contain arrow error message" ); assert!( @@ -71,9 +70,7 @@ fn test_from_arrow_no_backtrace() { let display = vx_error.to_string(); assert!( - display.contains( - "Arrow error: Not yet implemented: This feature isn't implemented yet" - ), + display.contains("Not yet implemented: This feature isn't implemented yet"), "should contain arrow error message" ); assert!( diff --git a/vortex-ffi/cinclude/vortex.h b/vortex-ffi/cinclude/vortex.h index dca15d82ee2..da574f03b4f 100644 --- a/vortex-ffi/cinclude/vortex.h +++ b/vortex-ffi/cinclude/vortex.h @@ -211,41 +211,9 @@ typedef enum { */ typedef enum { /** - * All other errors + * An error returned by Vortex */ VX_ERROR_CODE_OTHER = 0, - /** - * Index out of bounds - */ - VX_ERROR_CODE_OUT_OF_BOUNDS = 1, - /** - * Compute kernel execute error - */ - VX_ERROR_CODE_COMPUTE = 2, - /** - * An invalid argument was provided. - */ - VX_ERROR_CODE_INVALID_ARGUMENT = 3, - /** - * Serialization/deserialization error - */ - VX_ERROR_CODE_SERIALIZATION = 4, - /** - * Unimplemented function - */ - VX_ERROR_CODE_NOT_IMPLEMENTED = 5, - /** - * Type mismatch - */ - VX_ERROR_CODE_MISMATCHED_TYPES = 6, - /** - * Assertion failed - */ - VX_ERROR_CODE_ASSERTION_FAILED = 7, - /** - * IO error - */ - VX_ERROR_CODE_IO = 8, /** * Panic inside FFI */ diff --git a/vortex-ffi/src/dtype.rs b/vortex-ffi/src/dtype.rs index e0dc2467914..316b7e35c70 100644 --- a/vortex-ffi/src/dtype.rs +++ b/vortex-ffi/src/dtype.rs @@ -76,7 +76,9 @@ impl From<&DType> for vx_dtype_variant { DType::Struct(..) => vx_dtype_variant::DTYPE_STRUCT, DType::Map(..) => vortex_panic!("Map is not supported in FFI yet"), DType::Union(..) => vortex_panic!("Union is not supported in FFI yet"), - DType::Variant(_) => vortex_panic!("Variant is not supported in FFI yet"), + DType::Variant(_) => { + vortex_panic!("Variant is not supported in FFI yet") + } DType::Extension(_) => vx_dtype_variant::DTYPE_EXTENSION, } } diff --git a/vortex-ffi/src/error.rs b/vortex-ffi/src/error.rs index 8f13a699e59..2b2956cf385 100644 --- a/vortex-ffi/src/error.rs +++ b/vortex-ffi/src/error.rs @@ -7,7 +7,6 @@ use std::panic::catch_unwind; use std::ptr; use std::sync::Arc; -use vortex::error::VortexError; use vortex::error::VortexResult; use crate::box_wrapper; @@ -17,44 +16,14 @@ use crate::string::vx_view; #[repr(C)] #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub enum vx_error_code { - /// All other errors + /// An error returned by Vortex VX_ERROR_CODE_OTHER = 0, - /// Index out of bounds - VX_ERROR_CODE_OUT_OF_BOUNDS = 1, - /// Compute kernel execute error - VX_ERROR_CODE_COMPUTE = 2, - /// An invalid argument was provided. - VX_ERROR_CODE_INVALID_ARGUMENT = 3, - /// Serialization/deserialization error - VX_ERROR_CODE_SERIALIZATION = 4, - /// Unimplemented function - VX_ERROR_CODE_NOT_IMPLEMENTED = 5, - /// Type mismatch - VX_ERROR_CODE_MISMATCHED_TYPES = 6, - /// Assertion failed - VX_ERROR_CODE_ASSERTION_FAILED = 7, - /// IO error - VX_ERROR_CODE_IO = 8, + // 1 to 8, 10 and 11 are retired. They classified Vortex errors, which no longer carry a + // classification. Leaving them unused keeps VX_ERROR_CODE_PANIC stable. /// Panic inside FFI VX_ERROR_CODE_PANIC = 9, } -fn error_code(error: &VortexError) -> vx_error_code { - match error { - VortexError::OutOfBounds(..) => vx_error_code::VX_ERROR_CODE_OUT_OF_BOUNDS, - VortexError::Compute(..) => vx_error_code::VX_ERROR_CODE_COMPUTE, - VortexError::InvalidArgument(..) => vx_error_code::VX_ERROR_CODE_INVALID_ARGUMENT, - VortexError::Serde(..) => vx_error_code::VX_ERROR_CODE_SERIALIZATION, - VortexError::NotImplemented(..) => vx_error_code::VX_ERROR_CODE_NOT_IMPLEMENTED, - VortexError::MismatchedTypes(..) => vx_error_code::VX_ERROR_CODE_MISMATCHED_TYPES, - VortexError::AssertionFailed(..) => vx_error_code::VX_ERROR_CODE_ASSERTION_FAILED, - VortexError::Io(..) => vx_error_code::VX_ERROR_CODE_IO, - VortexError::Context(_, inner) => error_code(inner), - VortexError::Shared(inner) => error_code(inner), - _ => vx_error_code::VX_ERROR_CODE_OTHER, - } -} - pub(crate) struct VortexFFIError { message: Arc, code: vx_error_code, @@ -116,7 +85,7 @@ pub fn try_or_default( value } Ok(Err(err)) => { - write_error_with_code(error_out, &err.to_string(), error_code(&err)); + write_error(error_out, &err.to_string()); T::default() } Err(payload) => { @@ -145,7 +114,7 @@ pub fn try_or( value } Ok(Err(err)) => { - write_error_with_code(error_out, &err.to_string(), error_code(&err)); + write_error(error_out, &err.to_string()); error_value } Err(payload) => { @@ -232,13 +201,16 @@ mod tests { assert_eq!( try_or(&raw mut error, -1, || Err::(vortex_err!( - OutOfBounds: 5, 0, 3 + "index {} out of bounds from {} to {}", + 5, + 0, + 3 ))), -1 ); assert_eq!( unsafe { vx_error_get_code(error) }, - vx_error_code::VX_ERROR_CODE_OUT_OF_BOUNDS + vx_error_code::VX_ERROR_CODE_OTHER ); unsafe { vx_error_free(error) }; diff --git a/vortex-file/src/lib.rs b/vortex-file/src/lib.rs index 1c309112078..b46c766403c 100644 --- a/vortex-file/src/lib.rs +++ b/vortex-file/src/lib.rs @@ -205,7 +205,6 @@ pub(crate) fn enable_all_registered_array_encodings(session: &VortexSession) { use vortex_edition::EditionInclusion; use vortex_edition::EditionSessionExt; use vortex_error::VortexExpect; - use vortex_error::vortex_err; use vortex_layout::session::LayoutSessionExt; const TEST_EDITION: EditionId = EditionId::new("test", 2026, 7, 0); @@ -216,7 +215,6 @@ pub(crate) fn enable_all_registered_array_encodings(session: &VortexSession) { id: TEST_EDITION, min_library_version: None, }) - .map_err(|error| vortex_err!("{error}")) .vortex_expect("test edition is valid"); let component_ids = [ ( @@ -245,7 +243,6 @@ pub(crate) fn enable_all_registered_array_encodings(session: &VortexSession) { for id in ids { editions .declare_inclusion(EditionInclusion::new(kind, &id, TEST_EDITION)) - .map_err(|error| vortex_err!("{error}")) .vortex_expect("registered component has one test-edition inclusion"); } } @@ -263,12 +260,10 @@ pub(crate) fn enable_all_registered_array_encodings(session: &VortexSession) { id, TEST_EDITION, )) - .map_err(|error| vortex_err!("{error}")) .vortex_expect("default aggregate has one test-edition inclusion"); } session .enable_edition(TEST_EDITION) - .map_err(|error| vortex_err!("{error}")) .vortex_expect("test edition is registered"); } diff --git a/vortex-file/tests/common/mod.rs b/vortex-file/tests/common/mod.rs index 29b8ac0024f..3cffd4f9eeb 100644 --- a/vortex-file/tests/common/mod.rs +++ b/vortex-file/tests/common/mod.rs @@ -9,7 +9,6 @@ use vortex_edition::EditionId; use vortex_edition::EditionInclusion; use vortex_edition::EditionSessionExt; use vortex_error::VortexExpect; -use vortex_error::vortex_err; use vortex_layout::session::LayoutSessionExt; use vortex_session::VortexSession; @@ -23,7 +22,6 @@ pub fn enable_all_registered_array_encodings(session: &VortexSession) { id: TEST_EDITION, min_library_version: None, }) - .map_err(|error| vortex_err!("{error}")) .vortex_expect("test edition is valid"); let component_ids = [ ( @@ -52,7 +50,6 @@ pub fn enable_all_registered_array_encodings(session: &VortexSession) { for id in ids { editions .declare_inclusion(EditionInclusion::new(kind, &id, TEST_EDITION)) - .map_err(|error| vortex_err!("{error}")) .vortex_expect("registered component has one test-edition inclusion"); } } @@ -70,11 +67,9 @@ pub fn enable_all_registered_array_encodings(session: &VortexSession) { id, TEST_EDITION, )) - .map_err(|error| vortex_err!("{error}")) .vortex_expect("default aggregate has one test-edition inclusion"); } session .enable_edition(TEST_EDITION) - .map_err(|error| vortex_err!("{error}")) .vortex_expect("test edition is registered"); } diff --git a/vortex-jni/src/session.rs b/vortex-jni/src/session.rs index 8132fd8213a..82f3185ffd9 100644 --- a/vortex-jni/src/session.rs +++ b/vortex-jni/src/session.rs @@ -10,7 +10,6 @@ use vortex::VortexSessionDefault; use vortex::editions::CORE_2026_08_3; use vortex::editions::EditionSessionExt; use vortex::error::VortexExpect; -use vortex::error::vortex_err; use vortex::io::runtime::BlockingRuntime; use vortex::io::session::RuntimeSessionExt; use vortex::session::VortexSession; @@ -23,7 +22,6 @@ pub(crate) fn new_session() -> Box { let session = VortexSession::default().with_handle(RUNTIME.handle()); session .enable_edition(CORE_2026_08_3) - .map_err(|error| vortex_err!("{error}")) .vortex_expect("JNI-supported draft core edition is registered"); vortex_parquet_variant::initialize(&session); vortex_spatial::initialize(&session); diff --git a/vortex-json/src/lib.rs b/vortex-json/src/lib.rs index 0cf27f2db2f..15c06c82b9f 100644 --- a/vortex-json/src/lib.rs +++ b/vortex-json/src/lib.rs @@ -26,7 +26,6 @@ use vortex_array::scalar_fn::session::ScalarFnSessionExt; use vortex_arrow::ArrowSessionExt; use vortex_edition::EditionSessionExt; use vortex_error::VortexExpect; -use vortex_error::vortex_err; use vortex_session::VortexSession; /// Register JSON extension support with a session. @@ -42,15 +41,12 @@ pub fn initialize(session: &VortexSession) { session .editions() .declare_family(&editions::FAMILY) - .map_err(|error| vortex_err!("{error}")) .vortex_expect("JSON edition family is valid"); session .register_edition(&editions::DECLARATION) - .map_err(|error| vortex_err!("{error}")) .vortex_expect("JSON edition declaration is valid"); } session .enable_edition(editions::JSON_2026_08) - .map_err(|error| vortex_err!("{error}")) .vortex_expect("JSON edition is registered"); } diff --git a/vortex-row/src/codec.rs b/vortex-row/src/codec.rs index 2814ca03390..9b621200d48 100644 --- a/vortex-row/src/codec.rs +++ b/vortex-row/src/codec.rs @@ -273,7 +273,9 @@ pub(crate) fn row_width_for_dtype(dtype: &DType) -> VortexResult { DType::Variant(_) => { vortex_bail!("row encoding does not support Variant arrays (no well-defined ordering)") } - DType::Union(..) => vortex_bail!("row encoding does not support Union arrays"), + DType::Union(..) => { + vortex_bail!("row encoding does not support Union arrays") + } dtype => vortex_bail!("row encoding does not support dtype: {dtype:?}"), } } diff --git a/vortex-spatial/src/extension/coordinate.rs b/vortex-spatial/src/extension/coordinate.rs index 43599a705b7..fd20e3bb619 100644 --- a/vortex-spatial/src/extension/coordinate.rs +++ b/vortex-spatial/src/extension/coordinate.rs @@ -59,7 +59,9 @@ impl Dimension { ["x", "y", "z"] => Dimension::Xyz, ["x", "y", "m"] => Dimension::Xym, ["x", "y", "z", "m"] => Dimension::Xyzm, - _ => vortex_bail!("not a valid GeoArrow coordinate dimension: {names:?}"), + _ => { + vortex_bail!("not a valid GeoArrow coordinate dimension: {names:?}") + } }) } diff --git a/vortex-spatial/src/extension/linestring.rs b/vortex-spatial/src/extension/linestring.rs index 824692617da..0fcd508a63d 100644 --- a/vortex-spatial/src/extension/linestring.rs +++ b/vortex-spatial/src/extension/linestring.rs @@ -336,7 +336,7 @@ impl ArrowImportVTable for LineString { vortex_ensure!( linestring_meta.coord_type() == CoordType::Separated, "geoarrow.linestring with interleaved coordinates is not supported; \ - re-encode with separated (struct) coordinates" + re-encode with separated (struct) coordinates" ); ( linestring_meta.dimension().into(), diff --git a/vortex-spatial/src/extension/multilinestring.rs b/vortex-spatial/src/extension/multilinestring.rs index 93ff0e31c8e..0443f615211 100644 --- a/vortex-spatial/src/extension/multilinestring.rs +++ b/vortex-spatial/src/extension/multilinestring.rs @@ -264,7 +264,7 @@ impl ArrowImportVTable for MultiLineString { vortex_ensure!( multilinestring_meta.coord_type() == CoordType::Separated, "geoarrow.multilinestring with interleaved coordinates is not supported; \ - re-encode with separated (struct) coordinates" + re-encode with separated (struct) coordinates" ); ( multilinestring_meta.dimension().into(), diff --git a/vortex-spatial/src/extension/multipoint.rs b/vortex-spatial/src/extension/multipoint.rs index c0a2f9d95a5..2f3f04662e4 100644 --- a/vortex-spatial/src/extension/multipoint.rs +++ b/vortex-spatial/src/extension/multipoint.rs @@ -251,7 +251,7 @@ impl ArrowImportVTable for MultiPoint { vortex_ensure!( multipoint_meta.coord_type() == CoordType::Separated, "geoarrow.multipoint with interleaved coordinates is not supported; \ - re-encode with separated (struct) coordinates" + re-encode with separated (struct) coordinates" ); ( multipoint_meta.dimension().into(), diff --git a/vortex-spatial/src/extension/multipolygon.rs b/vortex-spatial/src/extension/multipolygon.rs index f6891e7708e..c7e88c4b71b 100644 --- a/vortex-spatial/src/extension/multipolygon.rs +++ b/vortex-spatial/src/extension/multipolygon.rs @@ -260,7 +260,7 @@ impl ArrowImportVTable for MultiPolygon { vortex_ensure!( multipolygon_meta.coord_type() == CoordType::Separated, "geoarrow.multipolygon with interleaved coordinates is not supported; \ - re-encode with separated (struct) coordinates" + re-encode with separated (struct) coordinates" ); ( multipolygon_meta.dimension().into(), diff --git a/vortex-spatial/src/extension/polygon.rs b/vortex-spatial/src/extension/polygon.rs index 73b39597899..4d2f632c521 100644 --- a/vortex-spatial/src/extension/polygon.rs +++ b/vortex-spatial/src/extension/polygon.rs @@ -324,7 +324,7 @@ impl ArrowImportVTable for Polygon { vortex_ensure!( polygon_meta.coord_type() == CoordType::Separated, "geoarrow.polygon with interleaved coordinates is not supported; \ - re-encode with separated (struct) coordinates" + re-encode with separated (struct) coordinates" ); ( polygon_meta.dimension().into(), diff --git a/vortex-spatial/src/lib.rs b/vortex-spatial/src/lib.rs index 26287e64d39..ecb3e1e55fd 100644 --- a/vortex-spatial/src/lib.rs +++ b/vortex-spatial/src/lib.rs @@ -11,7 +11,6 @@ use vortex_array::scalar_fn::session::ScalarFnSessionExt; use vortex_arrow::ArrowSessionExt; use vortex_edition::EditionSessionExt; use vortex_error::VortexExpect; -use vortex_error::vortex_err; use vortex_expr::stats::session::StatsSessionExt; use vortex_session::VortexSession; @@ -104,15 +103,12 @@ pub fn initialize(session: &VortexSession) { session .editions() .declare_family(&editions::FAMILY) - .map_err(|error| vortex_err!("{error}")) .vortex_expect("spatial edition family is valid"); session .register_edition(&editions::DECLARATION) - .map_err(|error| vortex_err!("{error}")) .vortex_expect("spatial edition declaration is valid"); } session .enable_edition(editions::SPATIAL_2026_08) - .map_err(|error| vortex_err!("{error}")) .vortex_expect("spatial edition is registered"); } diff --git a/vortex-tensor/src/lib.rs b/vortex-tensor/src/lib.rs index a6f9ea2a623..b2939c21d82 100644 --- a/vortex-tensor/src/lib.rs +++ b/vortex-tensor/src/lib.rs @@ -21,7 +21,6 @@ use vortex_array::session::ArraySessionExt; use vortex_arrow::ArrowSessionExt; use vortex_edition::EditionSessionExt; use vortex_error::VortexExpect; -use vortex_error::vortex_err; use vortex_session::VortexSession; use crate::scalar_fns::cosine_similarity::CosineSimilarity; @@ -84,16 +83,13 @@ pub fn initialize(session: &VortexSession) { session .editions() .declare_family(&editions::FAMILY) - .map_err(|error| vortex_err!("{error}")) .vortex_expect("tensor edition family is valid"); session .register_edition(&editions::DECLARATION) - .map_err(|error| vortex_err!("{error}")) .vortex_expect("tensor edition declaration is valid"); } session .enable_edition(editions::TENSOR_2026_04) - .map_err(|error| vortex_err!("{error}")) .vortex_expect("tensor edition is registered"); } diff --git a/vortex/benches/common_encoding_tree_throughput.rs b/vortex/benches/common_encoding_tree_throughput.rs index 0d523c133e0..e2bfcabfd4c 100644 --- a/vortex/benches/common_encoding_tree_throughput.rs +++ b/vortex/benches/common_encoding_tree_throughput.rs @@ -110,7 +110,9 @@ mod setup { fn global_reference(array: &FoRArray) -> Scalar { match array.references() { FoRReferences::Global(reference) => reference.clone(), - FoRReferences::Blocked(_) => vortex_panic!("FoR::encode uses a global reference"), + FoRReferences::Blocked(_) => { + vortex_panic!("FoR::encode uses a global reference") + } } } diff --git a/vortex/src/editions/mod.rs b/vortex/src/editions/mod.rs index 464cce2197b..36a2927ec21 100644 --- a/vortex/src/editions/mod.rs +++ b/vortex/src/editions/mod.rs @@ -44,7 +44,6 @@ pub use vortex_edition::declarations::core::CORE_2026_08_3; pub use vortex_edition::declarations::preview; pub use vortex_edition::declarations::preview::PREVIEW_2026_08_0; use vortex_error::VortexExpect; -use vortex_error::vortex_err; use vortex_session::VortexSession; /// The `core` edition enabled for writing by the default Vortex session. @@ -60,13 +59,11 @@ pub fn register_default_editions(session: &VortexSession) { session .editions() .declare_family(family) - .map_err(|e| vortex_err!("{e}")) .vortex_expect("edition families are valid"); } for declaration in EDITION_DECLARATIONS { session .register_edition(declaration) - .map_err(|e| vortex_err!("{e}")) .vortex_expect("edition declarations are valid"); } } @@ -78,6 +75,5 @@ pub fn register_default_editions(session: &VortexSession) { pub fn enable_default_editions(session: &VortexSession) { session .enable_edition(DEFAULT_CORE_EDITION) - .map_err(|e| vortex_err!("{e}")) .vortex_expect("default core edition is registered"); }