Skip to content

fix[sparse]: ensure cached patches are updated and use PatchesData #7811

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
joseph-isaacs merged 4 commits into from
May 7, 2026
Merged

fix[sparse]: ensure cached patches are updated and use PatchesData #7811

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0cdd587
fix[patches]: ensure cached patches are updated and use `PatchesData`
joseph-isaacs May 6, 2026
1ce40fa
fix[patches]: ensure cached patches are updated and use `PatchesData`
joseph-isaacs May 6, 2026
49a88d3
fix[patches]: ensure cached patches are updated and use `PatchesData`
joseph-isaacs May 6, 2026
edd2c4a
fix[patches]: ensure cached patches are updated and use `PatchesData`
joseph-isaacs May 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 25 additions & 71 deletions encodings/alp/src/alp/array.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ use vortex_array::arrays::Primitive;
use vortex_array::buffer::BufferHandle;
use vortex_array::dtype::DType;
use vortex_array::dtype::PType;
use vortex_array::patches::PatchSlotIndices;
use vortex_array::patches::Patches;
use vortex_array::patches::PatchesData;
use vortex_array::patches::PatchesMetadata;
use vortex_array::require_child;
use vortex_array::require_patches;
Expand Down Expand Up @@ -53,16 +55,13 @@ pub type ALPArray = Array<ALP>;
impl ArrayHash for ALPData {
fn array_hash<H: Hasher>(&self, state: &mut H, _precision: Precision) {
self.exponents.hash(state);
self.patch_offset.hash(state);
self.patch_offset_within_chunk.hash(state);
self.patches_data.hash(state);
}
}

impl ArrayEq for ALPData {
fn array_eq(&self, other: &Self, _precision: Precision) -> bool {
self.exponents == other.exponents
&& self.patch_offset == other.patch_offset
&& self.patch_offset_within_chunk == other.patch_offset_within_chunk
self.exponents == other.exponents && self.patches_data == other.patches_data
}
}

Expand All @@ -84,19 +83,10 @@ impl VTable for ALP {
len: usize,
slots: &[Option<ArrayRef>],
) -> VortexResult<()> {
let slots = ALPSlotsView::from_slots(slots);
validate_parts(
dtype,
len,
data.exponents,
slots.encoded,
patches_from_slots(
&slots,
data.patch_offset,
data.patch_offset_within_chunk,
len,
),
)
let alp_slots = ALPSlotsView::from_slots(slots);
let patches =
PatchesData::patches_from_slots(data.patches_data.as_ref(), len, slots, PATCH_SLOTS);
validate_parts(dtype, len, data.exponents, alp_slots.encoded, patches)
}

