Skip to content

Commit f3cbb32

Browse files
authored
Simplify error types, remove kind from struct (#10080)
Instead of error being an enum we convert an error to be a struct with message and error kind. I had followed the structure of Python errors which have couple error categories https://docs.python.org/3/builtins/exceptions.html#exception-hierarchy majority of which don't apply to this project Signed-off-by: Robert Kruszewski <github@robertk.io>
1 parent e5a4e66 commit f3cbb32

143 files changed

Lines changed: 1026 additions & 909 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎STYLE.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,8 @@
5555
- `vortex_ensure!` and `vortex_ensure_eq!` for checking conditions. Prefer
5656
`vortex_ensure_eq!(a, b)` with no message: the error already shows both expressions and values.
5757
Add a message only for context the expressions lack, and never repeat the values in it.
58-
- Add context to errors using `.with_context()`
58+
- Add context to an existing error with `.with_context()`, which keeps its source and backtrace;
59+
do not format it into a new `vortex_err!`, which discards them.
5960
- Include backtraces for better debugging
6061
- Use `VortexExpect` trait when unwrapping is appropriate with proper error context.
6162

‎encodings/alp/src/alp/array.rs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ impl VTable for ALP {
144144
let encoded_ptype = match &dtype {
145145
DType::Primitive(PType::F32, n) => DType::Primitive(PType::I32, *n),
146146
DType::Primitive(PType::F64, n) => DType::Primitive(PType::I64, *n),
147-
d => vortex_bail!(MismatchedTypes: "f32 or f64", d),
147+
d => vortex_bail!("expected type: f32 or f64 but instead got {}", d),
148148
};
149149
let encoded = children.get(0, &encoded_ptype, len)?;
150150

@@ -319,7 +319,9 @@ impl ALPData {
319319
DType::Primitive(PType::I64, nullability) => {
320320
Ok(DType::Primitive(PType::F64, *nullability))
321321
}
322-
_ => vortex_bail!("ALP encoded ints have invalid DType {}", encoded.dtype(),),
322+
_ => {
323+
vortex_bail!("ALP encoded ints have invalid DType {}", encoded.dtype(),)
324+
}
323325
}
324326
}
325327

‎encodings/datetime-parts/src/array.rs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -323,10 +323,16 @@ impl DateTimePartsData {
323323
);
324324
}
325325
if !seconds.dtype().is_int() || seconds.dtype().is_nullable() {
326-
vortex_bail!(MismatchedTypes: "non-nullable integer", seconds.dtype());
326+
vortex_bail!(
327+
"expected type: non-nullable integer but instead got {}",
328+
seconds.dtype()
329+
);
327330
}
328331
if !subseconds.dtype().is_int() || subseconds.dtype().is_nullable() {
329-
vortex_bail!(MismatchedTypes: "non-nullable integer", subseconds.dtype());
332+
vortex_bail!(
333+
"expected type: non-nullable integer but instead got {}",
334+
subseconds.dtype()
335+
);
330336
}
331337

332338
if len != seconds.len() || len != subseconds.len() {

‎encodings/datetime-parts/src/canonical.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,19 +41,19 @@ pub fn decode_to_temporal(
4141
ctx: &mut ExecutionCtx,
4242
) -> VortexResult<TemporalArray> {
4343
let DType::Extension(ext) = dtype else {
44-
vortex_panic!(Compute: "expected dtype to be DType::Extension variant")
44+
vortex_panic!("expected dtype to be DType::Extension variant")
4545
};
4646

4747
let Some(options) = ext.metadata_opt::<Timestamp>() else {
48-
vortex_panic!(Compute: "must decode TemporalMetadata from extension metadata");
48+
vortex_panic!("must decode TemporalMetadata from extension metadata");
4949
};
5050

5151
let divisor = match options.unit {
5252
TimeUnit::Nanoseconds => 1_000_000_000,
5353
TimeUnit::Microseconds => 1_000_000,
5454
TimeUnit::Milliseconds => 1_000,
5555
TimeUnit::Seconds => 1,
56-
TimeUnit::Days => vortex_panic!(InvalidArgument: "cannot decode into TimeUnit::D"),
56+
TimeUnit::Days => vortex_panic!("cannot decode into TimeUnit::D"),
5757
};
5858

5959
let days = parts.days.as_::<Primitive>();

‎encodings/datetime-parts/src/ops.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ impl OperationsVTable<DateTimeParts> for DateTimeParts {
3535
};
3636

3737
let Some(options) = ext.metadata_opt::<Timestamp>() else {
38-
vortex_panic!(Compute: "must decode TemporalMetadata from extension metadata");
38+
vortex_panic!("must decode TemporalMetadata from extension metadata");
3939
};
4040

4141
if !state.is_valid(index, ctx)? {

‎encodings/decimal-byte-parts/src/decimal_byte_parts/assemble.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,9 @@ fn assemble_wide_decimal_from_arrays(
154154
validity,
155155
)
156156
.into_array(),
157-
_ => vortex_bail!("expected between one and {MAX_LOWER_PARTS} lower parts"),
157+
_ => {
158+
vortex_bail!("expected between one and {MAX_LOWER_PARTS} lower parts")
159+
}
158160
}
159161
}))
160162
}

