Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion STYLE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
6 changes: 4 additions & 2 deletions encodings/alp/src/alp/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;

Expand Down Expand Up @@ -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(),)
}
}
}

Expand Down
10 changes: 8 additions & 2 deletions encodings/datetime-parts/src/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
6 changes: 3 additions & 3 deletions encodings/datetime-parts/src/canonical.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,19 +41,19 @@ pub fn decode_to_temporal(
ctx: &mut ExecutionCtx,
) -> VortexResult<TemporalArray> {
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::<Timestamp>() else {
vortex_panic!(Compute: "must decode TemporalMetadata from extension metadata");
vortex_panic!("must decode TemporalMetadata from extension metadata");
};

let divisor = match options.unit {
TimeUnit::Nanoseconds => 1_000_000_000,
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_::<Primitive>();
Expand Down
2 changes: 1 addition & 1 deletion encodings/datetime-parts/src/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ impl OperationsVTable<DateTimeParts> for DateTimeParts {
};

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

if !state.is_valid(index, ctx)? {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}
}))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())?;
Expand Down Expand Up @@ -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(())
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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<u8>) {
#[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<u8>,
#[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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
)
}
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,15 @@ 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() {
let has_negative_values = match_each_integer_ptype!(array.ptype(), |P| {
array.statistics().compute_min::<P>(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(())
Expand Down
26 changes: 17 additions & 9 deletions encodings/fastlanes/src/bitpacking/array/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down Expand Up @@ -437,10 +441,12 @@ impl BitPackedData {
bit_width: u8,
ctx: &mut ExecutionCtx,
) -> VortexResult<BitPackedArray> {
let parray: PrimitiveArray = array
.clone()
.try_downcast::<Primitive>()
.map_err(|a| vortex_err!(InvalidArgument: "Bitpacking can only encode primitive arrays, got {}", a.encoding_id()))?;
let parray: PrimitiveArray = array.clone().try_downcast::<Primitive>().map_err(|a| {
vortex_err!(
"Bitpacking can only encode primitive arrays, got {}",
a.encoding_id()
)
})?;
bitpack_encode(&parray, bit_width, None, ctx)
}

Expand All @@ -460,10 +466,12 @@ impl BitPackedData {
bit_widths: &[u8],
ctx: &mut ExecutionCtx,
) -> VortexResult<BitPackedArray> {
let parray: PrimitiveArray = array
.clone()
.try_downcast::<Primitive>()
.map_err(|a| vortex_err!(InvalidArgument: "Bitpacking can only encode primitive arrays, got {}", a.encoding_id()))?;
let parray: PrimitiveArray = array.clone().try_downcast::<Primitive>().map_err(|a| {
vortex_err!(
"Bitpacking can only encode primitive arrays, got {}",
a.encoding_id()
)
})?;
bitpack_encode_blocked(&parray, bit_widths, None, ctx)
}
}
Expand Down
4 changes: 3 additions & 1 deletion encodings/fastlanes/src/for_/array/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}
}

Expand Down
Loading
Loading