Skip to content

Commit 38e4019

Browse files
committed
fix
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
1 parent f96f160 commit 38e4019

5 files changed

Lines changed: 26 additions & 46 deletions

File tree

encodings/bytebool/src/array.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,7 @@ impl VTable for ByteBool {
8080
fn array_eq(array: &ByteBoolArray, other: &ByteBoolArray, precision: Precision) -> bool {
8181
array.dtype == other.dtype
8282
&& array.buffer.array_eq(&other.buffer, precision)
83-
&& array
84-
._validity()
85-
.array_eq(&other._validity(), precision)
83+
&& array._validity().array_eq(&other._validity(), precision)
8684
}
8785

8886
fn nbuffers(_array: &ByteBoolArray) -> usize {

encodings/bytebool/src/compute.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ impl CastReduce for ByteBool {
2525

2626
// If just changing nullability, we can optimize
2727
if array.dtype().eq_ignore_nullability(dtype) {
28-
let new_validity = array._validity()
28+
let new_validity = array
29+
._validity()
2930
.cast_nullability(dtype.nullability(), array.len())?;
3031

3132
return Ok(Some(
@@ -43,8 +44,7 @@ impl MaskReduce for ByteBool {
4344
Ok(Some(
4445
ByteBoolArray::new(
4546
array.buffer().clone(),
46-
array._validity()
47-
.and(Validity::Array(mask.clone()))?,
47+
array._validity().and(Validity::Array(mask.clone()))?,
4848
)
4949
.into_array(),
5050
))
@@ -61,8 +61,7 @@ impl TakeExecute for ByteBool {
6161
let bools = array.as_slice();
6262

6363
// This handles combining validity from both source array and nullable indices
64-
let validity = array._validity()
65-
.take(&indices.clone().into_array())?;
64+
let validity = array._validity().take(&indices.clone().into_array())?;
6665

6766
let taken_bools = match_each_integer_ptype!(indices.ptype(), |I| {
6867
indices

vortex-array/public-api.lock

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22496,10 +22496,14 @@ pub fn vortex_array::vtable::DynVTable::execute(&self, array: vortex_array::Arra
2249622496

2249722497
pub fn vortex_array::vtable::DynVTable::execute_parent(&self, array: &vortex_array::ArrayRef, parent: &vortex_array::ArrayRef, child_idx: usize, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
2249822498

22499+
pub fn vortex_array::vtable::DynVTable::put_slot(&self, array: &mut vortex_array::ArrayRef, slot_idx: usize, value: vortex_array::ArrayRef)
22500+
2249922501
pub fn vortex_array::vtable::DynVTable::reduce(&self, array: &vortex_array::ArrayRef) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
2250022502

2250122503
pub fn vortex_array::vtable::DynVTable::reduce_parent(&self, array: &vortex_array::ArrayRef, parent: &vortex_array::ArrayRef, child_idx: usize) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
2250222504

22505+
pub fn vortex_array::vtable::DynVTable::take_slot(&self, array: &mut vortex_array::ArrayRef, slot_idx: usize) -> core::option::Option<vortex_array::ArrayRef>
22506+
2250322507
pub fn vortex_array::vtable::DynVTable::with_slots_mut(&self, array: vortex_array::ArrayRef, f: &mut dyn core::ops::function::FnMut(&mut [core::option::Option<vortex_array::ArrayRef>])) -> vortex_error::VortexResult<vortex_array::ArrayRef>
2250422508

2250522509
impl<V: vortex_array::vtable::VTable> vortex_array::vtable::DynVTable for V
@@ -22512,10 +22516,14 @@ pub fn V::execute(&self, array: vortex_array::ArrayRef, ctx: &mut vortex_array::
2251222516

2251322517
pub fn V::execute_parent(&self, array: &vortex_array::ArrayRef, parent: &vortex_array::ArrayRef, child_idx: usize, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
2251422518

22519+
pub fn V::put_slot(&self, array: &mut vortex_array::ArrayRef, slot_idx: usize, value: vortex_array::ArrayRef)
22520+
2251522521
pub fn V::reduce(&self, array: &vortex_array::ArrayRef) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
2251622522

2251722523
pub fn V::reduce_parent(&self, array: &vortex_array::ArrayRef, parent: &vortex_array::ArrayRef, child_idx: usize) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
2251822524

22525+
pub fn V::take_slot(&self, array: &mut vortex_array::ArrayRef, slot_idx: usize) -> core::option::Option<vortex_array::ArrayRef>
22526+
2251922527
pub fn V::with_slots_mut(&self, array: vortex_array::ArrayRef, f: &mut dyn core::ops::function::FnMut(&mut [core::option::Option<vortex_array::ArrayRef>])) -> vortex_error::VortexResult<vortex_array::ArrayRef>
2252022528

2252122529
pub trait vortex_array::vtable::OperationsVTable<V: vortex_array::vtable::VTable>

vortex-array/src/arrays/patched/vtable/mod.rs

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -655,15 +655,13 @@ mod tests {
655655
let indices = array.patch_indices().clone();
656656
let values = array.patch_values().clone();
657657

658-
// Create new PatchedArray with same children using with_slots_mut
659-
let array_ref = array.clone().into_array();
660-
let vtable = array_ref.vtable().clone_boxed();
661-
let new_array = vtable.with_slots_mut(array_ref, &mut |slots| {
662-
slots[0] = Some(inner.clone());
663-
slots[1] = Some(lane_offsets.clone());
664-
slots[2] = Some(indices.clone());
665-
slots[3] = Some(values.clone());
666-
})?;
658+
// Create new PatchedArray with same children using put_slot
659+
let mut new_array = array.clone().into_array();
660+
let vtable = new_array.vtable().clone_boxed();
661+
vtable.put_slot(&mut new_array, 0, inner.clone());
662+
vtable.put_slot(&mut new_array, 1, lane_offsets.clone());
663+
vtable.put_slot(&mut new_array, 2, indices.clone());
664+
vtable.put_slot(&mut new_array, 3, values.clone());
667665

668666
assert!(new_array.is::<Patched>());
669667
assert_eq!(array.len(), new_array.len());
@@ -692,14 +690,12 @@ mod tests {
692690
let indices = array.patch_indices().clone();
693691
let values = array.patch_values().clone();
694692

695-
let array_ref = array.into_array();
696-
let vtable = array_ref.vtable().clone_boxed();
697-
let new_array = vtable.with_slots_mut(array_ref, &mut |slots| {
698-
slots[0] = Some(new_inner.clone());
699-
slots[1] = Some(lane_offsets.clone());
700-
slots[2] = Some(indices.clone());
701-
slots[3] = Some(values.clone());
702-
})?;
693+
let mut new_array = array.into_array();
694+
let vtable = new_array.vtable().clone_boxed();
695+
vtable.put_slot(&mut new_array, 0, new_inner.clone());
696+
vtable.put_slot(&mut new_array, 1, lane_offsets.clone());
697+
vtable.put_slot(&mut new_array, 2, indices.clone());
698+
vtable.put_slot(&mut new_array, 3, values.clone());
703699

704700
// Execute and verify the inner values changed (except at patch positions)
705701
let mut ctx = ExecutionCtx::new(VortexSession::empty());

vortex-array/src/vtable/dyn_.rs

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -48,16 +48,6 @@ pub trait DynVTable: 'static + Send + Sync + Debug {
4848
children: &dyn ArrayChildren,
4949
session: &VortexSession,
5050
) -> VortexResult<ArrayRef>;
51-
/// Unwrap the Arc, get mutable access to slots, apply a mutation, and re-wrap.
52-
///
53-
/// The callback receives `&mut [Option<ArrayRef>]` so callers can modify individual
54-
/// slots in-place without cloning all children.
55-
fn with_slots_mut(
56-
&self,
57-
array: ArrayRef,
58-
f: &mut dyn FnMut(&mut [Option<ArrayRef>]),
59-
) -> VortexResult<ArrayRef>;
60-
6151
/// Remove a child from a slot, returning it.
6252
///
6353
/// With unique ownership (Arc refcount == 1), the slot is set to `None` in-place.
@@ -136,17 +126,6 @@ impl<V: VTable> DynVTable for V {
136126
Ok(array.into_array())
137127
}
138128

139-
fn with_slots_mut(
140-
&self,
141-
array: ArrayRef,
142-
f: &mut dyn FnMut(&mut [Option<ArrayRef>]),
143-
) -> VortexResult<ArrayRef> {
144-
let arc = downcast_owned::<V>(array);
145-
let mut inner = Arc::try_unwrap(arc).unwrap_or_else(|arc| arc.as_ref().clone());
146-
f(V::slots_mut(&mut inner.array));
147-
Ok(inner.into_array())
148-
}
149-
150129
fn take_slot(&self, array: &mut ArrayRef, slot_idx: usize) -> Option<ArrayRef> {
151130
// If we have unique ownership, take in-place. Otherwise, clone the child.
152131
if let Some(inner) = downcast_mut::<V>(array) {

0 commit comments

Comments
 (0)