From 63c8d9ff5518b5f935dd47415a607b3200374814 Mon Sep 17 00:00:00 2001 From: Joe Isaacs Date: Mon, 20 Jul 2026 17:05:05 +0000 Subject: [PATCH 1/5] Remove redundant VTable::child overrides Both Patched and PythonVTable overrode VTable::child with behavior identical to the trait default: all four Patched slots are required (always Some), so the default idx-th non-None slot lookup returns exactly slots[idx], and PythonArray has no slots with nchildren() == 0, so its panicking override was unreachable behind the idx < nchildren guard in every caller. Signed-off-by: Claude Signed-off-by: Joe Isaacs Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01XbjnDVZQ9KqXgAAhxy9g7D --- vortex-array/src/arrays/patched/vtable/mod.rs | 10 ---------- vortex-python/src/arrays/py/vtable.rs | 4 ---- 2 files changed, 14 deletions(-) diff --git a/vortex-array/src/arrays/patched/vtable/mod.rs b/vortex-array/src/arrays/patched/vtable/mod.rs index e36d426c0be..71b1775cbf4 100644 --- a/vortex-array/src/arrays/patched/vtable/mod.rs +++ b/vortex-array/src/arrays/patched/vtable/mod.rs @@ -138,16 +138,6 @@ impl VTable for Patched { with_empty_buffers(self, array, buffers) } - fn child(array: ArrayView<'_, Self>, idx: usize) -> ArrayRef { - match idx { - PatchedSlots::INNER => array.inner().clone(), - PatchedSlots::LANE_OFFSETS => array.lane_offsets().clone(), - PatchedSlots::PATCH_INDICES => array.patch_indices().clone(), - PatchedSlots::PATCH_VALUES => array.patch_values().clone(), - _ => vortex_panic!("invalid child index for PatchedArray: {idx}"), - } - } - fn serialize( array: ArrayView<'_, Self>, _session: &VortexSession, diff --git a/vortex-python/src/arrays/py/vtable.rs b/vortex-python/src/arrays/py/vtable.rs index 91eb97d1dff..b8007a1083b 100644 --- a/vortex-python/src/arrays/py/vtable.rs +++ b/vortex-python/src/arrays/py/vtable.rs @@ -96,10 +96,6 @@ impl VTable for PythonVTable { 0 } - fn child(_array: ArrayView<'_, Self>, idx: usize) -> ArrayRef { - vortex_panic!("PythonArray child index {idx} out of bounds") - } - fn child_name(_array: ArrayView<'_, Self>, idx: usize) -> String { vortex_panic!("PythonArray child_name index {idx} out of bounds") } From 8dd71fa4395796cbf7234d24085d525378859ccc Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 21 Jul 2026 09:25:41 +0000 Subject: [PATCH 2/5] Remove VTable::child; derive children from slots at the call sites Every encoding used the default "idx-th non-None slot" definition, so the trait method was not a real customization point. Inline that definition into the DynArrayData children/nth_child/named_children implementations and drop the method from the vtable. Signed-off-by: Claude Signed-off-by: Joe Isaacs Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01XbjnDVZQ9KqXgAAhxy9g7D --- vortex-array/src/array/mod.rs | 21 ++++++++++++++------- vortex-array/src/array/vtable/mod.rs | 15 --------------- 2 files changed, 14 insertions(+), 22 deletions(-) diff --git a/vortex-array/src/array/mod.rs b/vortex-array/src/array/mod.rs index 3eea66c6a68..09cd3d6ed7d 100644 --- a/vortex-array/src/array/mod.rs +++ b/vortex-array/src/array/mod.rs @@ -268,8 +268,8 @@ impl DynArrayData for ArrayData { } fn children(&self, this: &ArrayRef) -> Vec { - let view = unsafe { ArrayView::new_unchecked(this, &self.data) }; - (0..V::nchildren(view)).map(|i| V::child(view, i)).collect() + let view: ArrayView<'_, V> = unsafe { ArrayView::new_unchecked(this, &self.data) }; + view.slots().iter().filter_map(|s| s.clone()).collect() } fn nchildren(&self, this: &ArrayRef) -> usize { @@ -278,8 +278,12 @@ impl DynArrayData for ArrayData { } fn nth_child(&self, this: &ArrayRef, idx: usize) -> Option { - let view = unsafe { ArrayView::new_unchecked(this, &self.data) }; - (idx < V::nchildren(view)).then(|| V::child(view, idx)) + let view: ArrayView<'_, V> = unsafe { ArrayView::new_unchecked(this, &self.data) }; + view.slots() + .iter() + .filter_map(|s| s.as_ref()) + .nth(idx) + .cloned() } fn children_names(&self, this: &ArrayRef) -> Vec { @@ -290,9 +294,12 @@ impl DynArrayData for ArrayData { } fn named_children(&self, this: &ArrayRef) -> Vec<(String, ArrayRef)> { - let view = unsafe { ArrayView::new_unchecked(this, &self.data) }; - (0..V::nchildren(view)) - .map(|i| (V::child_name(view, i), V::child(view, i))) + let view: ArrayView<'_, V> = unsafe { ArrayView::new_unchecked(this, &self.data) }; + view.slots() + .iter() + .filter_map(|s| s.as_ref()) + .enumerate() + .map(|(i, child)| (V::child_name(view, i), child.clone())) .collect() } diff --git a/vortex-array/src/array/vtable/mod.rs b/vortex-array/src/array/vtable/mod.rs index 344aac64a88..e1312cf4e8b 100644 --- a/vortex-array/src/array/vtable/mod.rs +++ b/vortex-array/src/array/vtable/mod.rs @@ -122,21 +122,6 @@ pub trait VTable: 'static + Clone + Sized + Send + Sync + Debug { array.slots().iter().filter(|s| s.is_some()).count() } - /// Returns the child at the given index. - /// - /// The default returns the `idx`-th non-None slot. - /// - /// # Panics - /// Panics if `idx >= nchildren(array)`. - fn child(array: ArrayView<'_, Self>, idx: usize) -> ArrayRef { - array - .slots() - .iter() - .filter_map(|s| s.clone()) - .nth(idx) - .vortex_expect("child index out of bounds") - } - /// Returns the name of the child at the given index. /// /// The default returns the slot name of the `idx`-th non-None slot. From ecb80b517ddb749dee9935863f32a5c61495aa11 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 21 Jul 2026 09:35:10 +0000 Subject: [PATCH 3/5] Remove VTable::nchildren; count non-None slots at the call sites Same treatment as VTable::child: the only override (PythonVTable) matched the default, so the method was not a real customization point. Signed-off-by: Claude Signed-off-by: Joe Isaacs Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01XbjnDVZQ9KqXgAAhxy9g7D --- vortex-array/src/array/mod.rs | 13 ++++++++----- vortex-array/src/array/vtable/mod.rs | 9 +-------- vortex-python/src/arrays/py/vtable.rs | 4 ---- 3 files changed, 9 insertions(+), 17 deletions(-) diff --git a/vortex-array/src/array/mod.rs b/vortex-array/src/array/mod.rs index 09cd3d6ed7d..2cf67d9d7cf 100644 --- a/vortex-array/src/array/mod.rs +++ b/vortex-array/src/array/mod.rs @@ -273,8 +273,8 @@ impl DynArrayData for ArrayData { } fn nchildren(&self, this: &ArrayRef) -> usize { - let view = unsafe { ArrayView::new_unchecked(this, &self.data) }; - V::nchildren(view) + let view: ArrayView<'_, V> = unsafe { ArrayView::new_unchecked(this, &self.data) }; + view.slots().iter().filter(|s| s.is_some()).count() } fn nth_child(&self, this: &ArrayRef, idx: usize) -> Option { @@ -287,9 +287,12 @@ impl DynArrayData for ArrayData { } fn children_names(&self, this: &ArrayRef) -> Vec { - let view = unsafe { ArrayView::new_unchecked(this, &self.data) }; - (0..V::nchildren(view)) - .map(|i| V::child_name(view, i)) + let view: ArrayView<'_, V> = unsafe { ArrayView::new_unchecked(this, &self.data) }; + view.slots() + .iter() + .filter(|s| s.is_some()) + .enumerate() + .map(|(i, _)| V::child_name(view, i)) .collect() } diff --git a/vortex-array/src/array/vtable/mod.rs b/vortex-array/src/array/vtable/mod.rs index e1312cf4e8b..41681041956 100644 --- a/vortex-array/src/array/vtable/mod.rs +++ b/vortex-array/src/array/vtable/mod.rs @@ -115,19 +115,12 @@ pub trait VTable: 'static + Clone + Sized + Send + Sync + Debug { buffers: &[BufferHandle], ) -> VortexResult>; - /// Returns the number of children in the array. - /// - /// The default counts non-None slots. - fn nchildren(array: ArrayView<'_, Self>) -> usize { - array.slots().iter().filter(|s| s.is_some()).count() - } - /// Returns the name of the child at the given index. /// /// The default returns the slot name of the `idx`-th non-None slot. /// /// # Panics - /// Panics if `idx >= nchildren(array)`. + /// Panics if `idx` is not less than the number of non-None slots. fn child_name(array: ArrayView<'_, Self>, idx: usize) -> String { array .slots() diff --git a/vortex-python/src/arrays/py/vtable.rs b/vortex-python/src/arrays/py/vtable.rs index b8007a1083b..23cd42e419c 100644 --- a/vortex-python/src/arrays/py/vtable.rs +++ b/vortex-python/src/arrays/py/vtable.rs @@ -92,10 +92,6 @@ impl VTable for PythonVTable { with_empty_buffers(self, array, buffers) } - fn nchildren(_array: ArrayView<'_, Self>) -> usize { - 0 - } - fn child_name(_array: ArrayView<'_, Self>, idx: usize) -> String { vortex_panic!("PythonArray child_name index {idx} out of bounds") } From 83dbd9d04869bb66bc30cffdc9bbe19cd29088a2 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 21 Jul 2026 09:41:56 +0000 Subject: [PATCH 4/5] Define child traversal once on ArrayRef via children_iter children/nchildren/nth_child/named_children no longer need the encoding vtable: implement them directly on ArrayRef over a single children_iter helper (the non-None slots in order) and drop them from DynArrayData. Only children_names still dispatches, for VTable::child_name. Signed-off-by: Claude Signed-off-by: Joe Isaacs Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01XbjnDVZQ9KqXgAAhxy9g7D --- vortex-array/src/array/erased.rs | 18 +++++++++--- vortex-array/src/array/mod.rs | 50 ++------------------------------ 2 files changed, 16 insertions(+), 52 deletions(-) diff --git a/vortex-array/src/array/erased.rs b/vortex-array/src/array/erased.rs index c3dc2e0eed7..c07eb4c3100 100644 --- a/vortex-array/src/array/erased.rs +++ b/vortex-array/src/array/erased.rs @@ -685,19 +685,26 @@ impl ArrayRef { // ArrayVisitor delegation methods + /// Returns an iterator over the children of the array: its non-None slots in order. + pub fn children_iter(&self) -> impl Iterator { + self.0.slots.iter().filter_map(|s| s.as_ref()) + } + /// Returns the children of the array. pub fn children(&self) -> Vec { - self.0.data.children(self) + self.children_iter().cloned().collect() } /// Returns the number of children of the array. pub fn nchildren(&self) -> usize { - self.0.data.nchildren(self) + self.children_iter().count() } /// Returns the nth child of the array without allocating a Vec. + /// + /// Returns `None` if the index is out of bounds. pub fn nth_child(&self, idx: usize) -> Option { - self.0.data.nth_child(self, idx) + self.children_iter().nth(idx).cloned() } /// Returns the names of the children of the array. @@ -707,7 +714,10 @@ impl ArrayRef { /// Returns the array's children with their names. pub fn named_children(&self) -> Vec<(String, ArrayRef)> { - self.0.data.named_children(self) + self.children_names() + .into_iter() + .zip(self.children_iter().cloned()) + .collect() } /// Returns the data buffers of the array. diff --git a/vortex-array/src/array/mod.rs b/vortex-array/src/array/mod.rs index 2cf67d9d7cf..3dfe3aa5459 100644 --- a/vortex-array/src/array/mod.rs +++ b/vortex-array/src/array/mod.rs @@ -80,23 +80,9 @@ pub(crate) trait DynArrayData: 'static + private::Sealed + Send + Sync + Debug { // --- Visitor methods (formerly in ArrayVisitor) --- - /// Returns the children of the array. - fn children(&self, this: &ArrayRef) -> Vec; - - /// Returns the number of children of the array. - fn nchildren(&self, this: &ArrayRef) -> usize; - - /// Returns the nth child of the array without allocating a Vec. - /// - /// Returns `None` if the index is out of bounds. - fn nth_child(&self, this: &ArrayRef, idx: usize) -> Option; - /// Returns the names of the children of the array. fn children_names(&self, this: &ArrayRef) -> Vec; - /// Returns the array's children with their names. - fn named_children(&self, this: &ArrayRef) -> Vec<(String, ArrayRef)>; - /// Returns the buffers of the array. fn buffers(&self, this: &ArrayRef) -> Vec; @@ -267,42 +253,10 @@ impl DynArrayData for ArrayData { Ok(()) } - fn children(&self, this: &ArrayRef) -> Vec { - let view: ArrayView<'_, V> = unsafe { ArrayView::new_unchecked(this, &self.data) }; - view.slots().iter().filter_map(|s| s.clone()).collect() - } - - fn nchildren(&self, this: &ArrayRef) -> usize { - let view: ArrayView<'_, V> = unsafe { ArrayView::new_unchecked(this, &self.data) }; - view.slots().iter().filter(|s| s.is_some()).count() - } - - fn nth_child(&self, this: &ArrayRef, idx: usize) -> Option { - let view: ArrayView<'_, V> = unsafe { ArrayView::new_unchecked(this, &self.data) }; - view.slots() - .iter() - .filter_map(|s| s.as_ref()) - .nth(idx) - .cloned() - } - fn children_names(&self, this: &ArrayRef) -> Vec { let view: ArrayView<'_, V> = unsafe { ArrayView::new_unchecked(this, &self.data) }; - view.slots() - .iter() - .filter(|s| s.is_some()) - .enumerate() - .map(|(i, _)| V::child_name(view, i)) - .collect() - } - - fn named_children(&self, this: &ArrayRef) -> Vec<(String, ArrayRef)> { - let view: ArrayView<'_, V> = unsafe { ArrayView::new_unchecked(this, &self.data) }; - view.slots() - .iter() - .filter_map(|s| s.as_ref()) - .enumerate() - .map(|(i, child)| (V::child_name(view, i), child.clone())) + (0..this.nchildren()) + .map(|i| V::child_name(view, i)) .collect() } From 841127d9034b9bbbeec94c43fae0ae60005942b2 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 21 Jul 2026 09:54:27 +0000 Subject: [PATCH 5/5] Remove VTable::child_name; derive children_names from slot_name The default child_name was just slot_name of the idx-th non-None slot, and the only override (PythonVTable) was unreachable. children_names now lives on ArrayRef next to the other child accessors, leaving slot_name as the single vtable source of child names. named_children stays as the public zip of children_names and children_iter for its external users. Signed-off-by: Claude Signed-off-by: Joe Isaacs Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01XbjnDVZQ9KqXgAAhxy9g7D --- vortex-array/src/array/erased.rs | 11 +++++++++-- vortex-array/src/array/mod.rs | 10 ---------- vortex-array/src/array/vtable/mod.rs | 17 ----------------- vortex-python/src/arrays/py/vtable.rs | 4 ---- 4 files changed, 9 insertions(+), 33 deletions(-) diff --git a/vortex-array/src/array/erased.rs b/vortex-array/src/array/erased.rs index c07eb4c3100..caa56ccbb40 100644 --- a/vortex-array/src/array/erased.rs +++ b/vortex-array/src/array/erased.rs @@ -707,9 +707,16 @@ impl ArrayRef { self.children_iter().nth(idx).cloned() } - /// Returns the names of the children of the array. + /// Returns the names of the children of the array: the slot names of the non-None slots + /// in order. pub fn children_names(&self) -> Vec { - self.0.data.children_names(self) + self.0 + .slots + .iter() + .enumerate() + .filter(|(_, s)| s.is_some()) + .map(|(slot_idx, _)| self.slot_name(slot_idx)) + .collect() } /// Returns the array's children with their names. diff --git a/vortex-array/src/array/mod.rs b/vortex-array/src/array/mod.rs index 3dfe3aa5459..e34accebfb2 100644 --- a/vortex-array/src/array/mod.rs +++ b/vortex-array/src/array/mod.rs @@ -80,9 +80,6 @@ pub(crate) trait DynArrayData: 'static + private::Sealed + Send + Sync + Debug { // --- Visitor methods (formerly in ArrayVisitor) --- - /// Returns the names of the children of the array. - fn children_names(&self, this: &ArrayRef) -> Vec; - /// Returns the buffers of the array. fn buffers(&self, this: &ArrayRef) -> Vec; @@ -253,13 +250,6 @@ impl DynArrayData for ArrayData { Ok(()) } - fn children_names(&self, this: &ArrayRef) -> Vec { - let view: ArrayView<'_, V> = unsafe { ArrayView::new_unchecked(this, &self.data) }; - (0..this.nchildren()) - .map(|i| V::child_name(view, i)) - .collect() - } - fn buffers(&self, this: &ArrayRef) -> Vec { let view = unsafe { ArrayView::new_unchecked(this, &self.data) }; (0..V::nbuffers(view)) diff --git a/vortex-array/src/array/vtable/mod.rs b/vortex-array/src/array/vtable/mod.rs index 41681041956..3078a6ae9f5 100644 --- a/vortex-array/src/array/vtable/mod.rs +++ b/vortex-array/src/array/vtable/mod.rs @@ -115,23 +115,6 @@ pub trait VTable: 'static + Clone + Sized + Send + Sync + Debug { buffers: &[BufferHandle], ) -> VortexResult>; - /// Returns the name of the child at the given index. - /// - /// The default returns the slot name of the `idx`-th non-None slot. - /// - /// # Panics - /// Panics if `idx` is not less than the number of non-None slots. - fn child_name(array: ArrayView<'_, Self>, idx: usize) -> String { - array - .slots() - .iter() - .enumerate() - .filter(|(_, s)| s.is_some()) - .nth(idx) - .map(|(slot_idx, _)| Self::slot_name(array, slot_idx)) - .vortex_expect("child_name index out of bounds") - } - /// Serialize encoding metadata into a byte buffer for IPC or file storage. /// /// Return `None` if the array cannot be serialized by this encoding. Buffers and children are diff --git a/vortex-python/src/arrays/py/vtable.rs b/vortex-python/src/arrays/py/vtable.rs index 23cd42e419c..5ec749d3a5f 100644 --- a/vortex-python/src/arrays/py/vtable.rs +++ b/vortex-python/src/arrays/py/vtable.rs @@ -92,10 +92,6 @@ impl VTable for PythonVTable { with_empty_buffers(self, array, buffers) } - fn child_name(_array: ArrayView<'_, Self>, idx: usize) -> String { - vortex_panic!("PythonArray child_name index {idx} out of bounds") - } - fn serialize( _array: ArrayView<'_, Self>, _session: &VortexSession,