Skip to content

Commit b3ed0d0

Browse files
gatesngatesn
authored
Unify ExtVTable per vtable design doc (#6662)
See #6544 --------- Signed-off-by: Nicholas Gates <nick@nickgates.com>
1 parent f8f3a4a commit b3ed0d0

14 files changed

Lines changed: 500 additions & 482 deletions

File tree

‎docs/developer-guide/internals/vtables.md‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,23 @@ plugin authors implement) from the public API (what callers use). The public API
9797
inputs, enforce invariants, and transform outputs without exposing those concerns to vtable
9898
implementors. With `dyn Trait`, the trait surface is the public API.
9999

100+
## File Layout Convention
101+
102+
Each vtable-backed concept `Foo` lives in its own module directory with a consistent file
103+
structure:
104+
105+
| File | Contents |
106+
|--------------|-------------------------------------------------------------------|
107+
| `vtable.rs` | `FooVTable` — the non-object-safe trait users implement |
108+
| `plugin.rs` | `FooPlugin` — registry trait for deserialization + blanket impl |
109+
| `typed.rs` | `Foo<V>` + `FooInner<V>` + `DynFoo` + impl (typed wrapper + guts) |
110+
| `erased.rs` | `FooRef` + Display/Debug/PartialEq/Hash impls (erased public API) |
111+
| `matcher.rs` | `Matcher` trait + blanket impl for `V: FooVTable` |
112+
| `mod.rs` | Re-exports, `FooId` type alias, sealed module |
113+
114+
The private internals (`FooInner`, `DynFoo`, sealed module) are `pub(super)` within the
115+
concept's module. Everything else is re-exported from `mod.rs`.
116+
100117
## Registration and Deserialization
101118

102119
Vtables are registered in the session by their ID. When a serialized value is encountered
@@ -113,13 +130,13 @@ All four vtable-backed types are converging on the pattern described above.
113130

114131
### ExtDType -- Done
115132

116-
The reference implementation. `ExtVTable`, `ExtDType<V>`, `ExtDTypeRef`, `ExtDTypeAdapter`,
117-
`DynExtDType`, and `ExtDTypePlugin` are all in place. Naming needs to be updated to match the
118-
conventions above (e.g. `ExtDTypeImpl` → `DynExtDType`, `ExtDTypeAdapter` → `ExtDTypeInner`,
119-
`DynExtVTable` → `ExtDTypePlugin`).
120-
`ExtDTypeMetadata` (the erased metadata wrapper) should be removed -- its methods
121-
(`serialize`, `Display`, `Debug`, `PartialEq`, `Hash`) should move to direct methods
122-
on `ExtDTypeRef`.
133+
The reference implementation. All components follow the convention:
134+
135+
- `ExtVTable` (vtable trait, uniquely not prefixed `ExtDType` for historical reasons)
136+
- `ExtDType<V>`, `ExtDTypeRef`, `ExtDTypeInner`, `DynExtDType`, `ExtDTypePlugin`
137+
- `Matcher` trait with blanket impl
138+
- File layout: `vtable.rs`, `plugin.rs`, `typed.rs`, `erased.rs`, `matcher.rs`
139+
- `ExtDTypeMetadata` wrapper removed; methods inlined on `ExtDTypeRef`
123140

124141
### Expr -- Not started
125142

‎vortex-array/public-api.lock‎

Lines changed: 15 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -6296,31 +6296,25 @@ pub fn vortex_array::dtype::extension::ExtDType<V>::hash<__H: core::hash::Hasher
62966296

62976297
impl<V: vortex_array::dtype::extension::ExtVTable> core::marker::StructuralPartialEq for vortex_array::dtype::extension::ExtDType<V>
62986298

6299-
pub struct vortex_array::dtype::extension::ExtDTypeMetadata<'a>
6300-
6301-
impl vortex_array::dtype::extension::ExtDTypeMetadata<'_>
6302-
6303-
pub fn vortex_array::dtype::extension::ExtDTypeMetadata<'_>::serialize(&self) -> vortex_error::VortexResult<alloc::vec::Vec<u8>>
6304-
6305-
impl core::cmp::Eq for vortex_array::dtype::extension::ExtDTypeMetadata<'_>
6299+
pub struct vortex_array::dtype::extension::ExtDTypeRef(_)
63066300

6307-
impl core::cmp::PartialEq for vortex_array::dtype::extension::ExtDTypeMetadata<'_>
6301+
impl vortex_array::dtype::extension::ExtDTypeRef
63086302

6309-
pub fn vortex_array::dtype::extension::ExtDTypeMetadata<'_>::eq(&self, other: &Self) -> bool
6303+
pub fn vortex_array::dtype::extension::ExtDTypeRef::display_metadata(&self) -> impl core::fmt::Display + '_
63106304

6311-
impl core::fmt::Debug for vortex_array::dtype::extension::ExtDTypeMetadata<'_>
6305+
pub fn vortex_array::dtype::extension::ExtDTypeRef::eq_ignore_nullability(&self, other: &Self) -> bool
63126306

6313-
pub fn vortex_array::dtype::extension::ExtDTypeMetadata<'_>::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result
6307+
pub fn vortex_array::dtype::extension::ExtDTypeRef::id(&self) -> vortex_array::dtype::extension::ExtId
63146308

6315-
impl core::fmt::Display for vortex_array::dtype::extension::ExtDTypeMetadata<'_>
6309+
pub fn vortex_array::dtype::extension::ExtDTypeRef::is_nullable(&self) -> bool
63166310

6317-
pub fn vortex_array::dtype::extension::ExtDTypeMetadata<'_>::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result
6311+
pub fn vortex_array::dtype::extension::ExtDTypeRef::nullability(&self) -> vortex_array::dtype::Nullability
63186312