‎encodings/fastlanes/src/bitpacking/array/bitpack_compress/blocked.rs‎

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ pub fn bitpack_encode_blocked(
9494
let num_blocks = array.len().div_ceil(FL_CHUNK_SIZE);
9595
vortex_ensure!(
9696
bit_widths.len() == num_blocks,
97-
InvalidArgument: "Expected {num_blocks} bit widths, got {}",
97+
"Expected {num_blocks} bit widths, got {}",
9898
bit_widths.len()
9999
);
100100
validate_bit_widths(bit_widths, array.ptype())?;
@@ -133,13 +133,13 @@ fn validate_bit_widths(bit_widths: &[u8], ptype: PType) -> VortexResult<()> {
133133
let bit_width = usize::from(bit_width);
134134
vortex_ensure!(
135135
bit_width <= max_bit_width,
136-
InvalidArgument: "Bit widths must be at most {max_bit_width} for {ptype}"
136+
"Bit widths must be at most {max_bit_width} for {ptype}"
137137
);
138138
has_packed_block |= bit_width < max_bit_width;
139139
}
140140
vortex_ensure!(
141141
bit_widths.is_empty() || has_packed_block,
142-
InvalidArgument: "Cannot pack: all blocks use the native bit width {max_bit_width}"
142+
"Cannot pack: all blocks use the native bit width {max_bit_width}"
143143
);
144144
Ok(())
145145
}
@@ -343,7 +343,6 @@ mod tests {
343343
use vortex_array::assert_arrays_eq;
344344
use vortex_array::validity::Validity;
345345
use vortex_buffer::Buffer;
346-
use vortex_error::VortexError;
347346
use vortex_error::vortex_bail;
348347
use vortex_error::vortex_err;
349348
use vortex_session::VortexSession;
@@ -542,33 +541,43 @@ mod tests {
542541
}
543542

544543
#[rstest]
545-
#[case::negative(PrimitiveArray::from_iter(-5i64..5))]
546-
#[case::float(PrimitiveArray::from_iter([1.0f32, 2.0]))]
547-
fn encode_blocked_rejects_invalid_values(#[case] array: PrimitiveArray) {
544+
#[case::negative(
545+
PrimitiveArray::from_iter(-5i64..5),
546+
"cannot bitpack array containing negative integers"
547+
)]
548+
#[case::float(PrimitiveArray::from_iter([1.0f32, 2.0]), "cannot bitpack f32 array")]
549+
fn encode_blocked_rejects_invalid_values(
550+
#[case] array: PrimitiveArray,
551+
#[case] expected: &str,
552+
) {
548553
let mut ctx = SESSION.create_execution_ctx();
549-
assert!(matches!(
550-
bitpack_blocked_to_best_bit_widths(&array, &mut ctx).unwrap_err(),
551-
VortexError::InvalidArgument(_, _)
552-
));
553-
assert!(matches!(
554-
BitPackedData::encode_blocked(&array.into_array(), &[1], &mut ctx).unwrap_err(),
555-
VortexError::InvalidArgument(_, _)
556-
));
554+
let err = bitpack_blocked_to_best_bit_widths(&array, &mut ctx)
555+
.unwrap_err()
556+
.to_string();
557+
assert!(err.contains(expected), "{err}");
558+
let err = BitPackedData::encode_blocked(&array.into_array(), &[1], &mut ctx)
559+
.unwrap_err()
560+
.to_string();
561+
assert!(err.contains(expected), "{err}");
557562
}
558563