fn nbuffers(_array: ArrayView<'_, Self>) -> usize {
Expand Down Expand Up @@ -219,18 +209,23 @@ pub struct ALPSlots {
pub patch_chunk_offsets: Option<ArrayRef>,
}

const PATCH_SLOTS: PatchSlotIndices = PatchSlotIndices {
indices: ALPSlots::PATCH_INDICES,
values: ALPSlots::PATCH_VALUES,
chunk_offsets: ALPSlots::PATCH_CHUNK_OFFSETS,
};

#[derive(Clone, Debug)]
pub struct ALPData {
patch_offset: Option<usize>,
patch_offset_within_chunk: Option<usize>,
patches_data: Option<PatchesData>,
exponents: Exponents,
}

impl Display for ALPData {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
write!(f, "exponents: {}", self.exponents)?;
if let Some(offset) = self.patch_offset {
write!(f, ", patch_offset: {offset}")?;
if let Some(pd) = &self.patches_data {
write!(f, ", patch_offset: {}", pd.offset())?;
}
Ok(())
}
Expand Down Expand Up @@ -339,14 +334,8 @@ impl ALPData {
/// See [`ALP::try_new`] for reference on preconditions that must pass before
/// calling this method.
pub fn new(exponents: Exponents, patches: Option<Patches>) -> Self {
let (patch_offset, patch_offset_within_chunk) = match &patches {
Some(p) => (Some(p.offset()), p.offset_within_chunk()),
None => (None, None),
};

Self {
patch_offset,
patch_offset_within_chunk,
patches_data: patches.as_ref().map(PatchesData::from_patches),
exponents,
}
}
Expand Down Expand Up @@ -411,20 +400,9 @@ impl ALP {

impl ALPData {
fn make_slots(encoded: &ArrayRef, patches: Option<&Patches>) -> Vec<Option<ArrayRef>> {
let (patch_indices, patch_values, patch_chunk_offsets) = match patches {
Some(p) => (
Some(p.indices().clone()),
Some(p.values().clone()),
p.chunk_offsets().clone(),
),
None => (None, None, None),
};
vec![
Some(encoded.clone()),
patch_indices,
patch_values,
patch_chunk_offsets,
]
let mut slots = vec![Some(encoded.clone())];
PatchesData::push_slots(&mut slots, patches);
slots
}

#[inline]
Expand All @@ -439,39 +417,15 @@ pub trait ALPArrayExt: ALPArraySlotsExt {
}

fn patches(&self) -> Option<Patches> {
patches_from_slots(
&self.slots_view(),
self.patch_offset,
self.patch_offset_within_chunk,
PatchesData::patches_from_slots(
self.patches_data.as_ref(),
self.as_ref().len(),
self.as_ref().slots(),
PATCH_SLOTS,
)
}
}

fn patches_from_slots(
slots: &ALPSlotsView,
patch_offset: Option<usize>,
patch_offset_within_chunk: Option<usize>,
len: usize,
) -> Option<Patches> {
match (slots.patch_indices, slots.patch_values) {
(Some(indices), Some(values)) => {
let patch_offset = patch_offset.vortex_expect("has patch slots but no patch_offset");
Some(unsafe {
Patches::new_unchecked(
len,
patch_offset,
indices.clone(),
values.clone(),
slots.patch_chunk_offsets.cloned(),
patch_offset_within_chunk,
)
})
}
_ => None,
}
}

fn validate_parts(
dtype: &DType,
len: usize,
Expand Down
77 changes: 20 additions & 57 deletions encodings/alp/src/alp_rd/array.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ use vortex_array::buffer::BufferHandle;
use vortex_array::dtype::DType;
use vortex_array::dtype::Nullability;
use vortex_array::dtype::PType;
use vortex_array::patches::PatchSlotIndices;
use vortex_array::patches::Patches;
use vortex_array::patches::PatchesData;
use vortex_array::patches::PatchesMetadata;
use vortex_array::require_child;
use vortex_array::require_patches;
Expand Down Expand Up @@ -74,8 +76,7 @@ impl ArrayHash for ALPRDData {
fn array_hash<H: Hasher>(&self, state: &mut H, precision: Precision) {
self.left_parts_dictionary.array_hash(state, precision);
self.right_bit_width.hash(state);
self.patch_offset.hash(state);
self.patch_offset_within_chunk.hash(state);
self.patches_data.hash(state);
}
}

Expand All @@ -84,8 +85,7 @@ impl ArrayEq for ALPRDData {
self.left_parts_dictionary
.array_eq(&other.left_parts_dictionary, precision)
&& self.right_bit_width == other.right_bit_width
&& self.patch_offset == other.patch_offset
&& self.patch_offset_within_chunk == other.patch_offset_within_chunk
&& self.patches_data == other.patches_data
}
}

Expand All @@ -112,13 +112,7 @@ impl VTable for ALPRD {
len,
left_parts_from_slots(slots),
right_parts_from_slots(slots),
patches_from_slots(
slots,
data.patch_offset,
data.patch_offset_within_chunk,
len,
)
.as_ref(),
patches_from_slots(slots, data.patches_data.as_ref(), len).as_ref(),
)
}

Expand Down Expand Up @@ -327,6 +321,11 @@ pub(super) const LEFT_PARTS_SLOT: usize = 0;
/// The right (least significant) parts of the real-double encoded values.
pub(super) const RIGHT_PARTS_SLOT: usize = 1;
/// The indices of left-parts exception values that could not be dictionary-encoded.
pub(super) const LP_PATCH_SLOTS: PatchSlotIndices = PatchSlotIndices {
indices: LP_PATCH_INDICES_SLOT,
values: LP_PATCH_VALUES_SLOT,
chunk_offsets: LP_PATCH_CHUNK_OFFSETS_SLOT,
};
pub(super) const LP_PATCH_INDICES_SLOT: usize = 2;
/// The exception values for left-parts that could not be dictionary-encoded.
pub(super) const LP_PATCH_VALUES_SLOT: usize = 3;
Expand All @@ -343,17 +342,16 @@ pub(super) const SLOT_NAMES: [&str; NUM_SLOTS] = [

#[derive(Clone, Debug)]
pub struct ALPRDData {
patch_offset: Option<usize>,
patch_offset_within_chunk: Option<usize>,
patches_data: Option<PatchesData>,
left_parts_dictionary: Buffer<u16>,
right_bit_width: u8,
}

impl Display for ALPRDData {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
write!(f, "right_bit_width: {}", self.right_bit_width)?;
if let Some(offset) = self.patch_offset {
write!(f, ", patch_offset: {offset}")?;
if let Some(pd) = &self.patches_data {
write!(f, ", patch_offset: {}", pd.offset())?;
}
Ok(())
}
Expand Down Expand Up @@ -438,14 +436,8 @@ impl ALPRDData {
right_bit_width: u8,
left_parts_patches: Option<Patches>,
) -> Self {
let (patch_offset, patch_offset_within_chunk) = match &left_parts_patches {
Some(patches) => (Some(patches.offset()), patches.offset_within_chunk()),
None => (None, None),
};

Self {
patch_offset,
patch_offset_within_chunk,
patches_data: left_parts_patches.as_ref().map(PatchesData::from_patches),
left_parts_dictionary,
right_bit_width,
}
Expand All @@ -466,21 +458,9 @@ impl ALPRDData {
right_parts: &ArrayRef,
patches: Option<&Patches>,
) -> Vec<Option<ArrayRef>> {
let (pi, pv, pco) = match patches {
Some(p) => (
Some(p.indices().clone()),
Some(p.values().clone()),
p.chunk_offsets().clone(),
),
None => (None, None, None),
};
vec![
Some(left_parts.clone()),
Some(right_parts.clone()),
pi,
pv,
pco,
]
let mut slots = vec![Some(left_parts.clone()), Some(right_parts.clone())];
PatchesData::push_slots(&mut slots, patches);
slots
}

/// Return all the owned parts of the array
Expand Down Expand Up @@ -519,26 +499,10 @@ fn right_parts_from_slots(slots: &[Option<ArrayRef>]) -> &ArrayRef {

fn patches_from_slots(
slots: &[Option<ArrayRef>],
patch_offset: Option<usize>,
patch_offset_within_chunk: Option<usize>,
patches_data: Option<&PatchesData>,
len: usize,
) -> Option<Patches> {
match (&slots[LP_PATCH_INDICES_SLOT], &slots[LP_PATCH_VALUES_SLOT]) {
(Some(indices), Some(values)) => {
let patch_offset = patch_offset.vortex_expect("ALPRDArray patch slots without offset");
Some(unsafe {
Patches::new_unchecked(
len,
patch_offset,
indices.clone(),
values.clone(),
slots[LP_PATCH_CHUNK_OFFSETS_SLOT].clone(),
patch_offset_within_chunk,
)
})
}
_ => None,
}
PatchesData::patches_from_slots(patches_data, len, slots, LP_PATCH_SLOTS)
}

fn validate_parts(
Expand Down Expand Up @@ -626,8 +590,7 @@ pub trait ALPRDArrayExt: TypedArrayRef<ALPRD> {
fn left_parts_patches(&self) -> Option<Patches> {
patches_from_slots(
self.as_ref().slots(),
self.patch_offset,
self.patch_offset_within_chunk,
self.patches_data.as_ref(),
self.as_ref().len(),
)
}
Expand Down
Loading
Loading