6319-
impl core::hash::Hash for vortex_array::dtype::extension::ExtDTypeMetadata<'_>
6313+
pub fn vortex_array::dtype::extension::ExtDTypeRef::serialize_metadata(&self) -> vortex_error::VortexResult<alloc::vec::Vec<u8>>
63206314

6321-
pub fn vortex_array::dtype::extension::ExtDTypeMetadata<'_>::hash<H: core::hash::Hasher>(&self, state: &mut H)
6315+
pub fn vortex_array::dtype::extension::ExtDTypeRef::storage_dtype(&self) -> &vortex_array::dtype::DType
63226316

6323-
pub struct vortex_array::dtype::extension::ExtDTypeRef(_)
6317+
pub fn vortex_array::dtype::extension::ExtDTypeRef::with_nullability(&self, nullability: vortex_array::dtype::Nullability) -> Self
63246318

63256319
impl vortex_array::dtype::extension::ExtDTypeRef
63266320

@@ -6334,22 +6328,6 @@ pub fn vortex_array::dtype::extension::ExtDTypeRef::metadata_opt<M: vortex_array
63346328

63356329
pub fn vortex_array::dtype::extension::ExtDTypeRef::try_downcast<V: vortex_array::dtype::extension::ExtVTable>(self) -> core::result::Result<vortex_array::dtype::extension::ExtDType<V>, vortex_array::dtype::extension::ExtDTypeRef>
63366330

6337-
impl vortex_array::dtype::extension::ExtDTypeRef
6338-
6339-
pub fn vortex_array::dtype::extension::ExtDTypeRef::eq_ignore_nullability(&self, other: &Self) -> bool
6340-
6341-
pub fn vortex_array::dtype::extension::ExtDTypeRef::id(&self) -> vortex_array::dtype::extension::ExtId
6342-
6343-
pub fn vortex_array::dtype::extension::ExtDTypeRef::is_nullable(&self) -> bool
6344-
6345-
pub fn vortex_array::dtype::extension::ExtDTypeRef::metadata_erased(&self) -> vortex_array::dtype::extension::ExtDTypeMetadata<'_>
6346-
6347-
pub fn vortex_array::dtype::extension::ExtDTypeRef::nullability(&self) -> vortex_array::dtype::Nullability
6348-
6349-
pub fn vortex_array::dtype::extension::ExtDTypeRef::storage_dtype(&self) -> &vortex_array::dtype::DType
6350-
6351-
pub fn vortex_array::dtype::extension::ExtDTypeRef::with_nullability(&self, nullability: vortex_array::dtype::Nullability) -> Self
6352-
63536331
impl core::clone::Clone for vortex_array::dtype::extension::ExtDTypeRef
63546332

63556333
pub fn vortex_array::dtype::extension::ExtDTypeRef::clone(&self) -> vortex_array::dtype::extension::ExtDTypeRef
@@ -6372,13 +6350,13 @@ impl core::hash::Hash for vortex_array::dtype::extension::ExtDTypeRef
63726350

63736351
pub fn vortex_array::dtype::extension::ExtDTypeRef::hash<H: core::hash::Hasher>(&self, state: &mut H)
63746352

6375-
pub trait vortex_array::dtype::extension::DynExtVTable: 'static + core::marker::Send + core::marker::Sync + core::fmt::Debug
6353+
pub trait vortex_array::dtype::extension::ExtDTypePlugin: 'static + core::marker::Send + core::marker::Sync + core::fmt::Debug
63766354

6377-
pub fn vortex_array::dtype::extension::DynExtVTable::deserialize(&self, data: &[u8], storage_dtype: vortex_array::dtype::DType) -> vortex_error::VortexResult<vortex_array::dtype::extension::ExtDTypeRef>
6355+
pub fn vortex_array::dtype::extension::ExtDTypePlugin::deserialize(&self, data: &[u8], storage_dtype: vortex_array::dtype::DType) -> vortex_error::VortexResult<vortex_array::dtype::extension::ExtDTypeRef>
63786356

6379-
pub fn vortex_array::dtype::extension::DynExtVTable::id(&self) -> vortex_array::dtype::extension::ExtId
6357+
pub fn vortex_array::dtype::extension::ExtDTypePlugin::id(&self) -> vortex_array::dtype::extension::ExtId
63806358

6381-
impl<V: vortex_array::dtype::extension::ExtVTable> vortex_array::dtype::extension::DynExtVTable for V
6359+
impl<V: vortex_array::dtype::extension::ExtVTable> vortex_array::dtype::extension::ExtDTypePlugin for V
63826360

63836361
pub fn V::deserialize(&self, data: &[u8], storage_dtype: vortex_array::dtype::DType) -> core::result::Result<vortex_array::dtype::extension::ExtDTypeRef, vortex_error::VortexError>
63846362

@@ -6492,7 +6470,7 @@ impl<S: vortex_session::SessionExt> vortex_array::dtype::session::DTypeSessionEx
64926470

64936471
pub fn S::dtypes(&self) -> vortex_session::Ref<'_, vortex_array::dtype::session::DTypeSession>
64946472

6495-
pub type vortex_array::dtype::session::ExtDTypeRegistry = vortex_session::registry::Registry<alloc::sync::Arc<dyn vortex_array::dtype::extension::DynExtVTable>>
6473+
pub type vortex_array::dtype::session::ExtDTypeRegistry = vortex_session::registry::Registry<alloc::sync::Arc<dyn vortex_array::dtype::extension::ExtDTypePlugin>>
64966474

64976475
pub enum vortex_array::dtype::DType
64986476

0 commit comments

Comments
 (0)