Skip to content

Commit c7b0ef9

Browse files
authored
Removes vortex_unwrap completely in favor of vortex_expect (#5805)
This change removes all the instances of `vortex_unwrap` and replaces them with `vortex_expect` essentially compelling the user to provide a justification of failure. Signed-off-by: Pratham Agarwal <[email protected]>
1 parent adf2182 commit c7b0ef9

File tree

72 files changed

+451
-345
lines changed

Some content is hidden

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

72 files changed

+451
-345
lines changed

STYLE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@
5454
- `vortex_panic!` for handling invariant violations
5555
- Add context to errors using `.with_context()`
5656
- Include backtraces for better debugging
57-
- Use `VortexExpect` and `VortexUnwrap` traits when unwrapping is appropriate
57+
- Use `VortexExpect` trait when unwrapping is appropriate with proper error context.
5858

5959
## Code Structure
6060

encodings/alp/src/alp_rd/mod.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ use vortex_dtype::DType;
3131
use vortex_dtype::NativePType;
3232
use vortex_dtype::match_each_integer_ptype;
3333
use vortex_error::VortexExpect;
34-
use vortex_error::VortexUnwrap;
3534
use vortex_error::vortex_panic;
3635
use vortex_utils::aliases::hash_map::HashMap;
3736

@@ -229,15 +228,15 @@ impl RDEncoder {
229228
// SAFETY: by construction, all values in left_parts can be packed to left_bit_width.
230229
let packed_left = unsafe {
231230
bitpack_encode_unchecked(primitive_left, left_bit_width as _)
232-
.vortex_unwrap()
231+
.vortex_expect("bitpack_encode_unchecked should succeed for left parts")
233232
.into_array()
234233
};
235234

236235
let primitive_right = PrimitiveArray::new(right_parts, Validity::NonNullable);
237236
// SAFETY: by construction, all values in right_parts are right_bit_width + leading zeros.
238237
let packed_right = unsafe {
239238
bitpack_encode_unchecked(primitive_right, self.right_bit_width as _)
240-
.vortex_unwrap()
239+
.vortex_expect("bitpack_encode_unchecked should succeed for right parts")
241240
.into_array()
242241
};
243242

@@ -252,7 +251,9 @@ impl RDEncoder {
252251
// SAFETY: We calculate bw such that it is wide enough to hold the largest position index.
253252
let packed_pos = unsafe {
254253
bitpack_encode_unchecked(exc_pos_array, bw)
255-
.vortex_unwrap()
254+
.vortex_expect(
255+
"bitpack_encode_unchecked should succeed for exception positions",
256+
)
256257
.into_array()
257258
};
258259

encodings/fsst/src/compress.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ use vortex_buffer::Buffer;
1313
use vortex_buffer::BufferMut;
1414
use vortex_dtype::DType;
1515
use vortex_error::VortexExpect;
16-
use vortex_error::VortexUnwrap;
1716

1817
use crate::FSSTArray;
1918

@@ -74,7 +73,11 @@ where
7473
uncompressed_lengths.push(0);
7574
}
7675
Some(s) => {
77-
uncompressed_lengths.push(s.len().try_into().vortex_unwrap());
76+
uncompressed_lengths.push(
77+
s.len()
78+
.try_into()
79+
.vortex_expect("string length must fit in i32"),
80+
);
7881

7982
// SAFETY: buffer is large enough
8083
unsafe { compressor.compress_into(s, &mut buffer) };

encodings/fsst/src/test_utils.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use vortex_array::arrays::VarBinArray;
1414
use vortex_dtype::DType;
1515
use vortex_dtype::NativePType;
1616
use vortex_dtype::Nullability;
17-
use vortex_error::VortexUnwrap;
17+
use vortex_error::VortexExpect;
1818

1919
use crate::fsst_compress;
2020
use crate::fsst_train_compressor;
@@ -56,5 +56,6 @@ pub fn gen_dict_fsst_test_data<T: NativePType>(
5656
let codes = (0..len)
5757
.map(|_| T::from(rng.random_range(0..unique_values)).unwrap())
5858
.collect::<PrimitiveArray>();
59-
DictArray::try_new(codes.into_array(), values).vortex_unwrap()
59+
DictArray::try_new(codes.into_array(), values)
60+
.vortex_expect("DictArray::try_new should succeed for test data")
6061
}

encodings/pco/src/array.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ use vortex_dtype::half;
5757
use vortex_error::VortexError;
5858
use vortex_error::VortexExpect;
5959
use vortex_error::VortexResult;
60-
use vortex_error::VortexUnwrap;
6160
use vortex_error::vortex_bail;
6261
use vortex_error::vortex_ensure;
6362
use vortex_error::vortex_err;
@@ -377,7 +376,7 @@ impl PcoArray {
377376
// may exceed the bounds of the slice, so we need to slice later.
378377
let (fd, _) = FileDecompressor::new(self.metadata.header.as_slice())
379378
.map_err(vortex_err_from_pco)
380-
.vortex_unwrap();
379+
.vortex_expect("FileDecompressor::new should succeed with valid header");
381380
let mut decompressed_values = BufferMut::<T>::with_capacity(slice_n_values);
382381
let mut page_idx = 0;
383382
let mut page_value_start = 0;
@@ -406,18 +405,20 @@ impl PcoArray {
406405
let (new_cd, _) = fd
407406
.chunk_decompressor(chunk_meta_bytes)
408407
.map_err(vortex_err_from_pco)
409-
.vortex_unwrap();
408+
.vortex_expect(
409+
"chunk_decompressor should succeed with valid chunk metadata",
410+
);
410411
cd = Some(new_cd);
411412
}
412413
let mut pd = cd
413414
.as_mut()
414415
.unwrap()
415416
.page_decompressor(page, page_n_values)
416417
.map_err(vortex_err_from_pco)
417-
.vortex_unwrap();
418+
.vortex_expect("page_decompressor should succeed with valid page data");
418419
pd.decompress(&mut decompressed_values[old_len..new_len])
419420
.map_err(vortex_err_from_pco)
420-
.vortex_unwrap();
421+
.vortex_expect("decompress should succeed with valid compressed data");
421422
} else {
422423
n_skipped_values += page_n_values;
423424
}

encodings/runend/src/compute/filter.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ use vortex_dtype::NativePType;
2222
use vortex_dtype::match_each_unsigned_integer_ptype;
2323
use vortex_error::VortexExpect;
2424
use vortex_error::VortexResult;
25-
use vortex_error::VortexUnwrap;
2625
use vortex_mask::Mask;
2726

2827
use crate::RunEndArray;
@@ -118,9 +117,9 @@ fn filter_run_end_primitive<R: NativePType + AddAssign + From<bool> + AsPrimitiv
118117
let end = min(run_ends[i].as_() - offset, length);
119118

120119
// Safety: predicate must be the same length as the array the ends have been taken from
121-
for pred in
122-
(start..end).map(|i| unsafe { mask.value_unchecked(i.try_into().vortex_unwrap()) })
123-
{
120+
for pred in (start..end).map(|i| unsafe {
121+
mask.value_unchecked(i.try_into().vortex_expect("index must fit in usize"))
122+
}) {
124123
count += <R as From<bool>>::from(pred);
125124
keep |= pred
126125
}

encodings/sparse/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -533,7 +533,7 @@ mod test {
533533
use vortex_dtype::DType;
534534
use vortex_dtype::Nullability;
535535
use vortex_dtype::PType;
536-
use vortex_error::VortexUnwrap;
536+
use vortex_error::VortexExpect;
537537
use vortex_scalar::PrimitiveScalar;
538538
use vortex_scalar::Scalar;
539539

@@ -684,7 +684,7 @@ mod test {
684684
.into_array(),
685685
None,
686686
)
687-
.vortex_unwrap();
687+
.vortex_expect("SparseArray::encode should succeed for test data");
688688
let canonical = sparse.to_primitive();
689689
assert_eq!(
690690
sparse.validity_mask(),

fuzz/fuzz_targets/file_io.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ use vortex_buffer::ByteBufferMut;
2121
use vortex_dtype::DType;
2222
use vortex_dtype::StructFields;
2323
use vortex_error::VortexExpect;
24-
use vortex_error::VortexUnwrap;
2524
use vortex_error::vortex_panic;
2625
use vortex_file::OpenOptionsSessionExt;
2726
use vortex_file::WriteOptionsSessionExt;
@@ -52,14 +51,15 @@ fuzz_target!(|fuzz: FuzzFileAction| -> Corpus {
5251
.clone()
5352
.unwrap_or_else(|| lit(true))
5453
.evaluate(&array_data)
55-
.vortex_unwrap();
54+
.vortex_expect("filter expression evaluation should succeed in fuzz test");
5655
let mask = bool_mask.to_bool().to_mask_fill_null_false();
57-
let filtered = filter(&array_data, &mask).vortex_unwrap();
56+
let filtered = filter(&array_data, &mask)
57+
.vortex_expect("filter operation should succeed in fuzz test");
5858
projection_expr
5959
.clone()
6060
.unwrap_or_else(root)
6161
.evaluate(&filtered)
62-
.vortex_unwrap()
62+
.vortex_expect("projection expression evaluation should succeed in fuzz test")
6363
};
6464

6565
let write_options = match compressor_strategy {
@@ -76,20 +76,20 @@ fuzz_target!(|fuzz: FuzzFileAction| -> Corpus {
7676
let _footer = write_options
7777
.blocking(&*RUNTIME)
7878
.write(&mut full_buff, array_data.to_array_iterator())
79-
.vortex_unwrap();
79+
.vortex_expect("file write should succeed in fuzz test");
8080

8181
let mut output = SESSION
8282
.open_options()
8383
.open_buffer(full_buff)
84-
.vortex_unwrap()
84+
.vortex_expect("open_buffer should succeed in fuzz test")
8585
.scan()
86-
.vortex_unwrap()
86+
.vortex_expect("scan should succeed in fuzz test")
8787
.with_projection(projection_expr.unwrap_or_else(root))
8888
.with_some_filter(filter_expr)
8989
.into_array_iter(&*RUNTIME)
90-
.vortex_unwrap()
90+
.vortex_expect("into_array_iter should succeed in fuzz test")
9191
.try_collect::<_, Vec<_>, _>()
92-
.vortex_unwrap();
92+
.vortex_expect("collect should succeed in fuzz test");
9393

9494
let output_array = match output.len() {
9595
0 => Canonical::empty(expected_array.dtype()).into_array(),
@@ -113,7 +113,7 @@ fuzz_target!(|fuzz: FuzzFileAction| -> Corpus {
113113
);
114114

115115
let bool_result = compare(&expected_array, &output_array, Operator::Eq)
116-
.vortex_unwrap()
116+
.vortex_expect("compare operation should succeed in fuzz test")
117117
.to_bool();
118118
let true_count = bool_result.bit_buffer().true_count();
119119
if true_count != expected_array.len() && (bool_result.all_valid() || expected_array.all_valid())

fuzz/src/array/fill_null.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ use vortex_dtype::match_each_decimal_value_type;
2121
use vortex_dtype::match_each_native_ptype;
2222
use vortex_error::VortexExpect;
2323
use vortex_error::VortexResult;
24-
use vortex_error::VortexUnwrap;
2524
use vortex_scalar::Scalar;
2625

2726
/// Apply fill_null on the canonical form of the array to get a consistent baseline.
@@ -87,7 +86,8 @@ fn fill_primitive_array(
8786
result_nullability: Nullability,
8887
) -> ArrayRef {
8988
match_each_native_ptype!(array.ptype(), |T| {
90-
let fill_val = T::try_from(fill_value).vortex_unwrap();
89+
let fill_val = T::try_from(fill_value)
90+
.vortex_expect("fill value conversion should succeed in fuzz test");
9191

9292
match array.validity() {
9393
Validity::NonNullable | Validity::AllValid => PrimitiveArray::from_byte_buffer(
@@ -129,7 +129,8 @@ fn fill_decimal_array(
129129
let decimal_scalar = fill_value.as_decimal();
130130

131131
match_each_decimal_value_type!(array.values_type(), |D| {
132-
let fill_val = D::try_from(decimal_scalar).vortex_unwrap();
132+
let fill_val = D::try_from(decimal_scalar)
133+
.vortex_expect("decimal fill value conversion should succeed in fuzz test");
133134

134135
match array.validity() {
135136
Validity::NonNullable | Validity::AllValid => DecimalArray::new(
@@ -156,7 +157,7 @@ fn fill_decimal_array(
156157
}
157158

158159
DecimalArray::try_new(new_data.freeze(), decimal_dtype, result_nullability.into())
159-
.vortex_unwrap()
160+
.vortex_expect("DecimalArray creation should succeed in fuzz test")
160161
.into_array()
161162
}
162163
}

fuzz/src/array/mask.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ use vortex_array::arrays::VarBinViewArray;
1717
use vortex_array::vtable::ValidityHelper;
1818
use vortex_dtype::ExtDType;
1919
use vortex_dtype::match_each_decimal_value_type;
20+
use vortex_error::VortexExpect;
2021
use vortex_error::VortexResult;
21-
use vortex_error::VortexUnwrap;
2222
use vortex_mask::Mask;
2323

2424
/// Apply mask on the canonical form of the array to get a consistent baseline.
@@ -94,13 +94,13 @@ pub fn mask_canonical_array(canonical: Canonical, mask: &Mask) -> VortexResult<A
9494
array.len(),
9595
new_validity,
9696
)
97-
.vortex_unwrap()
97+
.vortex_expect("StructArray creation should succeed in fuzz test")
9898
.into_array()
9999
}
100100
Canonical::Extension(array) => {
101101
// Recursively mask the storage array
102-
let masked_storage =
103-
mask_canonical_array(array.storage().to_canonical(), mask).vortex_unwrap();
102+
let masked_storage = mask_canonical_array(array.storage().to_canonical(), mask)
103+
.vortex_expect("mask_canonical_array should succeed in fuzz test");
104104

105105
if masked_storage.dtype().nullability()
106106
== array.ext_dtype().storage_dtype().nullability()

0 commit comments

Comments
 (0)