diff --git a/docs/developer-guide/internals/vtables.md b/docs/developer-guide/internals/vtables.md index 45ca3fe558a..e90c8a8f441 100644 --- a/docs/developer-guide/internals/vtables.md +++ b/docs/developer-guide/internals/vtables.md @@ -63,21 +63,21 @@ registered in the session by their ID. For a concept `Foo`, the components are organized into these files: -| File | Contains | -|---------------|-----------------------------------------------------------------------| -| `vtable.rs` | `FooVTable` trait definition | -| `typed.rs` | `Foo` data struct, inherent methods, `Deref` impl | -| `erased.rs` | `FooRef` struct, `DynFoo` sealed trait, blanket impl | -| `plugin.rs` | `FooPlugin` trait, registration | -| `matcher.rs` | Downcasting helpers (`is`, `as_`, `as_opt`, pattern matching traits) | +| File | Contains | +|--------------|----------------------------------------------------------------------| +| `vtable.rs` | `FooVTable` trait definition | +| `typed.rs` | `Foo` data struct, inherent methods, `Deref` impl | +| `erased.rs` | `FooRef` struct, `DynFoo` sealed trait, blanket impl | +| `plugin.rs` | `FooPlugin` trait, registration | +| `matcher.rs` | Downcasting helpers (`is`, `as_`, `as_opt`, pattern matching traits) | For Array encodings, each encoding has its own module (e.g. `arrays/primitive/`): -| File | Contains | -|-------------------------|-------------------------------------------------------------| -| `arrays/foo/mod.rs` | `V::Array` associated type, encoding-specific methods on it | -| `arrays/foo/vtable.rs` | `ArrayVTable` impl for this encoding | -| `arrays/foo/compute/` | Compute kernel implementations | +| File | Contains | +|------------------------|-------------------------------------------------------------| +| `arrays/foo/mod.rs` | `V::Array` associated type, encoding-specific methods on it | +| `arrays/foo/vtable.rs` | `ArrayVTable` impl for this encoding | +| `arrays/foo/compute/` | Compute kernel implementations | ## Example: ExtDType diff --git a/vortex-array/src/arrays/extension/compute/rules.rs b/vortex-array/src/arrays/extension/compute/rules.rs index d231c2691f6..45a3f31f1e7 100644 --- a/vortex-array/src/arrays/extension/compute/rules.rs +++ b/vortex-array/src/arrays/extension/compute/rules.rs @@ -95,18 +95,13 @@ mod tests { Ok(EmptyMetadata) } - fn validate_dtype( - &self, - _options: &Self::Metadata, - _storage_dtype: &DType, - ) -> VortexResult<()> { + fn validate_dtype(&self, _extension_dtype: &ExtDType) -> VortexResult<()> { Ok(()) } fn unpack_native<'a>( &self, - _metadata: &'a Self::Metadata, - _storage_dtype: &'a DType, + _extension_dtype: &'a ExtDType, _storage_value: &'a ScalarValue, ) -> VortexResult> { Ok("") @@ -197,18 +192,13 @@ mod tests { Ok(EmptyMetadata) } - fn validate_dtype( - &self, - _options: &Self::Metadata, - _storage_dtype: &DType, - ) -> VortexResult<()> { + fn validate_dtype(&self, _extension_dtype: &ExtDType) -> VortexResult<()> { Ok(()) } fn unpack_native<'a>( &self, - _metadata: &'a Self::Metadata, - _storage_dtype: &'a DType, + _extension_dtype: &'a ExtDType, _storage_value: &'a ScalarValue, ) -> VortexResult> { Ok("") diff --git a/vortex-array/src/dtype/extension/erased.rs b/vortex-array/src/dtype/extension/erased.rs index fd2b1814c35..272254d5d19 100644 --- a/vortex-array/src/dtype/extension/erased.rs +++ b/vortex-array/src/dtype/extension/erased.rs @@ -20,7 +20,6 @@ use crate::dtype::extension::ExtId; use crate::dtype::extension::ExtVTable; use crate::dtype::extension::Matcher; use crate::dtype::extension::typed::DynExtDType; -use crate::dtype::extension::typed::ExtDTypeInner; use crate::scalar::ScalarValue; /// A type-erased extension dtype. @@ -127,12 +126,10 @@ impl ExtDTypeRef { /// Downcast to the concrete [`ExtDType`]. /// /// Returns `Err(self)` if the downcast fails. - pub fn try_downcast(self) -> Result, ExtDTypeRef> { - if self.0.as_any().is::>() { - // SAFETY: type matches and ExtDTypeInner is the only implementor - let ptr = Arc::into_raw(self.0) as *const ExtDTypeInner; - let inner = unsafe { Arc::from_raw(ptr) }; - Ok(ExtDType(inner)) + pub fn try_downcast(self) -> Result>, ExtDTypeRef> { + if self.0.as_any().is::>() { + let ptr = Arc::into_raw(self.0) as *const ExtDType; + Ok(unsafe { Arc::from_raw(ptr) }) } else { Err(self) } @@ -143,7 +140,7 @@ impl ExtDTypeRef { /// # Panics /// /// Panics if the downcast fails. - pub fn downcast(self) -> ExtDType { + pub fn downcast(self) -> Arc> { self.try_downcast::() .map_err(|this| { vortex_err!( diff --git a/vortex-array/src/dtype/extension/matcher.rs b/vortex-array/src/dtype/extension/matcher.rs index 121f62d7aee..88e92f90233 100644 --- a/vortex-array/src/dtype/extension/matcher.rs +++ b/vortex-array/src/dtype/extension/matcher.rs @@ -1,9 +1,9 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: Copyright the Vortex contributors +use crate::dtype::extension::ExtDType; use crate::dtype::extension::ExtDTypeRef; use crate::dtype::extension::ExtVTable; -use crate::dtype::extension::typed::ExtDTypeInner; /// A trait for matching extension dtypes. pub trait Matcher { @@ -23,13 +23,13 @@ impl Matcher for V { type Match<'a> = &'a V::Metadata; fn matches(item: &ExtDTypeRef) -> bool { - item.0.as_any().is::>() + item.0.as_any().is::>() } fn try_match<'a>(item: &'a ExtDTypeRef) -> Option> { item.0 .as_any() - .downcast_ref::>() - .map(|inner| &inner.metadata) + .downcast_ref::>() + .map(|inner| inner.metadata()) } } diff --git a/vortex-array/src/dtype/extension/mod.rs b/vortex-array/src/dtype/extension/mod.rs index 2e818412e6d..c6ecc86bbf7 100644 --- a/vortex-array/src/dtype/extension/mod.rs +++ b/vortex-array/src/dtype/extension/mod.rs @@ -34,11 +34,11 @@ pub type ExtId = arcref::ArcRef; /// Private module to seal [`typed::DynExtDType`]. mod sealed { use crate::dtype::extension::ExtVTable; - use crate::dtype::extension::typed::ExtDTypeInner; + use crate::dtype::extension::typed::ExtDType; /// Marker trait to prevent external implementations of [`super::typed::DynExtDType`]. pub(crate) trait Sealed {} /// This can be the **only** implementor for [`super::typed::DynExtDType`]. - impl Sealed for ExtDTypeInner {} + impl Sealed for ExtDType {} } diff --git a/vortex-array/src/dtype/extension/typed.rs b/vortex-array/src/dtype/extension/typed.rs index 4cb1aff624c..32b4e42c9fe 100644 --- a/vortex-array/src/dtype/extension/typed.rs +++ b/vortex-array/src/dtype/extension/typed.rs @@ -32,7 +32,14 @@ use crate::scalar::ScalarValue; /// [`try_with_vtable()`]: ExtDType::try_with_vtable /// [`erased()`]: ExtDType::erased #[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub struct ExtDType(pub(super) Arc>); +pub struct ExtDType { + /// The extension dtype vtable. + vtable: V, + /// The extension dtype metadata. + metadata: V::Metadata, + /// The underlying storage dtype. + storage_dtype: DType, +} /// Convenience implementation for zero-sized VTables (or VTables that implement `Default`). impl ExtDType { @@ -49,60 +56,43 @@ impl ExtDType { metadata: V::Metadata, storage_dtype: DType, ) -> VortexResult { - vtable.validate_dtype(&metadata, &storage_dtype)?; - - Ok(Self(Arc::new(ExtDTypeInner:: { + let this = Self { vtable, metadata, storage_dtype, - }))) + }; + + this.vtable.validate_dtype(&this)?; + + Ok(this) } /// Returns the identifier of the extension type. pub fn id(&self) -> ExtId { - self.0.vtable.id() + self.vtable.id() } /// Returns the vtable of the extension type. pub fn vtable(&self) -> &V { - &self.0.vtable + &self.vtable } /// Returns the metadata of the extension type. pub fn metadata(&self) -> &V::Metadata { - &self.0.metadata + &self.metadata } /// Returns the storage dtype of the extension type. pub fn storage_dtype(&self) -> &DType { - &self.0.storage_dtype + &self.storage_dtype } /// Erase the concrete type information, returning a type-erased extension dtype. pub fn erased(self) -> ExtDTypeRef { - ExtDTypeRef(self.0) + ExtDTypeRef(Arc::new(self)) } } -// --------------------------------------------------------------------------- -// Private inner struct + sealed trait -// --------------------------------------------------------------------------- - -/// The private inner representation of an extension dtype, pairing a vtable with its metadata -/// and storage dtype. -/// -/// This is the sole implementor of [`DynExtDType`], enabling [`ExtDTypeRef`] to safely downcast -/// back to the concrete vtable type via [`Any`]. -#[derive(Debug, PartialEq, Eq, Hash)] -pub(super) struct ExtDTypeInner { - /// The extension dtype vtable. - pub(super) vtable: V, - /// The extension dtype metadata. - pub(super) metadata: V::Metadata, - /// The underlying storage dtype. - pub(super) storage_dtype: DType, -} - /// An object-safe, sealed trait encapsulating the behavior for extension dtypes. /// /// This provides type-erased access to the extension dtype's identity, storage dtype, and @@ -135,7 +125,7 @@ pub(super) trait DynExtDType: 'static + Send + Sync + super::sealed::Sealed { -> fmt::Result; } -impl DynExtDType for ExtDTypeInner { +impl DynExtDType for ExtDType { fn as_any(&self) -> &dyn Any { self } @@ -186,8 +176,7 @@ impl DynExtDType for ExtDTypeInner { } fn value_validate(&self, storage_value: &ScalarValue) -> VortexResult<()> { - self.vtable - .validate_scalar_value(&self.metadata, &self.storage_dtype, storage_value) + self.vtable.validate_scalar_value(self, storage_value) } fn value_display( @@ -195,10 +184,7 @@ impl DynExtDType for ExtDTypeInner { f: &mut fmt::Formatter<'_>, storage_value: &ScalarValue, ) -> fmt::Result { - match self - .vtable - .unpack_native(&self.metadata, &self.storage_dtype, storage_value) - { + match self.vtable.unpack_native(self, storage_value) { Ok(native) => fmt::Display::fmt(&native, f), Err(_) => write!( f, diff --git a/vortex-array/src/dtype/extension/vtable.rs b/vortex-array/src/dtype/extension/vtable.rs index 84f530ea06f..56be2b39121 100644 --- a/vortex-array/src/dtype/extension/vtable.rs +++ b/vortex-array/src/dtype/extension/vtable.rs @@ -7,7 +7,7 @@ use std::hash::Hash; use vortex_error::VortexResult; -use crate::dtype::DType; +use crate::dtype::extension::ExtDType; use crate::dtype::extension::ExtId; use crate::scalar::ScalarValue; @@ -36,7 +36,7 @@ pub trait ExtVTable: 'static + Sized + Send + Sync + Clone + Debug + Eq + Hash { fn deserialize_metadata(&self, metadata: &[u8]) -> VortexResult; /// Validate that the given storage type is compatible with this extension type. - fn validate_dtype(&self, metadata: &Self::Metadata, storage_dtype: &DType) -> VortexResult<()>; + fn validate_dtype(&self, ext_dtype: &ExtDType) -> VortexResult<()>; // Methods related to the extension scalar values. @@ -49,12 +49,10 @@ pub trait ExtVTable: 'static + Sized + Send + Sync + Clone + Debug + Eq + Hash { /// Returns an error if the storage [`ScalarValue`] is not compatible with the extension type. fn validate_scalar_value( &self, - metadata: &Self::Metadata, - storage_dtype: &DType, + ext_dtype: &ExtDType, storage_value: &ScalarValue, ) -> VortexResult<()> { - self.unpack_native(metadata, storage_dtype, storage_value) - .map(|_| ()) + self.unpack_native(ext_dtype, storage_value).map(|_| ()) } /// Validate and unpack a native value from the storage [`ScalarValue`]. @@ -68,8 +66,7 @@ pub trait ExtVTable: 'static + Sized + Send + Sync + Clone + Debug + Eq + Hash { /// Returns an error if the storage [`ScalarValue`] is not compatible with the extension type. fn unpack_native<'a>( &self, - metadata: &'a Self::Metadata, - storage_dtype: &'a DType, + ext_dtype: &'a ExtDType, storage_value: &'a ScalarValue, ) -> VortexResult>; } diff --git a/vortex-array/src/extension/datetime/date.rs b/vortex-array/src/extension/datetime/date.rs index 802bd8cf6a2..6d67051721b 100644 --- a/vortex-array/src/extension/datetime/date.rs +++ b/vortex-array/src/extension/datetime/date.rs @@ -91,12 +91,13 @@ impl ExtVTable for Date { TimeUnit::try_from(tag) } - fn validate_dtype(&self, metadata: &Self::Metadata, storage_dtype: &DType) -> VortexResult<()> { + fn validate_dtype(&self, ext_dtype: &ExtDType) -> VortexResult<()> { + let metadata = ext_dtype.metadata(); let ptype = date_ptype(metadata) .ok_or_else(|| vortex_err!("Date type does not support time unit {}", metadata))?; vortex_ensure!( - storage_dtype.as_ptype() == ptype, + ext_dtype.storage_dtype().as_ptype() == ptype, "Date storage dtype for {} must be {}", metadata, ptype @@ -107,10 +108,10 @@ impl ExtVTable for Date { fn unpack_native( &self, - metadata: &Self::Metadata, - _storage_dtype: &DType, + ext_dtype: &ExtDType, storage_value: &ScalarValue, ) -> VortexResult> { + let metadata = ext_dtype.metadata(); match metadata { TimeUnit::Milliseconds => Ok(DateValue::Milliseconds( storage_value.as_primitive().cast::()?, diff --git a/vortex-array/src/extension/datetime/time.rs b/vortex-array/src/extension/datetime/time.rs index 77982f5ee8e..59e7c98bc7e 100644 --- a/vortex-array/src/extension/datetime/time.rs +++ b/vortex-array/src/extension/datetime/time.rs @@ -92,12 +92,13 @@ impl ExtVTable for Time { TimeUnit::try_from(tag) } - fn validate_dtype(&self, metadata: &Self::Metadata, storage_dtype: &DType) -> VortexResult<()> { + fn validate_dtype(&self, ext_dtype: &ExtDType) -> VortexResult<()> { + let metadata = ext_dtype.metadata(); let ptype = time_ptype(metadata) .ok_or_else(|| vortex_err!("Time type does not support time unit {}", metadata))?; vortex_ensure!( - storage_dtype.as_ptype() == ptype, + ext_dtype.storage_dtype().as_ptype() == ptype, "Time storage dtype for {} must be {}", metadata, ptype @@ -108,13 +109,12 @@ impl ExtVTable for Time { fn unpack_native( &self, - metadata: &Self::Metadata, - _storage_dtype: &DType, + ext_dtype: &ExtDType, storage_value: &ScalarValue, ) -> VortexResult> { let length_of_time = storage_value.as_primitive().cast::()?; - let (span, value) = match *metadata { + let (span, value) = match *ext_dtype.metadata() { TimeUnit::Seconds => { let v = i32::try_from(length_of_time) .map_err(|e| vortex_err!("Time seconds value out of i32 range: {e}"))?; diff --git a/vortex-array/src/extension/datetime/timestamp.rs b/vortex-array/src/extension/datetime/timestamp.rs index d65f6b24cce..d314cef60a3 100644 --- a/vortex-array/src/extension/datetime/timestamp.rs +++ b/vortex-array/src/extension/datetime/timestamp.rs @@ -169,13 +169,9 @@ impl ExtVTable for Timestamp { }) } - fn validate_dtype( - &self, - _metadata: &Self::Metadata, - storage_dtype: &DType, - ) -> VortexResult<()> { + fn validate_dtype(&self, ext_dtype: &ExtDType) -> VortexResult<()> { vortex_ensure!( - matches!(storage_dtype, DType::Primitive(PType::I64, _)), + matches!(ext_dtype.storage_dtype(), DType::Primitive(PType::I64, _)), "Timestamp storage dtype must be i64" ); Ok(()) @@ -183,10 +179,10 @@ impl ExtVTable for Timestamp { fn unpack_native<'a>( &self, - metadata: &'a Self::Metadata, - _storage_dtype: &'a DType, + ext_dtype: &'a ExtDType, storage_value: &'a ScalarValue, ) -> VortexResult> { + let metadata = ext_dtype.metadata(); let ts_value = storage_value.as_primitive().cast::()?; let tz = metadata.tz.as_ref(); diff --git a/vortex-array/src/extension/tests/divisible_int.rs b/vortex-array/src/extension/tests/divisible_int.rs index c34fcea3df6..4bdd384422c 100644 --- a/vortex-array/src/extension/tests/divisible_int.rs +++ b/vortex-array/src/extension/tests/divisible_int.rs @@ -53,23 +53,22 @@ impl ExtVTable for DivisibleInt { fn validate_dtype( &self, - _metadata: &Self::Metadata, - storage_dtype: &DType, + ext_dtype: &crate::dtype::extension::ExtDType, ) -> VortexResult<()> { vortex_ensure!( - matches!(storage_dtype, DType::Primitive(PType::U64, _)), + matches!(ext_dtype.storage_dtype(), DType::Primitive(PType::U64, _)), "divisible int storage dtype must be u64" ); Ok(()) } - fn unpack_native( + fn unpack_native<'a>( &self, - metadata: &Self::Metadata, - _storage_dtype: &DType, - storage_value: &ScalarValue, - ) -> VortexResult> { + ext_dtype: &'a crate::dtype::extension::ExtDType, + storage_value: &'a ScalarValue, + ) -> VortexResult> { let value = storage_value.as_primitive().cast::()?; + let metadata = ext_dtype.metadata(); if value % metadata.0 != 0 { vortex_bail!("{} is not divisible by {}", value, metadata.0); } @@ -86,6 +85,7 @@ mod tests { use crate::dtype::DType; use crate::dtype::Nullability; use crate::dtype::PType; + use crate::dtype::extension::ExtDType; use crate::dtype::extension::ExtVTable; #[test] @@ -109,29 +109,28 @@ mod tests { #[test] fn rejects_wrong_storage_dtype() { - let vtable = DivisibleInt; let divisor = Divisor(10); assert!( - vtable - .validate_dtype( - &divisor, - &DType::Primitive(PType::I32, Nullability::NonNullable) - ) - .is_err() + ExtDType::try_new( + divisor.clone(), + DType::Primitive(PType::I32, Nullability::NonNullable) + ) + .is_err() ); assert!( - vtable - .validate_dtype(&divisor, &DType::Utf8(Nullability::NonNullable)) - .is_err() + ExtDType::::try_new( + divisor.clone(), + DType::Utf8(Nullability::NonNullable) + ) + .is_err() ); assert!( - vtable - .validate_dtype( - &divisor, - &DType::Primitive(PType::U64, Nullability::NonNullable) - ) - .is_ok() + ExtDType::try_new( + divisor, + DType::Primitive(PType::U64, Nullability::NonNullable) + ) + .is_ok() ); } } diff --git a/vortex-array/src/extension/uuid/vtable.rs b/vortex-array/src/extension/uuid/vtable.rs index 3564d0cf39f..d1a3d0d4a45 100644 --- a/vortex-array/src/extension/uuid/vtable.rs +++ b/vortex-array/src/extension/uuid/vtable.rs @@ -10,6 +10,7 @@ use vortex_error::vortex_err; use crate::dtype::DType; use crate::dtype::PType; +use crate::dtype::extension::ExtDType; use crate::dtype::extension::ExtId; use crate::dtype::extension::ExtVTable; use crate::extension::uuid::Uuid; @@ -46,11 +47,8 @@ impl ExtVTable for Uuid { Ok(UuidMetadata { version }) } - fn validate_dtype( - &self, - _metadata: &Self::Metadata, - storage_dtype: &DType, - ) -> VortexResult<()> { + fn validate_dtype(&self, ext_dtype: &ExtDType) -> VortexResult<()> { + let storage_dtype = ext_dtype.storage_dtype(); let DType::FixedSizeList(element_dtype, list_size, _nullability) = storage_dtype else { vortex_bail!("UUID storage dtype must be a FixedSizeList, got {storage_dtype}"); }; @@ -80,8 +78,7 @@ impl ExtVTable for Uuid { fn unpack_native<'a>( &self, - metadata: &'a Self::Metadata, - _storage_dtype: &'a DType, + ext_dtype: &ExtDType, storage_value: &'a ScalarValue, ) -> VortexResult> { let elements = storage_value.as_list(); @@ -106,7 +103,7 @@ impl ExtVTable for Uuid { let parsed = uuid::Uuid::from_bytes(bytes); // Verify the parsed UUID matches the expected version, if one is set. - if let Some(expected) = metadata.version { + if let Some(expected) = ext_dtype.metadata().version { let expected = expected as u8; let actual = parsed .get_version() @@ -139,6 +136,7 @@ mod tests { use crate::dtype::DType; use crate::dtype::Nullability; use crate::dtype::PType; + use crate::dtype::extension::ExtDType; use crate::dtype::extension::ExtVTable; use crate::extension::uuid::Uuid; use crate::extension::uuid::UuidMetadata; @@ -187,7 +185,8 @@ mod tests { fn validate_correct_storage_dtype(#[case] nullability: Nullability) -> VortexResult<()> { let metadata = UuidMetadata::default(); let storage_dtype = uuid_storage_dtype(nullability); - Uuid.validate_dtype(&metadata, &storage_dtype) + ExtDType::try_with_vtable(Uuid, metadata, storage_dtype)?; + Ok(()) } #[test] @@ -198,8 +197,7 @@ mod tests { Nullability::NonNullable, ); assert!( - Uuid.validate_dtype(&UuidMetadata::default(), &storage_dtype) - .is_err() + ExtDType::try_with_vtable(Uuid, UuidMetadata::default(), storage_dtype).is_err() ); } @@ -211,8 +209,7 @@ mod tests { Nullability::NonNullable, ); assert!( - Uuid.validate_dtype(&UuidMetadata::default(), &storage_dtype) - .is_err() + ExtDType::try_with_vtable(Uuid, UuidMetadata::default(), storage_dtype).is_err() ); } @@ -224,8 +221,7 @@ mod tests { Nullability::NonNullable, ); assert!( - Uuid.validate_dtype(&UuidMetadata::default(), &storage_dtype) - .is_err() + ExtDType::try_with_vtable(Uuid, UuidMetadata::default(), storage_dtype).is_err() ); } @@ -233,8 +229,7 @@ mod tests { fn validate_rejects_non_fsl() { let storage_dtype = DType::Primitive(PType::U8, Nullability::NonNullable); assert!( - Uuid.validate_dtype(&UuidMetadata::default(), &storage_dtype) - .is_err() + ExtDType::try_with_vtable(Uuid, UuidMetadata::default(), storage_dtype).is_err() ); } @@ -243,8 +238,10 @@ mod tests { let expected = uuid::Uuid::parse_str("550e8400-e29b-41d4-a716-446655440000") .map_err(|e| vortex_error::vortex_err!("{e}"))?; - let metadata = UuidMetadata::default(); - let storage_dtype = uuid_storage_dtype(Nullability::NonNullable); + let ext_dtype = ExtDType::try_new( + UuidMetadata::default(), + uuid_storage_dtype(Nullability::NonNullable), + )?; let children: Vec = expected .as_bytes() .iter() @@ -259,7 +256,7 @@ mod tests { let storage_value = storage_scalar .value() .ok_or_else(|| vortex_error::vortex_err!("expected non-null scalar"))?; - let result = Uuid.unpack_native(&metadata, &storage_dtype, storage_value)?; + let result = Uuid.unpack_native(&ext_dtype, storage_value)?; assert_eq!(result, expected); assert_eq!(result.to_string(), "550e8400-e29b-41d4-a716-446655440000"); Ok(()) @@ -273,10 +270,13 @@ mod tests { assert_eq!(v4_uuid.get_version(), Some(Version::Random)); // Metadata says v7, but the UUID is v4. - let metadata = UuidMetadata { - version: Some(Version::SortRand), - }; - let storage_dtype = uuid_storage_dtype(Nullability::NonNullable); + let ext_dtype = ExtDType::try_with_vtable( + Uuid, + UuidMetadata { + version: Some(Version::SortRand), + }, + uuid_storage_dtype(Nullability::NonNullable), + )?; let children: Vec = v4_uuid .as_bytes() .iter() @@ -291,10 +291,7 @@ mod tests { let storage_value = storage_scalar .value() .ok_or_else(|| vortex_error::vortex_err!("expected non-null scalar"))?; - assert!( - Uuid.unpack_native(&metadata, &storage_dtype, storage_value) - .is_err() - ); + assert!(Uuid.unpack_native(&ext_dtype, storage_value).is_err()); Ok(()) } diff --git a/vortex-array/src/scalar/arrow.rs b/vortex-array/src/scalar/arrow.rs index bce11e27e0f..45672e745fa 100644 --- a/vortex-array/src/scalar/arrow.rs +++ b/vortex-array/src/scalar/arrow.rs @@ -466,16 +466,14 @@ mod tests { fn validate_dtype( &self, - _options: &Self::Metadata, - _storage_dtype: &DType, + _ext_dtype: &crate::dtype::extension::ExtDType, ) -> VortexResult<()> { Ok(()) } fn unpack_native<'a>( &self, - _metadata: &'a Self::Metadata, - _storage_dtype: &'a DType, + _ext_dtype: &'a crate::dtype::extension::ExtDType, _storage_value: &'a ScalarValue, ) -> VortexResult> { Ok("") diff --git a/vortex-array/src/scalar/tests/casting.rs b/vortex-array/src/scalar/tests/casting.rs index 72b44a5ed55..af047e7497a 100644 --- a/vortex-array/src/scalar/tests/casting.rs +++ b/vortex-array/src/scalar/tests/casting.rs @@ -45,16 +45,14 @@ mod tests { fn validate_dtype( &self, - _options: &Self::Metadata, - _storage_dtype: &DType, + _ext_dtype: &ExtDType, ) -> VortexResult<()> { Ok(()) } fn unpack_native<'a>( &self, - _metadata: &'a Self::Metadata, - _storage_dtype: &'a DType, + _ext_dtype: &'a ExtDType, _storage_value: &'a ScalarValue, ) -> VortexResult> { Ok("") @@ -266,16 +264,14 @@ mod tests { fn validate_dtype( &self, - _options: &Self::Metadata, - _storage_dtype: &DType, + _ext_dtype: &ExtDType, ) -> VortexResult<()> { Ok(()) } fn unpack_native<'a>( &self, - _metadata: &'a Self::Metadata, - _storage_dtype: &'a DType, + _ext_dtype: &'a ExtDType, _storage_value: &'a ScalarValue, ) -> VortexResult> { Ok("") @@ -329,16 +325,14 @@ mod tests { fn validate_dtype( &self, - _options: &Self::Metadata, - _storage_dtype: &DType, + _ext_dtype: &ExtDType, ) -> VortexResult<()> { Ok(()) } fn unpack_native<'a>( &self, - _metadata: &'a Self::Metadata, - _storage_dtype: &'a DType, + _ext_dtype: &'a ExtDType, _storage_value: &'a ScalarValue, ) -> VortexResult> { Ok("") diff --git a/vortex-array/src/scalar/typed_view/extension/tests.rs b/vortex-array/src/scalar/typed_view/extension/tests.rs index 52a4127e12c..63c1da8b27e 100644 --- a/vortex-array/src/scalar/typed_view/extension/tests.rs +++ b/vortex-array/src/scalar/typed_view/extension/tests.rs @@ -34,16 +34,14 @@ impl ExtVTable for TestI32Ext { fn validate_dtype( &self, - _options: &Self::Metadata, - _storage_dtype: &DType, + _ext_dtype: &ExtDType, ) -> VortexResult<()> { Ok(()) } fn unpack_native<'a>( &self, - _metadata: &'a Self::Metadata, - _storage_dtype: &'a DType, + _ext_dtype: &'a ExtDType, _storage_value: &'a ScalarValue, ) -> VortexResult> { Ok("") @@ -123,16 +121,14 @@ fn test_ext_scalar_partial_ord_different_types() { fn validate_dtype( &self, - _options: &Self::Metadata, - _storage_dtype: &DType, + _ext_dtype: &ExtDType, ) -> VortexResult<()> { Ok(()) } fn unpack_native<'a>( &self, - _metadata: &'a Self::Metadata, - _storage_dtype: &'a DType, + _ext_dtype: &'a ExtDType, _storage_value: &'a ScalarValue, ) -> VortexResult> { Ok("") @@ -311,16 +307,14 @@ fn test_ext_scalar_with_metadata() { fn validate_dtype( &self, - _options: &Self::Metadata, - _storage_dtype: &DType, + _ext_dtype: &ExtDType, ) -> VortexResult<()> { Ok(()) } fn unpack_native<'a>( &self, - _metadata: &'a Self::Metadata, - _storage_dtype: &'a DType, + _ext_dtype: &'a ExtDType, _storage_value: &'a ScalarValue, ) -> VortexResult> { Ok("") diff --git a/vortex-duckdb/src/convert/dtype.rs b/vortex-duckdb/src/convert/dtype.rs index aeffa40140b..78cfed459f3 100644 --- a/vortex-duckdb/src/convert/dtype.rs +++ b/vortex-duckdb/src/convert/dtype.rs @@ -593,16 +593,14 @@ mod tests { fn validate_dtype( &self, - _options: &Self::Metadata, - _storage_dtype: &DType, + _ext_dtype: &ExtDType, ) -> VortexResult<()> { Ok(()) } fn unpack_native<'a>( &self, - _metadata: &'a Self::Metadata, - _storage_dtype: &'a DType, + _ext_dtype: &'a ExtDType, _storage_value: &'a ScalarValue, ) -> VortexResult> { Ok("") diff --git a/vortex-tensor/src/fixed_shape/vtable.rs b/vortex-tensor/src/fixed_shape/vtable.rs index fd3b1533ee7..15e47456ba9 100644 --- a/vortex-tensor/src/fixed_shape/vtable.rs +++ b/vortex-tensor/src/fixed_shape/vtable.rs @@ -2,6 +2,7 @@ // SPDX-FileCopyrightText: Copyright the Vortex contributors use vortex::dtype::DType; +use vortex::dtype::extension::ExtDType; use vortex::dtype::extension::ExtId; use vortex::dtype::extension::ExtVTable; use vortex::error::VortexResult; @@ -32,7 +33,8 @@ impl ExtVTable for FixedShapeTensor { proto::deserialize(metadata) } - fn validate_dtype(&self, metadata: &Self::Metadata, storage_dtype: &DType) -> VortexResult<()> { + fn validate_dtype(&self, ext_dtype: &ExtDType) -> VortexResult<()> { + let storage_dtype = ext_dtype.storage_dtype(); let DType::FixedSizeList(element_dtype, list_size, _nullability) = storage_dtype else { vortex_bail!( "FixedShapeTensor storage dtype must be a FixedSizeList, got {storage_dtype}" @@ -50,7 +52,7 @@ impl ExtVTable for FixedShapeTensor { "FixedShapeTensor element dtype must be non-nullable (may change in the future)" ); - let element_count: usize = metadata.logical_shape().iter().product(); + let element_count: usize = ext_dtype.metadata().logical_shape().iter().product(); vortex_ensure_eq!( element_count, *list_size as usize, @@ -63,8 +65,7 @@ impl ExtVTable for FixedShapeTensor { fn unpack_native<'a>( &self, - _metadata: &'a Self::Metadata, - _storage_dtype: &'a DType, + _ext_dtype: &'a ExtDType, storage_value: &'a ScalarValue, ) -> VortexResult> { // TODO(connor): This is just a placeholder. However, even if we have a dedicated native