diff --git a/encodings/sequence/public-api.lock b/encodings/sequence/public-api.lock index 6d4095649d0..5c1e127b7c6 100644 --- a/encodings/sequence/public-api.lock +++ b/encodings/sequence/public-api.lock @@ -12,11 +12,11 @@ pub fn vortex_sequence::SequenceArray::last(&self) -> vortex_array::scalar::type pub fn vortex_sequence::SequenceArray::multiplier(&self) -> vortex_array::scalar::typed_view::primitive::pvalue::PValue -pub fn vortex_sequence::SequenceArray::new(base: vortex_array::scalar::typed_view::primitive::pvalue::PValue, multiplier: vortex_array::scalar::typed_view::primitive::pvalue::PValue, ptype: vortex_array::dtype::ptype::PType, nullability: vortex_array::dtype::nullability::Nullability, length: usize) -> vortex_error::VortexResult - pub fn vortex_sequence::SequenceArray::ptype(&self) -> vortex_array::dtype::ptype::PType -pub fn vortex_sequence::SequenceArray::typed_new>(base: T, multiplier: T, nullability: vortex_array::dtype::nullability::Nullability, length: usize) -> vortex_error::VortexResult +pub fn vortex_sequence::SequenceArray::try_new(base: vortex_array::scalar::typed_view::primitive::pvalue::PValue, multiplier: vortex_array::scalar::typed_view::primitive::pvalue::PValue, ptype: vortex_array::dtype::ptype::PType, nullability: vortex_array::dtype::nullability::Nullability, length: usize) -> vortex_error::VortexResult + +pub fn vortex_sequence::SequenceArray::try_new_typed>(base: T, multiplier: T, nullability: vortex_array::dtype::nullability::Nullability, length: usize) -> vortex_error::VortexResult impl core::clone::Clone for vortex_sequence::SequenceArray @@ -104,7 +104,7 @@ impl vortex_array::vtable::VTable for vortex_sequence::SequenceVTable pub type vortex_sequence::SequenceVTable::Array = vortex_sequence::SequenceArray -pub type vortex_sequence::SequenceVTable::Metadata = vortex_array::metadata::ProstMetadata +pub type vortex_sequence::SequenceVTable::Metadata = vortex_sequence::array::SequenceMetadata pub type vortex_sequence::SequenceVTable::OperationsVTable = vortex_sequence::SequenceVTable @@ -124,7 +124,7 @@ pub fn vortex_sequence::SequenceVTable::child(_array: &vortex_sequence::Sequence pub fn vortex_sequence::SequenceVTable::child_name(_array: &vortex_sequence::SequenceArray, idx: usize) -> alloc::string::String -pub fn vortex_sequence::SequenceVTable::deserialize(bytes: &[u8], _dtype: &vortex_array::dtype::DType, _len: usize, _buffers: &[vortex_array::buffer::BufferHandle], _session: &vortex_session::VortexSession) -> vortex_error::VortexResult +pub fn vortex_sequence::SequenceVTable::deserialize(bytes: &[u8], dtype: &vortex_array::dtype::DType, _len: usize, _buffers: &[vortex_array::buffer::BufferHandle], _session: &vortex_session::VortexSession) -> vortex_error::VortexResult pub fn vortex_sequence::SequenceVTable::dtype(array: &vortex_sequence::SequenceArray) -> &vortex_array::dtype::DType diff --git a/encodings/sequence/src/array.rs b/encodings/sequence/src/array.rs index 207cff5c5a6..ae280882ed5 100644 --- a/encodings/sequence/src/array.rs +++ b/encodings/sequence/src/array.rs @@ -50,8 +50,14 @@ use crate::rules::RULES; vtable!(Sequence); -#[derive(Clone, prost::Message)] +#[derive(Debug, Clone, Copy)] pub struct SequenceMetadata { + base: PValue, + multiplier: PValue, +} + +#[derive(Clone, prost::Message)] +pub struct ProstSequenceMetadata { #[prost(message, tag = "1")] base: Option, #[prost(message, tag = "2")] @@ -78,13 +84,13 @@ pub struct SequenceArray { } impl SequenceArray { - pub fn typed_new>( + pub fn try_new_typed>( base: T, multiplier: T, nullability: Nullability, length: usize, ) -> VortexResult { - Self::new( + Self::try_new( base.into(), multiplier.into(), T::PTYPE, @@ -94,7 +100,7 @@ impl SequenceArray { } /// Constructs a sequence array using two integer values (with the same ptype). - pub fn new( + pub fn try_new( base: PValue, multiplier: PValue, ptype: PType, @@ -111,16 +117,23 @@ impl SequenceArray { )) })?; - Ok(Self::unchecked_new( - base, - multiplier, - ptype, - nullability, - length, - )) - } - - pub(crate) fn unchecked_new( + // SAFETY: we just validated that `ptype` is an integer and that the final + // element is representable via `try_last`. + Ok(unsafe { Self::new_unchecked(base, multiplier, ptype, nullability, length) }) + } + + /// Constructs a [`SequenceArray`] without validating that the `ptype` is an integer + /// type or that the final element is representable. + /// + /// # Safety + /// + /// The caller must ensure that: + /// - `ptype` is an integer type (i.e., `ptype.is_int()` returns `true`). + /// - `base + (length - 1) * multiplier` does not overflow the range of `ptype`. + /// + /// Violating the first invariant will cause a panic. Violating the second will + /// cause silent wraparound when materializing elements, producing incorrect values. + pub(crate) unsafe fn new_unchecked( base: PValue, multiplier: PValue, ptype: PType, @@ -226,7 +239,7 @@ impl SequenceArray { impl VTable for SequenceVTable { type Array = SequenceArray; - type Metadata = ProstMetadata; + type Metadata = SequenceMetadata; type OperationsVTable = Self; type ValidityVTable = Self; @@ -289,40 +302,37 @@ impl VTable for SequenceVTable { } fn metadata(array: &SequenceArray) -> VortexResult { - Ok(ProstMetadata(SequenceMetadata { - base: Some((&array.base()).into()), - multiplier: Some((&array.multiplier()).into()), - })) + Ok(SequenceMetadata { + base: array.base(), + multiplier: array.multiplier(), + }) } fn serialize(metadata: Self::Metadata) -> VortexResult>> { - Ok(Some(metadata.serialize())) + let prost = ProstMetadata(ProstSequenceMetadata { + base: Some((&metadata.base).into()), + multiplier: Some((&metadata.multiplier).into()), + }); + + Ok(Some(prost.serialize())) } fn deserialize( bytes: &[u8], - _dtype: &DType, + dtype: &DType, _len: usize, _buffers: &[BufferHandle], _session: &VortexSession, ) -> VortexResult { - Ok(ProstMetadata( - as DeserializeMetadata>::deserialize(bytes)?, - )) - } + let prost = ProstMetadata( + as DeserializeMetadata>::deserialize(bytes)?, + ); - fn build( - dtype: &DType, - len: usize, - metadata: &Self::Metadata, - _buffers: &[BufferHandle], - _children: &dyn ArrayChildren, - ) -> VortexResult { let ptype = dtype.as_ptype(); - // We go via scalar to cast the scalar values into the correct PType + // We go via Scalar to validate that the value is valid for the ptype. let base = Scalar::from_proto_value( - metadata + prost .0 .base .as_ref() @@ -331,10 +341,10 @@ impl VTable for SequenceVTable { )? .as_primitive() .pvalue() - .vortex_expect("non-nullable primitive"); + .vortex_expect("sequence array base should be a non-nullable primitive"); let multiplier = Scalar::from_proto_value( - metadata + prost .0 .multiplier .as_ref() @@ -343,15 +353,25 @@ impl VTable for SequenceVTable { )? .as_primitive() .pvalue() - .vortex_expect("non-nullable primitive"); + .vortex_expect("sequence array multiplier should be a non-nullable primitive"); - Ok(SequenceArray::unchecked_new( - base, - multiplier, - ptype, + Ok(SequenceMetadata { base, multiplier }) + } + + fn build( + dtype: &DType, + len: usize, + metadata: &Self::Metadata, + _buffers: &[BufferHandle], + _children: &dyn ArrayChildren, + ) -> VortexResult { + SequenceArray::try_new( + metadata.base, + metadata.multiplier, + dtype.as_ptype(), dtype.nullability(), len, - )) + ) } fn with_children(_array: &mut Self::Array, children: Vec) -> VortexResult<()> { @@ -433,7 +453,7 @@ mod tests { #[test] fn test_sequence_canonical() { - let arr = SequenceArray::typed_new(2i64, 3, Nullability::NonNullable, 4).unwrap(); + let arr = SequenceArray::try_new_typed(2i64, 3, Nullability::NonNullable, 4).unwrap(); let canon = PrimitiveArray::from_iter((0..4).map(|i| 2i64 + i * 3)); @@ -442,7 +462,7 @@ mod tests { #[test] fn test_sequence_slice_canonical() { - let arr = SequenceArray::typed_new(2i64, 3, Nullability::NonNullable, 4) + let arr = SequenceArray::try_new_typed(2i64, 3, Nullability::NonNullable, 4) .unwrap() .slice(2..3) .unwrap(); @@ -454,7 +474,7 @@ mod tests { #[test] fn test_sequence_scalar_at() { - let scalar = SequenceArray::typed_new(2i64, 3, Nullability::NonNullable, 4) + let scalar = SequenceArray::try_new_typed(2i64, 3, Nullability::NonNullable, 4) .unwrap() .scalar_at(2) .unwrap(); @@ -467,19 +487,19 @@ mod tests { #[test] fn test_sequence_min_max() { - assert!(SequenceArray::typed_new(-127i8, -1i8, Nullability::NonNullable, 2).is_ok()); - assert!(SequenceArray::typed_new(126i8, -1i8, Nullability::NonNullable, 2).is_ok()); + assert!(SequenceArray::try_new_typed(-127i8, -1i8, Nullability::NonNullable, 2).is_ok()); + assert!(SequenceArray::try_new_typed(126i8, -1i8, Nullability::NonNullable, 2).is_ok()); } #[test] fn test_sequence_too_big() { - assert!(SequenceArray::typed_new(127i8, 1i8, Nullability::NonNullable, 2).is_err()); - assert!(SequenceArray::typed_new(-128i8, -1i8, Nullability::NonNullable, 2).is_err()); + assert!(SequenceArray::try_new_typed(127i8, 1i8, Nullability::NonNullable, 2).is_err()); + assert!(SequenceArray::try_new_typed(-128i8, -1i8, Nullability::NonNullable, 2).is_err()); } #[test] fn positive_multiplier_is_strict_sorted() -> VortexResult<()> { - let arr = SequenceArray::typed_new(0i64, 3, Nullability::NonNullable, 4)?; + let arr = SequenceArray::try_new_typed(0i64, 3, Nullability::NonNullable, 4)?; let is_sorted = arr .statistics() @@ -495,7 +515,7 @@ mod tests { #[test] fn zero_multiplier_is_sorted_not_strict() -> VortexResult<()> { - let arr = SequenceArray::typed_new(5i64, 0, Nullability::NonNullable, 4)?; + let arr = SequenceArray::try_new_typed(5i64, 0, Nullability::NonNullable, 4)?; let is_sorted = arr .statistics() @@ -511,7 +531,7 @@ mod tests { #[test] fn negative_multiplier_not_sorted() -> VortexResult<()> { - let arr = SequenceArray::typed_new(10i64, -1, Nullability::NonNullable, 4)?; + let arr = SequenceArray::try_new_typed(10i64, -1, Nullability::NonNullable, 4)?; let is_sorted = arr .statistics() @@ -530,7 +550,7 @@ mod tests { #[test] fn test_large_multiplier_sorted() -> VortexResult<()> { let large_multiplier = (i64::MAX as u64) + 1; - let arr = SequenceArray::typed_new(0, large_multiplier, Nullability::NonNullable, 2)?; + let arr = SequenceArray::try_new_typed(0, large_multiplier, Nullability::NonNullable, 2)?; let is_sorted = arr .statistics() diff --git a/encodings/sequence/src/compress.rs b/encodings/sequence/src/compress.rs index 1a561f8b23f..83b70a90359 100644 --- a/encodings/sequence/src/compress.rs +++ b/encodings/sequence/src/compress.rs @@ -48,7 +48,7 @@ fn encode_primitive_array + CheckedAdd + CheckedSu ) -> VortexResult> { if slice.len() == 1 { // The multiplier here can be any value, zero is chosen - return SequenceArray::typed_new(slice[0], P::zero(), nullability, 1) + return SequenceArray::try_new_typed(slice[0], P::zero(), nullability, 1) .map(|a| Some(a.to_array())); } let base = slice[0]; @@ -69,7 +69,7 @@ fn encode_primitive_array + CheckedAdd + CheckedSu .windows(2) .all(|w| Some(w[1]) == w[0].checked_add(&multiplier)) .then_some( - SequenceArray::typed_new(base, multiplier, nullability, slice.len()) + SequenceArray::try_new_typed(base, multiplier, nullability, slice.len()) .map(|a| a.to_array()), ) .transpose() diff --git a/encodings/sequence/src/compute/cast.rs b/encodings/sequence/src/compute/cast.rs index 05efb1402e0..be330df22da 100644 --- a/encodings/sequence/src/compute/cast.rs +++ b/encodings/sequence/src/compute/cast.rs @@ -32,7 +32,7 @@ impl CastReduce for SequenceVTable { // For SequenceArray, we can just create a new one with the same parameters // but different nullability return Ok(Some( - SequenceArray::new( + SequenceArray::try_new( array.base(), array.multiplier(), *target_ptype, @@ -71,7 +71,7 @@ impl CastReduce for SequenceVTable { .ok_or_else(|| vortex_err!("Cast resulted in null multiplier value"))?; return Ok(Some( - SequenceArray::new( + SequenceArray::try_new( new_base, new_multiplier, *target_ptype, @@ -102,7 +102,8 @@ mod tests { #[test] fn test_cast_sequence_nullability() { - let sequence = SequenceArray::typed_new(0u32, 1u32, Nullability::NonNullable, 4).unwrap(); + let sequence = + SequenceArray::try_new_typed(0u32, 1u32, Nullability::NonNullable, 4).unwrap(); // Cast to nullable let casted = sequence @@ -118,7 +119,7 @@ mod tests { #[test] fn test_cast_sequence_u32_to_i64() { let sequence = - SequenceArray::typed_new(100u32, 10u32, Nullability::NonNullable, 4).unwrap(); + SequenceArray::try_new_typed(100u32, 10u32, Nullability::NonNullable, 4).unwrap(); let casted = sequence .to_array() @@ -137,7 +138,8 @@ mod tests { #[test] fn test_cast_sequence_i16_to_i32_nullable() { // Test ptype change AND nullability change in one cast - let sequence = SequenceArray::typed_new(5i16, 3i16, Nullability::NonNullable, 3).unwrap(); + let sequence = + SequenceArray::try_new_typed(5i16, 3i16, Nullability::NonNullable, 3).unwrap(); let casted = sequence .to_array() @@ -158,7 +160,8 @@ mod tests { #[test] fn test_cast_sequence_to_float_delegates_to_canonical() { - let sequence = SequenceArray::typed_new(0i32, 1i32, Nullability::NonNullable, 5).unwrap(); + let sequence = + SequenceArray::try_new_typed(0i32, 1i32, Nullability::NonNullable, 5).unwrap(); // Cast to float should delegate to canonical (SequenceArray doesn't support float) let casted = sequence @@ -180,15 +183,15 @@ mod tests { } #[rstest] - #[case::i32(SequenceArray::typed_new(0i32, 1i32, Nullability::NonNullable, 5).unwrap())] - #[case::u64(SequenceArray::typed_new(1000u64, 100u64, Nullability::NonNullable, 4).unwrap())] + #[case::i32(SequenceArray::try_new_typed(0i32, 1i32, Nullability::NonNullable, 5).unwrap())] + #[case::u64(SequenceArray::try_new_typed(1000u64, 100u64, Nullability::NonNullable, 4).unwrap())] // TODO(DK): SequenceArray does not actually conform. You cannot cast this array to u8 even // though all its values are representable therein. // - // #[case::negative_step(SequenceArray::typed_new(100i32, -10i32, Nullability::NonNullable, + // #[case::negative_step(SequenceArray::try_new_typed(100i32, -10i32, Nullability::NonNullable, // 5).unwrap())] - #[case::single(SequenceArray::typed_new(42i64, 0i64, Nullability::NonNullable, 1).unwrap())] - #[case::constant(SequenceArray::typed_new( + #[case::single(SequenceArray::try_new_typed(42i64, 0i64, Nullability::NonNullable, 1).unwrap())] + #[case::constant(SequenceArray::try_new_typed( 100i32, 0i32, // multiplier of 0 means constant array Nullability::NonNullable, diff --git a/encodings/sequence/src/compute/compare.rs b/encodings/sequence/src/compute/compare.rs index 9d22c19a04c..7d85da472a9 100644 --- a/encodings/sequence/src/compute/compare.rs +++ b/encodings/sequence/src/compute/compare.rs @@ -147,7 +147,7 @@ mod tests { #[test] fn test_compare_match() { - let lhs = SequenceArray::typed_new(2i64, 1, NonNullable, 4).unwrap(); + let lhs = SequenceArray::try_new_typed(2i64, 1, NonNullable, 4).unwrap(); let rhs = ConstantArray::new(4i64, lhs.len()); let result = lhs.to_array().binary(rhs.to_array(), Operator::Eq).unwrap(); let expected = BoolArray::from_iter([false, false, true, false]); @@ -156,7 +156,7 @@ mod tests { #[test] fn test_compare_match_scale() { - let lhs = SequenceArray::typed_new(2i64, 3, Nullable, 4).unwrap(); + let lhs = SequenceArray::try_new_typed(2i64, 3, Nullable, 4).unwrap(); let rhs = ConstantArray::new(8i64, lhs.len()); let result = lhs.to_array().binary(rhs.to_array(), Operator::Eq).unwrap(); let expected = BoolArray::from_iter([Some(false), Some(false), Some(true), Some(false)]); @@ -165,7 +165,7 @@ mod tests { #[test] fn test_compare_no_match() { - let lhs = SequenceArray::typed_new(2i64, 1, NonNullable, 4).unwrap(); + let lhs = SequenceArray::try_new_typed(2i64, 1, NonNullable, 4).unwrap(); let rhs = ConstantArray::new(1i64, lhs.len()); let result = lhs.to_array().binary(rhs.to_array(), Operator::Eq).unwrap(); let expected = BoolArray::from_iter([false, false, false, false]); diff --git a/encodings/sequence/src/compute/filter.rs b/encodings/sequence/src/compute/filter.rs index 42550763ad3..d42149dbe57 100644 --- a/encodings/sequence/src/compute/filter.rs +++ b/encodings/sequence/src/compute/filter.rs @@ -55,18 +55,18 @@ mod tests { use crate::SequenceArray; #[rstest] - #[case(SequenceArray::typed_new(0i32, 1, Nullability::NonNullable, 5).unwrap())] - #[case(SequenceArray::typed_new(10i32, 2, Nullability::NonNullable, 5).unwrap())] - #[case(SequenceArray::typed_new(100i32, -3, Nullability::NonNullable, 5).unwrap())] - #[case(SequenceArray::typed_new(0i32, 1, Nullability::NonNullable, 1).unwrap())] - #[case(SequenceArray::typed_new(0i32, 1, Nullability::NonNullable, MEDIUM_SIZE).unwrap())] - #[case(SequenceArray::typed_new(0i32, 1, Nullability::NonNullable, LARGE_SIZE).unwrap())] - #[case(SequenceArray::typed_new(0i64, 1, Nullability::NonNullable, 5).unwrap())] - #[case(SequenceArray::typed_new(1000i64, 50, Nullability::NonNullable, 5).unwrap())] - #[case(SequenceArray::typed_new(-100i64, 10, Nullability::NonNullable, MEDIUM_SIZE).unwrap())] - #[case(SequenceArray::typed_new(0u32, 1, Nullability::NonNullable, 5).unwrap())] - #[case(SequenceArray::typed_new(0u32, 5, Nullability::NonNullable, MEDIUM_SIZE).unwrap())] - #[case(SequenceArray::typed_new(0u64, 1, Nullability::NonNullable, LARGE_SIZE).unwrap())] + #[case(SequenceArray::try_new_typed(0i32, 1, Nullability::NonNullable, 5).unwrap())] + #[case(SequenceArray::try_new_typed(10i32, 2, Nullability::NonNullable, 5).unwrap())] + #[case(SequenceArray::try_new_typed(100i32, -3, Nullability::NonNullable, 5).unwrap())] + #[case(SequenceArray::try_new_typed(0i32, 1, Nullability::NonNullable, 1).unwrap())] + #[case(SequenceArray::try_new_typed(0i32, 1, Nullability::NonNullable, MEDIUM_SIZE).unwrap())] + #[case(SequenceArray::try_new_typed(0i32, 1, Nullability::NonNullable, LARGE_SIZE).unwrap())] + #[case(SequenceArray::try_new_typed(0i64, 1, Nullability::NonNullable, 5).unwrap())] + #[case(SequenceArray::try_new_typed(1000i64, 50, Nullability::NonNullable, 5).unwrap())] + #[case(SequenceArray::try_new_typed(-100i64, 10, Nullability::NonNullable, MEDIUM_SIZE).unwrap())] + #[case(SequenceArray::try_new_typed(0u32, 1, Nullability::NonNullable, 5).unwrap())] + #[case(SequenceArray::try_new_typed(0u32, 5, Nullability::NonNullable, MEDIUM_SIZE).unwrap())] + #[case(SequenceArray::try_new_typed(0u64, 1, Nullability::NonNullable, LARGE_SIZE).unwrap())] fn test_filter_sequence_conformance(#[case] array: SequenceArray) { test_filter_conformance(&array.to_array()); } diff --git a/encodings/sequence/src/compute/list_contains.rs b/encodings/sequence/src/compute/list_contains.rs index f67d3efeb33..ad2d7bf9611 100644 --- a/encodings/sequence/src/compute/list_contains.rs +++ b/encodings/sequence/src/compute/list_contains.rs @@ -73,7 +73,7 @@ mod tests { // [1, 3] in 1 // 2 // 3 - let array = SequenceArray::typed_new(1, 1, Nullability::NonNullable, 3).unwrap(); + let array = SequenceArray::try_new_typed(1, 1, Nullability::NonNullable, 3).unwrap(); let expr = list_contains(lit(list_scalar.clone()), root()); let result = array.apply(&expr).unwrap(); @@ -85,7 +85,7 @@ mod tests { // [1, 3] in 1 // 3 // 5 - let array = SequenceArray::typed_new(1, 2, Nullability::NonNullable, 3).unwrap(); + let array = SequenceArray::try_new_typed(1, 2, Nullability::NonNullable, 3).unwrap(); let expr = list_contains(lit(list_scalar), root()); let result = array.apply(&expr).unwrap(); diff --git a/encodings/sequence/src/compute/mod.rs b/encodings/sequence/src/compute/mod.rs index b08266b03ec..6e31a6e5da1 100644 --- a/encodings/sequence/src/compute/mod.rs +++ b/encodings/sequence/src/compute/mod.rs @@ -20,13 +20,13 @@ mod tests { #[rstest] // Basic sequence arrays - A[i] = base + i * multiplier - #[case::sequence_i32(SequenceArray::typed_new( + #[case::sequence_i32(SequenceArray::try_new_typed( 0i32, // base 1i32, // multiplier Nullability::NonNullable, 5 // length ).unwrap())] // Results in [0, 1, 2, 3, 4] - #[case::sequence_i64_step2(SequenceArray::typed_new( + #[case::sequence_i64_step2(SequenceArray::try_new_typed( 10i64, // base 2i64, // multiplier Nullability::NonNullable, @@ -34,13 +34,13 @@ mod tests { ).unwrap())] // Results in [10, 12, 14, 16, 18] // Different types - #[case::sequence_u32(SequenceArray::typed_new( + #[case::sequence_u32(SequenceArray::try_new_typed( 100u32, // base 10u32, // multiplier Nullability::NonNullable, 5 // length ).unwrap())] // Results in [100, 110, 120, 130, 140] - #[case::sequence_i16(SequenceArray::typed_new( + #[case::sequence_i16(SequenceArray::try_new_typed( -10i16, // base 3i16, // multiplier Nullability::NonNullable, @@ -48,19 +48,19 @@ mod tests { ).unwrap())] // Results in [-10, -7, -4, -1, 2] // Edge cases - #[case::sequence_single(SequenceArray::typed_new( + #[case::sequence_single(SequenceArray::try_new_typed( 42i32, 0i32, // multiplier of 0 means constant array Nullability::NonNullable, 1 ).unwrap())] - #[case::sequence_zero_multiplier(SequenceArray::typed_new( + #[case::sequence_zero_multiplier(SequenceArray::try_new_typed( 100i32, 0i32, // All values will be 100 Nullability::NonNullable, 5 ).unwrap())] - #[case::sequence_negative_step(SequenceArray::typed_new( + #[case::sequence_negative_step(SequenceArray::try_new_typed( 100i32, -10i32, // Decreasing sequence Nullability::NonNullable, @@ -68,13 +68,13 @@ mod tests { ).unwrap())] // Results in [100, 90, 80, 70, 60] // Large arrays - #[case::sequence_large(SequenceArray::typed_new( + #[case::sequence_large(SequenceArray::try_new_typed( 0i64, 1i64, Nullability::NonNullable, 2000 ).unwrap())] // Results in [0, 1, 2, ..., 1999] - #[case::sequence_large_step(SequenceArray::typed_new( + #[case::sequence_large_step(SequenceArray::try_new_typed( 1000i32, 100i32, Nullability::NonNullable, diff --git a/encodings/sequence/src/compute/slice.rs b/encodings/sequence/src/compute/slice.rs index d18fec1218b..32945c79906 100644 --- a/encodings/sequence/src/compute/slice.rs +++ b/encodings/sequence/src/compute/slice.rs @@ -12,14 +12,17 @@ use crate::SequenceVTable; impl SliceReduce for SequenceVTable { fn slice(array: &Self::Array, range: Range) -> VortexResult> { + // SAFETY: this is a slice of an already-validated `SequenceArray`, so this is still valid. Ok(Some( - SequenceArray::unchecked_new( - array.index_value(range.start), - array.multiplier(), - array.ptype(), - array.dtype().nullability(), - range.len(), - ) + unsafe { + SequenceArray::new_unchecked( + array.index_value(range.start), + array.multiplier(), + array.ptype(), + array.dtype().nullability(), + range.len(), + ) + } .to_array(), )) } diff --git a/encodings/sequence/src/compute/take.rs b/encodings/sequence/src/compute/take.rs index fa3c8e4a3c4..f7de138b780 100644 --- a/encodings/sequence/src/compute/take.rs +++ b/encodings/sequence/src/compute/take.rs @@ -112,49 +112,49 @@ mod test { use crate::SequenceArray; #[rstest] - #[case::basic_sequence(SequenceArray::typed_new( + #[case::basic_sequence(SequenceArray::try_new_typed( 0i32, 1i32, Nullability::NonNullable, 10 ).unwrap())] - #[case::sequence_with_multiplier(SequenceArray::typed_new( + #[case::sequence_with_multiplier(SequenceArray::try_new_typed( 10i32, 5i32, Nullability::Nullable, 20 ).unwrap())] - #[case::sequence_i64(SequenceArray::typed_new( + #[case::sequence_i64(SequenceArray::try_new_typed( 100i64, 10i64, Nullability::NonNullable, 50 ).unwrap())] - #[case::sequence_u32(SequenceArray::typed_new( + #[case::sequence_u32(SequenceArray::try_new_typed( 0u32, 2u32, Nullability::NonNullable, 100 ).unwrap())] - #[case::sequence_negative_step(SequenceArray::typed_new( + #[case::sequence_negative_step(SequenceArray::try_new_typed( 1000i32, -10i32, Nullability::Nullable, 30 ).unwrap())] - #[case::sequence_constant(SequenceArray::typed_new( + #[case::sequence_constant(SequenceArray::try_new_typed( 42i32, 0i32, // multiplier of 0 means all values are the same Nullability::Nullable, 15 ).unwrap())] - #[case::sequence_i16(SequenceArray::typed_new( + #[case::sequence_i16(SequenceArray::try_new_typed( -100i16, 3i16, Nullability::NonNullable, 25 ).unwrap())] - #[case::sequence_large(SequenceArray::typed_new( + #[case::sequence_large(SequenceArray::try_new_typed( 0i64, 1i64, Nullability::Nullable, @@ -168,7 +168,7 @@ mod test { #[test] #[should_panic(expected = "out of bounds")] fn test_bounds_check() { - let array = SequenceArray::typed_new(0i32, 1i32, Nullability::NonNullable, 10).unwrap(); + let array = SequenceArray::try_new_typed(0i32, 1i32, Nullability::NonNullable, 10).unwrap(); let indices = vortex_array::arrays::PrimitiveArray::from_iter([0i32, 20]); let _array = array .take(indices.to_array()) diff --git a/vortex-cuda/src/kernel/encodings/sequence.rs b/vortex-cuda/src/kernel/encodings/sequence.rs index 20f87d5a648..312d7ef1475 100644 --- a/vortex-cuda/src/kernel/encodings/sequence.rs +++ b/vortex-cuda/src/kernel/encodings/sequence.rs @@ -126,7 +126,7 @@ mod tests { ) { let mut cuda_ctx = CudaSession::create_execution_ctx(&VortexSession::empty()).unwrap(); - let array = SequenceArray::typed_new(base, multiplier, nullability, len).unwrap(); + let array = SequenceArray::try_new_typed(base, multiplier, nullability, len).unwrap(); let cpu_result = array.to_canonical().unwrap().into_array(); diff --git a/vortex-duckdb/src/e2e_test/vortex_scan_test.rs b/vortex-duckdb/src/e2e_test/vortex_scan_test.rs index f2191fcbb6f..8988a4264c3 100644 --- a/vortex-duckdb/src/e2e_test/vortex_scan_test.rs +++ b/vortex-duckdb/src/e2e_test/vortex_scan_test.rs @@ -792,7 +792,7 @@ async fn write_vortex_file_with_encodings() -> NamedTempFile { let rle_array = RunEndArray::try_new(run_ends.into_array(), run_values.into_array()).unwrap(); // 5. Sequence array - let sequence_array = SequenceArray::new( + let sequence_array = SequenceArray::try_new( PValue::I64(0), PValue::I64(10), PType::I64, diff --git a/vortex-duckdb/src/exporter/sequence.rs b/vortex-duckdb/src/exporter/sequence.rs index 55537635e35..380f57c95bf 100644 --- a/vortex-duckdb/src/exporter/sequence.rs +++ b/vortex-duckdb/src/exporter/sequence.rs @@ -54,7 +54,7 @@ mod tests { #[test] fn test_sequence() { - let arr = SequenceArray::typed_new(2, 5, Nullability::NonNullable, 100).unwrap(); + let arr = SequenceArray::try_new_typed(2, 5, Nullability::NonNullable, 100).unwrap(); let mut chunk = DataChunk::new([LogicalType::new(cpp::duckdb_type::DUCKDB_TYPE_INTEGER)]); new_exporter(&arr) diff --git a/vortex-layout/src/layouts/row_idx/mod.rs b/vortex-layout/src/layouts/row_idx/mod.rs index 44b8f012ca6..507d62a42cc 100644 --- a/vortex-layout/src/layouts/row_idx/mod.rs +++ b/vortex-layout/src/layouts/row_idx/mod.rs @@ -246,7 +246,7 @@ impl LayoutReader for RowIdxLayoutReader { // Returns a SequenceArray representing the row indices for the given row range, fn idx_array(row_offset: u64, row_range: &Range) -> SequenceArray { - SequenceArray::new( + SequenceArray::try_new( PValue::U64(row_offset + row_range.start), PValue::U64(1), PType::U64, diff --git a/vortex-python/src/arrays/range_to_sequence.rs b/vortex-python/src/arrays/range_to_sequence.rs index 4ff44616ba1..d3d3b8cd507 100644 --- a/vortex-python/src/arrays/range_to_sequence.rs +++ b/vortex-python/src/arrays/range_to_sequence.rs @@ -42,7 +42,7 @@ pub fn sequence_array_from_range + Into> vortex_bail!("Step, {}, does not fit in requested dtype: {}", step, dtype); }; - Ok(SequenceArray::typed_new::(start, step, dtype.nullability(), len)?.to_array()) + Ok(SequenceArray::try_new_typed::(start, step, dtype.nullability(), len)?.to_array()) } fn range_len(start: isize, stop: isize, step: isize) -> Option {