Skip to content

Introduce VortexError::Other and simplify Display implementation #6410

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
AdamGS wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Introduce VortexError::Other and simplify Display implementation #6410

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
53 changes: 52 additions & 1 deletion Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions encodings/datetime-parts/src/canonical.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ pub fn decode_to_temporal(
ctx: &mut ExecutionCtx,
) -> VortexResult<TemporalArray> {
let DType::Extension(ext) = array.dtype().clone() else {
vortex_panic!(ComputeError: "expected dtype to be DType::Extension variant")
vortex_panic!(Compute: "expected dtype to be DType::Extension variant")
};

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

let divisor = match options.unit {
Expand Down
2 changes: 1 addition & 1 deletion encodings/datetime-parts/src/ops.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ impl OperationsVTable<DateTimePartsVTable> for DateTimePartsVTable {
};

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

if !array.is_valid(index)? {
Expand Down
4 changes: 2 additions & 2 deletions encodings/fastlanes/src/bitpacking/array/bitpack_compress.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ pub fn bitpack_encode(
array.statistics().compute_min::<P>().unwrap_or_default() < 0
});
if has_negative_values {
vortex_bail!("cannot bitpack_encode array containing negative integers")
vortex_bail!(InvalidArgument: "cannot bitpack_encode array containing negative integers")
}
}

Expand All @@ -59,7 +59,7 @@ pub fn bitpack_encode(
if bit_width >= array.ptype().bit_width() as u8 {
// Nothing we can do
vortex_bail!(
"Cannot pack - specified bit width {bit_width} >= {}",
InvalidArgument: "Cannot pack - specified bit width {bit_width} >= {}",
array.ptype().bit_width()
)
}
Expand Down
2 changes: 1 addition & 1 deletion encodings/fastlanes/src/bitpacking/array/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ impl BitPackedArray {
if let Some(parray) = array.as_opt::<PrimitiveVTable>() {
bitpack_encode(parray, bit_width, None)
} else {
vortex_bail!("Bitpacking can only encode primitive arrays");
vortex_bail!(InvalidArgument: "Bitpacking can only encode primitive arrays");
}
}

Expand Down
2 changes: 1 addition & 1 deletion vortex-array/src/arrays/decimal/array.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ impl DecimalArray {
let expected_len = values_type.byte_width() * validity_len;
vortex_ensure!(
buffer.len() == expected_len,
"expected buffer of size {} bytes, was {} bytes",
InvalidArgument: "expected buffer of size {} bytes, was {} bytes",
expected_len,
buffer.len(),
);
Expand Down
6 changes: 3 additions & 3 deletions vortex-array/src/arrays/fixed_size_list/array.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ impl FixedSizeListArray {
if let Some(validity_len) = validity.maybe_len() {
vortex_ensure!(
len == validity_len,
"validity with size {validity_len} does not match fixed-size list array size {len}",
InvalidArgument: "validity with size {validity_len} does not match fixed-size list array size {len}",
);
}

Expand All @@ -195,14 +195,14 @@ impl FixedSizeListArray {
if list_size == 0 {
vortex_ensure!(
elements.is_empty(),
"a degenerate (`list_size == 0`) `FixedSizeList` should have no underlying elements"
InvalidArgument: "a degenerate (`list_size == 0`) `FixedSizeList` should have no underlying elements"
);
return Ok(());
}

vortex_ensure!(
len * list_size as usize == elements.len(),
"the `elements` array has the incorrect number of elements to construct a \
InvalidArgument: "the `elements` array has the incorrect number of elements to construct a \
`FixedSizeList[{list_size}] array of length {len}",
);

Expand Down
16 changes: 8 additions & 8 deletions vortex-array/src/arrays/list/array.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,13 +163,13 @@ impl ListArray {
// Offsets must have at least one element
vortex_ensure!(
!offsets.is_empty(),
"Offsets must have at least one element, [0] for an empty list"
InvalidArgument: "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(),
"offsets have invalid type {}",
InvalidArgument: "offsets have invalid type {}",
offsets.dtype()
);

Expand All @@ -178,9 +178,9 @@ impl ListArray {

// Offsets must be sorted (but not strictly sorted, zero-length lists are allowed)
if let Some(is_sorted) = offsets.statistics().compute_is_sorted() {
vortex_ensure!(is_sorted, "offsets must be sorted");
vortex_ensure!(is_sorted, InvalidArgument: "offsets must be sorted");
} else {
vortex_bail!("offsets must report is_sorted statistic");
vortex_bail!(InvalidArgument: "offsets must report is_sorted statistic");
}

// Validate that offsets min is non-negative, and max does not exceed the length of
Expand All @@ -202,7 +202,7 @@ impl ListArray {

vortex_ensure!(
min >= 0,
"offsets minimum {min} outside valid range [0, {max}]"
InvalidArgument: "offsets minimum {min} outside valid range [0, {max}]"
);

vortex_ensure!(
Expand All @@ -211,15 +211,15 @@ impl ListArray {
<P as NativePType>::PTYPE,
elements.len()
)),
"Max offset {max} is beyond the length of the elements array {}",
InvalidArgument: "Max offset {max} is beyond the length of the elements array {}",
elements.len()
);
}
})
} else {
// TODO(aduffy): fallback to slower validation pathway?
vortex_bail!(
"offsets array with encoding {} must support min_max compute function",
InvalidArgument: "offsets array with encoding {} must support min_max compute function",
offsets.encoding_id()
);
};
Expand All @@ -228,7 +228,7 @@ impl ListArray {
if let Some(validity_len) = validity.maybe_len() {
vortex_ensure!(
validity_len == offsets.len() - 1,
"validity with size {validity_len} does not match array size {}",
InvalidArgument: "validity with size {validity_len} does not match array size {}",
offsets.len() - 1
);
}
Expand Down
1 change: 1 addition & 0 deletions vortex-array/src/arrays/primitive/array/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ impl PrimitiveArray {
&& buffer.len() != len
{
return Err(vortex_err!(
InvalidArgument:
"Buffer and validity length mismatch: buffer={}, validity={}",
buffer.len(),
len
Expand Down
6 changes: 3 additions & 3 deletions vortex-array/src/arrays/primitive/compute/cast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ fn cast<F: NativePType, T: NativePType>(array: &[F], mask: Mask) -> VortexResult
let mut buffer = BufferMut::with_capacity(array.len());
for item in array {
let item = T::from(*item).ok_or_else(
|| vortex_err!(ComputeError: "Failed to cast {} to {:?}", item, T::PTYPE),
|| vortex_err!(Compute: "Failed to cast {} to {:?}", item, T::PTYPE),
)?;
// SAFETY: we've pre-allocated the required capacity
unsafe { buffer.push_unchecked(item) }
Expand All @@ -80,7 +80,7 @@ fn cast<F: NativePType, T: NativePType>(array: &[F], mask: Mask) -> VortexResult
for (item, valid) in array.iter().zip(b.iter()) {
if valid {
let item = T::from(*item).ok_or_else(
|| vortex_err!(ComputeError: "Failed to cast {} to {:?}", item, T::PTYPE),
|| vortex_err!(Compute: "Failed to cast {} to {:?}", item, T::PTYPE),
)?;
// SAFETY: we've pre-allocated the required capacity
unsafe { buffer.push_unchecked(item) }
Expand Down Expand Up @@ -181,7 +181,7 @@ mod test {
fn cast_i32_u32() {
let arr = buffer![-1i32].into_array();
let error = cast(&arr, PType::U32.into()).err().unwrap();
let VortexError::ComputeError(s, _) = error else {
let VortexError::Compute(s, _) = error else {
unreachable!()
};
assert_eq!(s.to_string(), "Failed to cast -1 to U32");
Expand Down
8 changes: 4 additions & 4 deletions vortex-array/src/arrays/struct_/array.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ impl StructArray {
// Check field count matches
if fields.len() != dtype.names().len() {
vortex_bail!(
"Got {} fields but dtype has {} names",
InvalidArgument: "Got {} fields but dtype has {} names",
fields.len(),
dtype.names().len()
);
Expand All @@ -311,7 +311,7 @@ impl StructArray {
for (i, (field, struct_dt)) in fields.iter().zip(dtype.fields()).enumerate() {
if field.len() != length {
vortex_bail!(
"Field {} has length {} but expected {}",
InvalidArgument: "Field {} has length {} but expected {}",
i,
field.len(),
length
Expand All @@ -320,7 +320,7 @@ impl StructArray {

if field.dtype() != &struct_dt {
vortex_bail!(
"Field {} has dtype {} but expected {}",
InvalidArgument: "Field {} has dtype {} but expected {}",
i,
field.dtype(),
struct_dt
Expand All @@ -333,7 +333,7 @@ impl StructArray {
&& validity_len != length
{
vortex_bail!(
"Validity has length {} but expected {}",
InvalidArgument: "Validity has length {} but expected {}",
validity_len,
length
);
Expand Down
12 changes: 7 additions & 5 deletions vortex-array/src/arrays/varbin/array.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,20 +189,20 @@ impl VarBinArray {
// Check nullability matches
vortex_ensure!(
dtype.is_nullable() != (validity == &Validity::NonNullable),
"incorrect validity {:?} for dtype {}",
InvalidArgument: "incorrect validity {:?} for dtype {}",
validity,
dtype
);

// Check offsets has at least one element
vortex_ensure!(
!offsets.is_empty(),
"Offsets must have at least one element"
InvalidArgument: "Offsets must have at least one element"
);

// Check offsets are sorted
if let Some(is_sorted) = offsets.statistics().compute_is_sorted() {
vortex_ensure!(is_sorted, "offsets must be sorted");
vortex_ensure!(is_sorted, InvalidArgument: "offsets must be sorted");
}

// Skip host-only validation when offsets/bytes are not host-resident.
Expand All @@ -211,10 +211,12 @@ impl VarBinArray {
.scalar_at(offsets.len() - 1)?
.as_primitive()
.as_::<usize>()
.ok_or_else(|| vortex_err!("Last offset must be convertible to usize"))?;
.ok_or_else(
|| vortex_err!(InvalidArgument: "Last offset must be convertible to usize"),
)?;
vortex_ensure!(
last_offset <= bytes.len(),
"Last offset {} exceeds bytes length {}",
InvalidArgument: "Last offset {} exceeds bytes length {}",
last_offset,
bytes.len()
);
Expand Down
Loading
Loading