Skip to content

Commit 64d9161

Browse files
authored
Fix weird signature of with_slots functions (#7758)
with_slots doesn't need to pass the original ref to the arrayinner methods as unique ownership arc is handled earlier Signed-off-by: Robert Kruszewski <github@robertk.io>
1 parent 1c7b179 commit 64d9161

2 files changed

Lines changed: 17 additions & 21 deletions

File tree

vortex-array/src/array/erased.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,7 @@ impl ArrayRef {
472472

473473
// SAFETY: ensured by the caller — the None slot is either put back or driven to completion
474474
// via the builder path before the parent escapes the executor.
475-
let new_parent = unsafe { self.0.with_slots_unchecked(&self, new_slots) };
475+
let new_parent = unsafe { self.0.with_slots_unchecked(new_slots) };
476476
Ok((new_parent, child))
477477
}
478478

@@ -497,7 +497,7 @@ impl ArrayRef {
497497
let mut slots = self.slots().to_vec();
498498
slots[slot_idx] = Some(replacement);
499499
let inner = Arc::clone(&self.0);
500-
inner.with_slots(self, slots)
500+
inner.with_slots(slots)
501501
}
502502

503503
/// Returns a new array with the provided slots.
@@ -536,7 +536,7 @@ impl ArrayRef {
536536
}
537537
}
538538
let inner = Arc::clone(&self.0);
539-
inner.with_slots(self, slots)
539+
inner.with_slots(slots)
540540
}
541541

542542
pub fn reduce(&self) -> VortexResult<Option<ArrayRef>> {

vortex-array/src/array/mod.rs

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ pub(crate) trait DynArray: 'static + private::Sealed + Send + Sync + Debug {
144144
fn dyn_array_eq(&self, other: &ArrayRef, precision: crate::Precision) -> bool;
145145

146146
/// Returns a new array with the given slots.
147-
fn with_slots(&self, this: ArrayRef, slots: Vec<Option<ArrayRef>>) -> VortexResult<ArrayRef>;
147+
fn with_slots(&self, slots: Vec<Option<ArrayRef>>) -> VortexResult<ArrayRef>;
148148

149149
/// Returns a new array with the given slots, bypassing encoding-level validation.
150150
///
@@ -158,11 +158,7 @@ pub(crate) trait DynArray: 'static + private::Sealed + Send + Sync + Debug {
158158
/// The array returned may have slots whose content does not match the encoding's normal
159159
/// invariants. Callers must re-establish those invariants before handing the array to
160160
/// anything outside the executor.
161-
unsafe fn with_slots_unchecked(
162-
&self,
163-
this: &ArrayRef,
164-
slots: Vec<Option<ArrayRef>>,
165-
) -> ArrayRef;
161+
unsafe fn with_slots_unchecked(&self, slots: Vec<Option<ArrayRef>>) -> ArrayRef;
166162

167163
/// Attempt to reduce the array to a simpler representation.
168164
fn reduce(&self, this: &ArrayRef) -> VortexResult<Option<ArrayRef>>;
@@ -423,28 +419,28 @@ impl<V: VTable> DynArray for ArrayInner<V> {
423419
})
424420
}
425421

426-
fn with_slots(&self, this: ArrayRef, slots: Vec<Option<ArrayRef>>) -> VortexResult<ArrayRef> {
427-
let data = self.data.clone();
428-
let stats = this.statistics().to_owned();
422+
fn with_slots(&self, slots: Vec<Option<ArrayRef>>) -> VortexResult<ArrayRef> {
423+
let stats = self.stats.clone();
429424
Ok(Array::<V>::try_from_parts(
430-
ArrayParts::new(self.vtable.clone(), this.dtype().clone(), this.len(), data)
431-
.with_slots(slots),
425+
ArrayParts::new(
426+
self.vtable.clone(),
427+
self.dtype.clone(),
428+
self.len,
429+
self.data.clone(),
430+
)
431+
.with_slots(slots),
432432
)?
433-
.with_stats_set(stats)
433+
.with_stats_set(stats.into())
434434
.into_array())
435435
}
436436

437-
unsafe fn with_slots_unchecked(
438-
&self,
439-
this: &ArrayRef,
440-
slots: Vec<Option<ArrayRef>>,
441-
) -> ArrayRef {
437+
unsafe fn with_slots_unchecked(&self, slots: Vec<Option<ArrayRef>>) -> ArrayRef {
442438
// SAFETY: we intentionally skip `V::validate` here. Caller guarantees that the resulting
443439
// array is either repaired or not externally observed.
444440
let inner = unsafe {
445441
ArrayInner::<V>::from_data_unchecked(
446442
self.vtable.clone(),
447-
this.dtype().clone(),
443+
self.dtype.clone(),
448444
self.len,
449445
self.data.clone(),
450446
slots,

0 commit comments

Comments
 (0)