Skip to content

Commit 15dd7ab

Browse files
gatesngatesn
authored
Remove ScalarFnInner (#6897)
Signed-off-by: Nicholas Gates <nick@nickgates.com>
1 parent af5e659 commit 15dd7ab

File tree

5 files changed

+91
-72
lines changed

5 files changed

+91
-72
lines changed

‎vortex-array/public-api.lock‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18636,7 +18636,7 @@ pub fn vortex_array::scalar_fn::EmptyOptions::hash<__H: core::hash::Hasher>(&sel
1863618636

1863718637
impl core::marker::StructuralPartialEq for vortex_array::scalar_fn::EmptyOptions
1863818638

18639-
pub struct vortex_array::scalar_fn::ScalarFn<V: vortex_array::scalar_fn::ScalarFnVTable>(_)
18639+
pub struct vortex_array::scalar_fn::ScalarFn<V: vortex_array::scalar_fn::ScalarFnVTable>
1864018640

1864118641
impl<V: vortex_array::scalar_fn::ScalarFnVTable> vortex_array::scalar_fn::ScalarFn<V>
1864218642

@@ -18684,6 +18684,10 @@ pub fn vortex_array::scalar_fn::ScalarFnRef::as_<V: vortex_array::scalar_fn::Sca
1868418684

1868518685
pub fn vortex_array::scalar_fn::ScalarFnRef::as_opt<V: vortex_array::scalar_fn::ScalarFnVTable>(&self) -> core::option::Option<&<V as vortex_array::scalar_fn::ScalarFnVTable>::Options>
1868618686

18687+
pub fn vortex_array::scalar_fn::ScalarFnRef::downcast<V: vortex_array::scalar_fn::ScalarFnVTable>(self) -> alloc::sync::Arc<vortex_array::scalar_fn::ScalarFn<V>>
18688+
18689+
pub fn vortex_array::scalar_fn::ScalarFnRef::downcast_ref<V: vortex_array::scalar_fn::ScalarFnVTable>(&self) -> core::option::Option<&vortex_array::scalar_fn::ScalarFn<V>>
18690+
1868718691
pub fn vortex_array::scalar_fn::ScalarFnRef::execute(&self, args: &dyn vortex_array::scalar_fn::ExecutionArgs, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult<vortex_array::ArrayRef>
1868818692

1868918693
pub fn vortex_array::scalar_fn::ScalarFnRef::id(&self) -> vortex_array::scalar_fn::ScalarFnId
@@ -18698,9 +18702,9 @@ pub fn vortex_array::scalar_fn::ScalarFnRef::return_dtype(&self, arg_types: &[vo
1869818702

1869918703
pub fn vortex_array::scalar_fn::ScalarFnRef::signature(&self) -> vortex_array::scalar_fn::ScalarFnSignature<'_>
1870018704

18701-
pub fn vortex_array::scalar_fn::ScalarFnRef::validity(&self, expr: &vortex_array::expr::Expression) -> vortex_error::VortexResult<vortex_array::expr::Expression>
18705+
pub fn vortex_array::scalar_fn::ScalarFnRef::try_downcast<V: vortex_array::scalar_fn::ScalarFnVTable>(self) -> core::result::Result<alloc::sync::Arc<vortex_array::scalar_fn::ScalarFn<V>>, vortex_array::scalar_fn::ScalarFnRef>
1870218706

18703-
pub fn vortex_array::scalar_fn::ScalarFnRef::vtable_ref<V: vortex_array::scalar_fn::ScalarFnVTable>(&self) -> core::option::Option<&V>
18707+
pub fn vortex_array::scalar_fn::ScalarFnRef::validity(&self, expr: &vortex_array::expr::Expression) -> vortex_error::VortexResult<vortex_array::expr::Expression>
1870418708

1870518709
impl core::clone::Clone for vortex_array::scalar_fn::ScalarFnRef
1870618710

‎vortex-array/src/arrays/scalar_fn/vtable/mod.rs‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ use std::marker::PhantomData;
1010
use std::ops::Deref;
1111

1212
use itertools::Itertools;
13-
use vortex_error::VortexExpect;
1413
use vortex_error::VortexResult;
1514
use vortex_error::vortex_bail;
1615
use vortex_error::vortex_ensure;
@@ -273,18 +272,11 @@ impl<F: scalar_fn::ScalarFnVTable> Matcher for ExactScalarFn<F> {
273272

274273
fn try_match(array: &dyn DynArray) -> Option<Self::Match<'_>> {
275274
let scalar_fn_array = array.as_opt::<ScalarFnVTable>()?;
276-
let scalar_fn_vtable = scalar_fn_array
277-
.scalar_fn
278-
.vtable_ref::<F>()
279-
.vortex_expect("ScalarFn VTable type mismatch in ExactScalarFn matcher");
280-
let scalar_fn_options = scalar_fn_array
281-
.scalar_fn
282-
.as_opt::<F>()
283-
.vortex_expect("ScalarFn options type mismatch in ExactScalarFn matcher");
275+
let scalar_fn = scalar_fn_array.scalar_fn.downcast_ref::<F>()?;
284276
Some(ScalarFnArrayView {
285277
array,
286-
vtable: scalar_fn_vtable,
287-
options: scalar_fn_options,
278+
vtable: scalar_fn.vtable(),
279+
options: scalar_fn.options(),
288280
})
289281
}
290282
}

‎vortex-array/src/scalar_fn/erased.rs‎

Lines changed: 42 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
//! Type-erased scalar function ([`ScalarFnRef`]).
55
6+
use std::any::type_name;
67
use std::fmt::Debug;
78
use std::fmt::Display;
89
use std::fmt::Formatter;
@@ -12,6 +13,7 @@ use std::sync::Arc;
1213

1314
use vortex_error::VortexExpect;
1415
use vortex_error::VortexResult;
16+
use vortex_error::vortex_err;
1517
use vortex_utils::debug_with::DebugWith;
1618

1719
use crate::ArrayRef;
@@ -34,7 +36,7 @@ use crate::scalar_fn::fns::not::Not;
3436
use crate::scalar_fn::options::ScalarFnOptions;
3537
use crate::scalar_fn::signature::ScalarFnSignature;
3638
use crate::scalar_fn::typed::DynScalarFn;
37-
use crate::scalar_fn::typed::ScalarFnInner;
39+
use crate::scalar_fn::typed::ScalarFn;
3840

3941
/// A type-erased scalar function, pairing a vtable with bound options behind a trait object.
4042
///
@@ -54,22 +56,15 @@ impl ScalarFnRef {
5456

5557
/// Returns whether the scalar function is of the given vtable type.
5658
pub fn is<V: ScalarFnVTable>(&self) -> bool {
57-
self.0.as_any().is::<ScalarFnInner<V>>()
59+
self.0.as_any().is::<ScalarFn<V>>()
5860
}
5961

6062
/// Returns the typed options for this scalar function if it matches the given vtable type.
6163
pub fn as_opt<V: ScalarFnVTable>(&self) -> Option<&V::Options> {
62-
self.downcast_inner::<V>().map(|inner| &inner.options)
63-
}
64-
65-
/// Returns a reference to the typed vtable if it matches the given vtable type.
66-
pub fn vtable_ref<V: ScalarFnVTable>(&self) -> Option<&V> {
67-
self.downcast_inner::<V>().map(|inner| &inner.vtable)
68-
}
69-
70-
/// Downcast the inner to the concrete `ScalarFnInner<V>`.
71-
fn downcast_inner<V: ScalarFnVTable>(&self) -> Option<&ScalarFnInner<V>> {
72-
self.0.as_any().downcast_ref::<ScalarFnInner<V>>()
64+
self.0
65+
.as_any()
66+
.downcast_ref::<ScalarFn<V>>()
67+
.map(|sf| sf.options())
7368
}
7469

7570
/// Returns the typed options for this scalar function if it matches the given vtable type.
@@ -82,6 +77,40 @@ impl ScalarFnRef {
8277
.vortex_expect("Expression options type mismatch")
8378
}
8479

80+
/// Downcast to the concrete [`ScalarFn`].
81+
///
82+
/// Returns `Err(self)` if the downcast fails.
83+
pub fn try_downcast<V: ScalarFnVTable>(self) -> Result<Arc<ScalarFn<V>>, ScalarFnRef> {
84+
if self.0.as_any().is::<ScalarFn<V>>() {
85+
let ptr = Arc::into_raw(self.0) as *const ScalarFn<V>;
86+
Ok(unsafe { Arc::from_raw(ptr) })
87+
} else {
88+
Err(self)
89+
}
90+
}
91+
92+
/// Downcast to the concrete [`ScalarFn`].
93+
///
94+
/// # Panics
95+
///
96+
/// Panics if the downcast fails.
97+
pub fn downcast<V: ScalarFnVTable>(self) -> Arc<ScalarFn<V>> {
98+
self.try_downcast::<V>()
99+
.map_err(|this| {
100+
vortex_err!(
101+
"Failed to downcast ScalarFnRef {} to {}",
102+
this.0.id(),
103+
type_name::<V>(),
104+
)
105+
})
106+
.vortex_expect("Failed to downcast ScalarFnRef")
107+
}
108+
109+
/// Try to downcast into a typed [`ScalarFn`].
110+
pub fn downcast_ref<V: ScalarFnVTable>(&self) -> Option<&ScalarFn<V>> {
111+
self.0.as_any().downcast_ref::<ScalarFn<V>>()
112+
}
113+
85114
/// The type-erased options for this scalar function.
86115
pub fn options(&self) -> ScalarFnOptions<'_> {
87116
ScalarFnOptions { inner: &*self.0 }

‎vortex-array/src/scalar_fn/mod.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,11 @@ pub type ScalarFnId = ArcRef<str>;
3636
/// Private module to seal [`typed::DynScalarFn`].
3737
mod sealed {
3838
use crate::scalar_fn::ScalarFnVTable;
39-
use crate::scalar_fn::typed::ScalarFnInner;
39+
use crate::scalar_fn::typed::ScalarFn;
4040

4141
/// Marker trait to prevent external implementations of [`super::typed::DynScalarFn`].
4242
pub(crate) trait Sealed {}
4343

4444
/// This can be the **only** implementor for [`super::typed::DynScalarFn`].
45-
impl<V: ScalarFnVTable> Sealed for ScalarFnInner<V> {}
45+
impl<V: ScalarFnVTable> Sealed for ScalarFn<V> {}
4646
}

‎vortex-array/src/scalar_fn/typed.rs‎

Lines changed: 37 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
//! Typed and inner representations of scalar functions.
55
//!
66
//! - [`ScalarFn<V>`]: The public typed wrapper, parameterized by a concrete [`ScalarFnVTable`].
7-
//! - [`ScalarFnInner<V>`]: The private inner struct that holds the vtable + options.
7+
//! - [`ScalarFn<V>`]: The private inner struct that holds the vtable + options.
88
//! - [`DynScalarFn`]: The private sealed trait for type-erased dispatch (bound, options in self).
99
1010
use std::any::Any;
@@ -35,9 +35,43 @@ use crate::scalar_fn::ScalarFnRef;
3535
use crate::scalar_fn::ScalarFnVTable;
3636
use crate::scalar_fn::SimplifyCtx;
3737

38+
/// A typed scalar function instance, parameterized by a concrete [`ScalarFnVTable`].
39+
///
40+
/// You can construct one via [`new()`], and erase the type with [`erased()`] to obtain a
41+
/// [`ScalarFnRef`].
42+
///
43+
/// [`new()`]: ScalarFn::new
44+
/// [`erased()`]: ScalarFn::erased
45+
pub struct ScalarFn<V: ScalarFnVTable> {
46+
vtable: V,
47+
options: V::Options,
48+
}
49+
50+
impl<V: ScalarFnVTable> ScalarFn<V> {
51+
/// Create a new typed scalar function instance.
52+
pub fn new(vtable: V, options: V::Options) -> Self {
53+
Self { vtable, options }
54+
}
55+
56+
/// Returns a reference to the vtable.
57+
pub fn vtable(&self) -> &V {
58+
&self.vtable
59+
}
60+
61+
/// Returns a reference to the options.
62+
pub fn options(&self) -> &V::Options {
63+
&self.options
64+
}
65+
66+
/// Erase the concrete type information, returning a type-erased [`ScalarFnRef`].
67+
pub fn erased(self) -> ScalarFnRef {
68+
ScalarFnRef(Arc::new(self))
69+
}
70+
}
71+
3872
/// An object-safe, sealed trait for bound scalar function dispatch.
3973
///
40-
/// Options are stored inside the implementing [`ScalarFnInner<V>`], not passed externally.
74+
/// Options are stored inside the implementing [`ScalarFn<V>`], not passed externally.
4175
/// This is the sole trait behind [`ScalarFnRef`]'s `Arc<dyn DynScalarFn>`.
4276
pub(super) trait DynScalarFn: 'static + Send + Sync + super::sealed::Sealed {
4377
fn as_any(&self) -> &dyn Any;
@@ -86,16 +120,7 @@ pub(super) trait DynScalarFn: 'static + Send + Sync + super::sealed::Sealed {
86120
fn options_debug(&self, f: &mut Formatter<'_>) -> fmt::Result;
87121
}
88122

89-
/// The private inner representation of a bound scalar function, pairing a vtable with its options.
90-
///
91-
/// This is the sole implementor of [`DynScalarFn`], enabling [`ScalarFnRef`] to safely downcast
92-
/// back to the concrete vtable type via [`Any`].
93-
pub(super) struct ScalarFnInner<V: ScalarFnVTable> {
94-
pub(super) vtable: V,
95-
pub(super) options: V::Options,
96-
}
97-
98-
impl<V: ScalarFnVTable> DynScalarFn for ScalarFnInner<V> {
123+
impl<V: ScalarFnVTable> DynScalarFn for ScalarFn<V> {
99124
#[inline(always)]
100125
fn as_any(&self) -> &dyn Any {
101126
self
@@ -232,34 +257,3 @@ impl<V: ScalarFnVTable> DynScalarFn for ScalarFnInner<V> {
232257
Debug::fmt(&self.options, f)
233258
}
234259
}
235-
236-
/// A typed scalar function instance, parameterized by a concrete [`ScalarFnVTable`].
237-
///
238-
/// You can construct one via [`new()`], and erase the type with [`erased()`] to obtain a
239-
/// [`ScalarFnRef`].
240-
///
241-
/// [`new()`]: ScalarFn::new
242-
/// [`erased()`]: ScalarFn::erased
243-
pub struct ScalarFn<V: ScalarFnVTable>(pub(super) Arc<ScalarFnInner<V>>);
244-
245-
impl<V: ScalarFnVTable> ScalarFn<V> {
246-
/// Create a new typed scalar function instance.
247-
pub fn new(vtable: V, options: V::Options) -> Self {
248-
Self(Arc::new(ScalarFnInner { vtable, options }))
249-
}
250-
251-
/// Returns a reference to the vtable.
252-
pub fn vtable(&self) -> &V {
253-
&self.0.vtable
254-
}
255-
256-
/// Returns a reference to the options.
257-
pub fn options(&self) -> &V::Options {
258-
&self.0.options
259-
}
260-
261-
/// Erase the concrete type information, returning a type-erased [`ScalarFnRef`].
262-
pub fn erased(self) -> ScalarFnRef {
263-
ScalarFnRef(self.0)
264-
}
265-
}

0 commit comments

Comments
 (0)