Skip to content

Commit 6df2004

Browse files
chore: fallible canonical (#6006)
Adding `ScalarFnArray` meant now canonical can fail # break `to_canonical` now return a `VortexResult<Canonical>` `append_to_builder` now return `VortexResult<()>` --------- Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
1 parent 6fe029b commit 6df2004

File tree

113 files changed

+674
-438
lines changed

Some content is hidden

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

113 files changed

+674
-438
lines changed

benchmarks/compress-bench/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ pub fn chunked_to_vec_record_batch(
2323
.map(|array| {
2424
// TODO(connor)[ListView]: The rust Parquet implementation does not support writing
2525
// `ListView` to Parquet files yet.
26-
let converted_array = recursive_list_from_list_view(array.clone());
26+
let converted_array = recursive_list_from_list_view(array.clone())?;
2727
Ok(RecordBatch::try_from(converted_array.as_ref())?)
2828
})
2929
.collect::<anyhow::Result<Vec<_>>>()?;

encodings/alp/src/alp/array.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -460,8 +460,8 @@ impl BaseArrayVTable<ALPVTable> for ALPVTable {
460460
}
461461

462462
impl CanonicalVTable<ALPVTable> for ALPVTable {
463-
fn canonicalize(array: &ALPArray) -> Canonical {
464-
Canonical::Primitive(decompress_into_array(array.clone()))
463+
fn canonicalize(array: &ALPArray) -> VortexResult<Canonical> {
464+
Ok(Canonical::Primitive(decompress_into_array(array.clone())))
465465
}
466466
}
467467

encodings/alp/src/alp/compute/cast.rs

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -60,56 +60,58 @@ mod tests {
6060
use vortex_dtype::DType;
6161
use vortex_dtype::Nullability;
6262
use vortex_dtype::PType;
63+
use vortex_error::VortexExpect;
64+
use vortex_error::VortexResult;
6365

6466
use crate::ALPVTable;
6567

6668
#[test]
67-
fn test_cast_alp_f32_to_f64() {
69+
fn test_cast_alp_f32_to_f64() -> VortexResult<()> {
6870
let values = buffer![1.5f32, 2.5, 3.5, 4.5].into_array();
6971
let alp = ALPVTable
7072
.as_vtable()
71-
.encode(&values.to_canonical(), None)
72-
.unwrap()
73-
.unwrap();
73+
.encode(&values.to_canonical()?, None)?
74+
.vortex_expect("must encode");
7475

7576
let casted = cast(
7677
alp.as_ref(),
7778
&DType::Primitive(PType::F64, Nullability::NonNullable),
78-
)
79-
.unwrap();
79+
)?;
8080
assert_eq!(
8181
casted.dtype(),
8282
&DType::Primitive(PType::F64, Nullability::NonNullable)
8383
);
8484

85-
let decoded = casted.to_canonical().into_primitive();
85+
let decoded = casted.to_canonical()?.into_primitive();
8686
let values = decoded.as_slice::<f64>();
8787
assert_eq!(values.len(), 4);
8888
assert!((values[0] - 1.5).abs() < f64::EPSILON);
8989
assert!((values[1] - 2.5).abs() < f64::EPSILON);
90+
91+
Ok(())
9092
}
9193

9294
#[test]
93-
fn test_cast_alp_to_int() {
95+
fn test_cast_alp_to_int() -> VortexResult<()> {
9496
let values = buffer![1.0f32, 2.0, 3.0, 4.0].into_array();
9597
let alp = ALPVTable
9698
.as_vtable()
97-
.encode(&values.to_canonical(), None)
98-
.unwrap()
99-
.unwrap();
99+
.encode(&values.to_canonical()?, None)?
100+
.vortex_expect("must encode");
100101

101102
let casted = cast(
102103
alp.as_ref(),
103104
&DType::Primitive(PType::I32, Nullability::NonNullable),
104-
)
105-
.unwrap();
105+
)?;
106106
assert_eq!(
107107
casted.dtype(),
108108
&DType::Primitive(PType::I32, Nullability::NonNullable)
109109
);
110110

111-
let decoded = casted.to_canonical().into_primitive();
111+
let decoded = casted.to_canonical()?.into_primitive();
112112
assert_arrays_eq!(decoded, PrimitiveArray::from_iter([1i32, 2, 3, 4]));
113+
114+
Ok(())
113115
}
114116

115117
#[rstest]
@@ -118,12 +120,13 @@ mod tests {
118120
#[case(PrimitiveArray::from_option_iter([Some(1.1f32), None, Some(2.2), Some(3.3), None]).into_array())]
119121
#[case(buffer![42.42f64].into_array())]
120122
#[case(buffer![0.0f32, -1.5, 2.5, -3.5, 4.5].into_array())]
121-
fn test_cast_alp_conformance(#[case] array: vortex_array::ArrayRef) {
123+
fn test_cast_alp_conformance(#[case] array: vortex_array::ArrayRef) -> VortexResult<()> {
122124
let alp = ALPVTable
123125
.as_vtable()
124-
.encode(&array.to_canonical(), None)
125-
.unwrap()
126-
.unwrap();
126+
.encode(&array.to_canonical()?, None)?
127+
.vortex_expect("cannot fail");
127128
test_cast_conformance(alp.as_ref());
129+
130+
Ok(())
128131
}
129132
}

encodings/alp/src/alp/compute/filter.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ mod test {
5959
fn test_filter_alp_conformance(#[case] array: ArrayRef) {
6060
let alp = ALPVTable
6161
.as_vtable()
62-
.encode(&array.to_canonical(), None)
62+
.encode(&array.to_canonical().unwrap(), None)
6363
.unwrap()
6464
.unwrap();
6565
test_filter_conformance(alp.as_ref());

encodings/alp/src/alp/compute/mask.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ mod test {
5757
fn test_mask_alp_conformance(#[case] array: vortex_array::ArrayRef) {
5858
let alp = ALPVTable
5959
.as_vtable()
60-
.encode(&array.to_canonical(), None)
60+
.encode(&array.to_canonical().unwrap(), None)
6161
.unwrap()
6262
.unwrap();
6363
test_mask_conformance(alp.as_ref());

encodings/alp/src/alp/compute/take.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ mod test {
5454
fn test_take_alp_conformance(#[case] array: vortex_array::ArrayRef) {
5555
let alp = ALPVTable
5656
.as_vtable()
57-
.encode(&array.to_canonical(), None)
57+
.encode(&array.to_canonical().unwrap(), None)
5858
.unwrap()
5959
.unwrap();
6060
test_take_conformance(alp.as_ref());

encodings/alp/src/alp_rd/array.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,7 @@ impl BaseArrayVTable<ALPRDVTable> for ALPRDVTable {
430430
}
431431

432432
impl CanonicalVTable<ALPRDVTable> for ALPRDVTable {
433-
fn canonicalize(array: &ALPRDArray) -> Canonical {
433+
fn canonicalize(array: &ALPRDArray) -> VortexResult<Canonical> {
434434
let left_parts = array.left_parts().to_primitive();
435435
let right_parts = array.right_parts().to_primitive();
436436

@@ -461,7 +461,7 @@ impl CanonicalVTable<ALPRDVTable> for ALPRDVTable {
461461
)
462462
};
463463

464-
Canonical::Primitive(decoded_array)
464+
Ok(Canonical::Primitive(decoded_array))
465465
}
466466
}
467467

encodings/bytebool/src/array.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,10 +215,13 @@ impl BaseArrayVTable<ByteBoolVTable> for ByteBoolVTable {
215215
}
216216

217217
impl CanonicalVTable<ByteBoolVTable> for ByteBoolVTable {
218-
fn canonicalize(array: &ByteBoolArray) -> Canonical {
218+
fn canonicalize(array: &ByteBoolArray) -> VortexResult<Canonical> {
219219
let boolean_buffer = BitBuffer::from(array.as_slice());
220220
let validity = array.validity().clone();
221-
Canonical::Bool(BoolArray::from_bit_buffer(boolean_buffer, validity))
221+
Ok(Canonical::Bool(BoolArray::from_bit_buffer(
222+
boolean_buffer,
223+
validity,
224+
)))
222225
}
223226
}
224227

encodings/datetime-parts/src/canonical.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,15 @@ use vortex_dtype::datetime::TemporalMetadata;
1717
use vortex_dtype::datetime::TimeUnit;
1818
use vortex_dtype::match_each_integer_ptype;
1919
use vortex_error::VortexExpect as _;
20+
use vortex_error::VortexResult;
2021
use vortex_error::vortex_panic;
2122

2223
use crate::DateTimePartsArray;
2324
use crate::DateTimePartsVTable;
2425

2526
impl CanonicalVTable<DateTimePartsVTable> for DateTimePartsVTable {
26-
fn canonicalize(array: &DateTimePartsArray) -> Canonical {
27-
Canonical::Extension(decode_to_temporal(array).into())
27+
fn canonicalize(array: &DateTimePartsArray) -> VortexResult<Canonical> {
28+
Ok(Canonical::Extension(decode_to_temporal(array).into()))
2829
}
2930
}
3031

encodings/decimal-byte-parts/src/decimal_byte_parts/mod.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -235,13 +235,13 @@ impl BaseArrayVTable<DecimalBytePartsVTable> for DecimalBytePartsVTable {
235235
}
236236

237237
impl CanonicalVTable<DecimalBytePartsVTable> for DecimalBytePartsVTable {
238-
fn canonicalize(array: &DecimalBytePartsArray) -> Canonical {
238+
fn canonicalize(array: &DecimalBytePartsArray) -> VortexResult<Canonical> {
239239
// TODO(joe): support parts len != 1
240240
let prim = array.msp.to_primitive();
241241
// Depending on the decimal type and the min/max of the primitive array we can choose
242242
// the correct buffer size
243243

244-
match_each_signed_integer_ptype!(prim.ptype(), |P| {
244+
Ok(match_each_signed_integer_ptype!(prim.ptype(), |P| {
245245
// SAFETY: The primitive array's buffer is already validated with correct type.
246246
// The decimal dtype matches the array's dtype, and validity is preserved.
247247
Canonical::Decimal(unsafe {
@@ -251,7 +251,7 @@ impl CanonicalVTable<DecimalBytePartsVTable> for DecimalBytePartsVTable {
251251
prim.validity().clone(),
252252
)
253253
})
254-
})
254+
}))
255255
}
256256
}
257257

0 commit comments

Comments
 (0)