559564
#[rstest]
560-
#[case::too_few(vec![3])]
561-
#[case::too_many(vec![3, 3, 3])]
562-
#[case::wider_than_type(vec![3, 33])]
563-
fn encode_blocked_rejects_invalid_bit_widths(#[case] bit_widths: Vec<u8>) {
565+
#[case::too_few(vec![3], "Expected 2 bit widths, got 1")]
566+
#[case::too_many(vec![3, 3, 3], "Expected 2 bit widths, got 3")]
567+
#[case::wider_than_type(vec![3, 33], "Bit widths must be at most 32 for u32")]
568+
fn encode_blocked_rejects_invalid_bit_widths(
569+
#[case] bit_widths: Vec<u8>,
570+
#[case] expected: &str,
571+
) {
564572
let array = PrimitiveArray::from_iter((0..2048u32).map(|i| i % 8));
565573
let err = BitPackedData::encode_blocked(
566574
&array.into_array(),
567575
&bit_widths,
568576
&mut SESSION.create_execution_ctx(),
569577
)
570-
.unwrap_err();
571-
assert!(matches!(err, VortexError::InvalidArgument(_, _)));
578+
.unwrap_err()
579+
.to_string();
580+
assert!(err.contains(expected), "{err}");
572581
}
573582

574583
#[test]

‎encodings/fastlanes/src/bitpacking/array/bitpack_compress/global.rs‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ pub fn bitpack_encode(
5858
if bit_width >= array.ptype().bit_width() as u8 {
5959
// Nothing we can do
6060
vortex_bail!(
61-
InvalidArgument: "Cannot pack - specified bit width {bit_width} >= {}",
61+
"Cannot pack - specified bit width {bit_width} >= {}",
6262
array.ptype().bit_width()
6363
)
6464
}
@@ -434,7 +434,6 @@ mod test {
434434
use vortex_array::builders::ArrayBuilder;
435435
use vortex_array::builders::PrimitiveBuilder;
436436
use vortex_buffer::Buffer;
437-
use vortex_error::VortexError;
438437
use vortex_error::vortex_err;
439438
use vortex_session::VortexSession;
440439

@@ -493,8 +492,12 @@ mod test {
493492
assert!(array.ptype().is_signed_int());
494493

495494
let err = BitPackedData::encode(&array.into_array(), 1024u32.ilog2() as u8, &mut ctx)
496-
.unwrap_err();
497-
assert!(matches!(err, VortexError::InvalidArgument(_, _)));
495+
.unwrap_err()
496+
.to_string();
497+
assert!(
498+
err.contains("cannot bitpack array containing negative integers"),
499+
"{err}"
500+
);
498501
}
499502

500503
#[test]

‎encodings/fastlanes/src/bitpacking/array/bitpack_compress/mod.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,15 @@ fn ensure_non_negative_integers(
3333
) -> VortexResult<()> {
3434
vortex_ensure!(
3535
array.ptype().is_int(),
36-
InvalidArgument: "cannot bitpack {} array",
36+
"cannot bitpack {} array",
3737
array.ptype()
3838
);
3939
if array.ptype().is_signed_int() {
4040
let has_negative_values = match_each_integer_ptype!(array.ptype(), |P| {
4141
array.statistics().compute_min::<P>(ctx).unwrap_or_default() < 0
4242
});
4343
if has_negative_values {
44-
vortex_bail!(InvalidArgument: "cannot bitpack array containing negative integers")
44+
vortex_bail!("cannot bitpack array containing negative integers")
4545
}
4646
}
4747
Ok(())

‎encodings/fastlanes/src/bitpacking/array/mod.rs‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,11 @@ impl BitPackedData {
330330
length: usize,
331331
offset: u16,
332332
) -> VortexResult<()> {
333-
vortex_ensure!(ptype.is_int(), MismatchedTypes: "integer", ptype);
333+
vortex_ensure!(
334+
ptype.is_int(),
335+
"expected type: integer but instead got {}",
336+
ptype
337+
);
334338

335339
if let Some(validity_len) = validity.maybe_len() {
336340
vortex_ensure_eq!(
@@ -437,10 +441,12 @@ impl BitPackedData {
437441
bit_width: u8,
438442
ctx: &mut ExecutionCtx,
439443
) -> VortexResult<BitPackedArray> {
440-
let parray: PrimitiveArray = array
441-
.clone()
442-
.try_downcast::<Primitive>()
443-
.map_err(|a| vortex_err!(InvalidArgument: "Bitpacking can only encode primitive arrays, got {}", a.encoding_id()))?;
444+
let parray: PrimitiveArray = array.clone().try_downcast::<Primitive>().map_err(|a| {
445+
vortex_err!(
446+
"Bitpacking can only encode primitive arrays, got {}",
447+
a.encoding_id()
448+
)
449+
})?;
444450
bitpack_encode(&parray, bit_width, None, ctx)
445451
}
446452

@@ -460,10 +466,12 @@ impl BitPackedData {
460466
bit_widths: &[u8],
461467
ctx: &mut ExecutionCtx,
462468
) -> VortexResult<BitPackedArray> {
463-
let parray: PrimitiveArray = array
464-
.clone()
465-
.try_downcast::<Primitive>()
466-
.map_err(|a| vortex_err!(InvalidArgument: "Bitpacking can only encode primitive arrays, got {}", a.encoding_id()))?;
469+
let parray: PrimitiveArray = array.clone().try_downcast::<Primitive>().map_err(|a| {
470+
vortex_err!(
471+
"Bitpacking can only encode primitive arrays, got {}",
472+
a.encoding_id()
473+
)
474+
})?;
467475
bitpack_encode_blocked(&parray, bit_widths, None, ctx)
468476
}
469477
}

0 commit comments

Comments
 (0)