Skip to content

fallible scalar_at and validity #6102

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 8 commits into from
Jan 22, 2026
Merged

fallible scalar_at and validity #6102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
728d243
wip more fallible
joseph-isaacs Jan 22, 2026
9b92a0a
wip more fallible
joseph-isaacs Jan 22, 2026
a756d46
wip more fallible
joseph-isaacs Jan 22, 2026
39a8a89
wip more fallible
joseph-isaacs Jan 22, 2026
c7fb120
wip more fallible
joseph-isaacs Jan 22, 2026
687c01a
fix
joseph-isaacs Jan 22, 2026
d4117ca
fix
joseph-isaacs Jan 22, 2026
d197298
fix
joseph-isaacs Jan 22, 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
40 changes: 17 additions & 23 deletions encodings/alp/src/alp/array.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ use vortex_array::vtable::ValidityVTableFromChild;
use vortex_array::vtable::VisitorVTable;
use vortex_dtype::DType;
use vortex_dtype::PType;
use vortex_error::VortexError;
use vortex_error::VortexExpect;
use vortex_error::VortexResult;
use vortex_error::vortex_bail;
Expand Down Expand Up @@ -101,20 +100,14 @@ impl VTable for ALPVTable {
let patches = metadata
.patches
.map(|p| {
let indices = children.get(1, &p.indices_dtype(), p.len())?;
let values = children.get(2, dtype, p.len())?;
let indices = children.get(1, &p.indices_dtype()?, p.len()?)?;
let values = children.get(2, dtype, p.len()?)?;
let chunk_offsets = p
.chunk_offsets_dtype()
.chunk_offsets_dtype()?
.map(|dtype| children.get(3, &dtype, usize::try_from(p.chunk_offsets_len())?))
.transpose()?;

Ok::<_, VortexError>(Patches::new(
len,
p.offset(),
indices,
values,
chunk_offsets,
))
Patches::new(len, p.offset()?, indices, values, chunk_offsets)
})
.transpose()?;

Expand Down Expand Up @@ -167,7 +160,7 @@ impl VTable for ALPVTable {
indices,
values,
chunk_offsets,
));
)?);
}

Ok(())
Expand All @@ -184,7 +177,7 @@ impl VTable for ALPVTable {
fn slice(array: &Self::Array, range: Range<usize>) -> VortexResult<Option<ArrayRef>> {
Ok(Some(
ALPArray::new(
array.encoded().slice(range.clone()),
array.encoded().slice(range.clone())?,
array.exponents(),
array
.patches()
Expand Down Expand Up @@ -506,7 +499,7 @@ mod tests {
encoded.to_array().execute::<Canonical>(&mut ctx).unwrap()
};
// Compare against the traditional array-based decompress path
let expected = decompress_into_array(encoded);
let expected = decompress_into_array(encoded).unwrap();

assert_arrays_eq!(result_canonical.into_array(), expected);
}
Expand All @@ -530,7 +523,7 @@ mod tests {
encoded.to_array().execute::<Canonical>(&mut ctx).unwrap()
};
// Compare against the traditional array-based decompress path
let expected = decompress_into_array(encoded);
let expected = decompress_into_array(encoded).unwrap();

assert_arrays_eq!(result_canonical.into_array(), expected);
}
Expand Down Expand Up @@ -560,7 +553,7 @@ mod tests {
encoded.to_array().execute::<Canonical>(&mut ctx).unwrap()
};
// Compare against the traditional array-based decompress path
let expected = decompress_into_array(encoded);
let expected = decompress_into_array(encoded).unwrap();

assert_arrays_eq!(result_canonical.into_array(), expected);
}
Expand Down Expand Up @@ -588,7 +581,7 @@ mod tests {
encoded.to_array().execute::<Canonical>(&mut ctx).unwrap()
};
// Compare against the traditional array-based decompress path
let expected = decompress_into_array(encoded);
let expected = decompress_into_array(encoded).unwrap();

assert_arrays_eq!(result_canonical.into_array(), expected);
}
Expand Down Expand Up @@ -619,7 +612,7 @@ mod tests {
encoded.to_array().execute::<Canonical>(&mut ctx).unwrap()
};
// Compare against the traditional array-based decompress path
let expected = decompress_into_array(encoded);
let expected = decompress_into_array(encoded).unwrap();

assert_arrays_eq!(result_canonical.into_array(), expected);
}
Expand All @@ -646,7 +639,7 @@ mod tests {

let slice_end = size - slice_start;
let slice_len = slice_end - slice_start;
let sliced_encoded = encoded.slice(slice_start..slice_end);
let sliced_encoded = encoded.slice(slice_start..slice_end).unwrap();

let result_canonical = {
let mut ctx = SESSION.create_execution_ctx();
Expand All @@ -657,7 +650,7 @@ mod tests {
for idx in 0..slice_len {
let expected_value = values[slice_start + idx];

let result_valid = result_primitive.validity().is_valid(idx);
let result_valid = result_primitive.validity().is_valid(idx).unwrap();
assert_eq!(
result_valid,
expected_value.is_some(),
Expand Down Expand Up @@ -693,7 +686,7 @@ mod tests {

let slice_end = size - slice_start;
let slice_len = slice_end - slice_start;
let sliced_encoded = encoded.slice(slice_start..slice_end);
let sliced_encoded = encoded.slice(slice_start..slice_end).unwrap();

let result_primitive = sliced_encoded.to_primitive();

Expand Down Expand Up @@ -749,7 +742,8 @@ mod tests {
original_patches.indices().clone(),
original_patches.values().clone(),
None, // NO chunk_offsets - this triggers the bug!
);
)
.unwrap();

// Build a new ALPArray with the same encoded data but patches without chunk_offsets.
let alp_without_chunk_offsets = ALPArray::new(
Expand All @@ -759,7 +753,7 @@ mod tests {
);

// The legacy decompress_into_array path should work correctly.
let result_legacy = decompress_into_array(alp_without_chunk_offsets.clone());
let result_legacy = decompress_into_array(alp_without_chunk_offsets.clone()).unwrap();
let legacy_slice = result_legacy.as_slice::<f64>();

// Verify the legacy path produces correct values.
Expand Down
44 changes: 22 additions & 22 deletions encodings/alp/src/alp/compress.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ where
valid_exceptional_positions.into_array(),
valid_exceptional_values,
Some(chunk_offsets.into_array()),
))
)?)
};
Ok((exponents, encoded_array, patches))
}
Expand Down Expand Up @@ -150,7 +150,7 @@ mod tests {
assert_arrays_eq!(encoded.encoded(), expected_encoded);
assert_eq!(encoded.exponents(), Exponents { e: 9, f: 6 });

let decoded = decompress_into_array(encoded);
let decoded = decompress_into_array(encoded).unwrap();
assert_arrays_eq!(decoded, array);
}

Expand All @@ -163,7 +163,7 @@ mod tests {
assert_arrays_eq!(encoded.encoded(), expected_encoded);
assert_eq!(encoded.exponents(), Exponents { e: 9, f: 6 });

let decoded = decompress_into_array(encoded);
let decoded = decompress_into_array(encoded).unwrap();
let expected = PrimitiveArray::from_option_iter(vec![None, Some(1.234f32), None]);
assert_arrays_eq!(decoded, expected);
}
Expand All @@ -179,7 +179,7 @@ mod tests {
assert_arrays_eq!(encoded.encoded(), expected_encoded);
assert_eq!(encoded.exponents(), Exponents { e: 16, f: 13 });

let decoded = decompress_into_array(encoded);
let decoded = decompress_into_array(encoded).unwrap();
let expected_decoded = PrimitiveArray::new(values, Validity::NonNullable);
assert_arrays_eq!(decoded, expected_decoded);
}
Expand All @@ -196,7 +196,7 @@ mod tests {
assert_arrays_eq!(encoded.encoded(), expected_encoded);
assert_eq!(encoded.exponents(), Exponents { e: 16, f: 13 });

let decoded = decompress_into_array(encoded);
let decoded = decompress_into_array(encoded).unwrap();
assert_arrays_eq!(decoded, array);
}

Expand All @@ -217,7 +217,7 @@ mod tests {

assert_arrays_eq!(encoded, array);

let _decoded = decompress_into_array(encoded);
let _decoded = decompress_into_array(encoded).unwrap();
}

#[test]
Expand Down Expand Up @@ -361,9 +361,9 @@ mod tests {
let original = PrimitiveArray::new(Buffer::from(values), Validity::NonNullable);
let encoded = alp_encode(&original, None).unwrap();

let sliced_alp = encoded.slice(512..1024);
let sliced_alp = encoded.slice(512..1024).unwrap();

let expected_slice = original.slice(512..1024);
let expected_slice = original.slice(512..1024).unwrap();
assert_arrays_eq!(sliced_alp, expected_slice);
}

Expand All @@ -373,9 +373,9 @@ mod tests {
let original = PrimitiveArray::new(Buffer::from(values), Validity::NonNullable);
let encoded = alp_encode(&original, None).unwrap();

let sliced_alp = encoded.slice(512..1024);
let sliced_alp = encoded.slice(512..1024).unwrap();

let expected_slice = original.slice(512..1024);
let expected_slice = original.slice(512..1024).unwrap();
assert_arrays_eq!(sliced_alp, expected_slice);
}

Expand All @@ -389,9 +389,9 @@ mod tests {
let original = PrimitiveArray::new(Buffer::from(values), Validity::NonNullable);
let encoded = alp_encode(&original, None).unwrap();

let sliced_alp = encoded.slice(512..1024);
let sliced_alp = encoded.slice(512..1024).unwrap();

let expected_slice = original.slice(512..1024);
let expected_slice = original.slice(512..1024).unwrap();
assert_arrays_eq!(sliced_alp, expected_slice);
assert!(encoded.patches().is_some());
}
Expand All @@ -409,9 +409,9 @@ mod tests {
let original = PrimitiveArray::new(Buffer::from(values), Validity::NonNullable);
let encoded = alp_encode(&original, None).unwrap();

let sliced_alp = encoded.slice(1023..1025);
let sliced_alp = encoded.slice(1023..1025).unwrap();

let expected_slice = original.slice(1023..1025);
let expected_slice = original.slice(1023..1025).unwrap();
assert_arrays_eq!(sliced_alp, expected_slice);
assert!(encoded.patches().is_some());
}
Expand All @@ -425,10 +425,10 @@ mod tests {
let original = PrimitiveArray::from_option_iter(values);
let encoded = alp_encode(&original, None).unwrap();

let sliced_alp = encoded.slice(512..1024);
let sliced_alp = encoded.slice(512..1024).unwrap();
let decoded = sliced_alp.to_primitive();

let expected_slice = original.slice(512..1024);
let expected_slice = original.slice(512..1024).unwrap();
assert_arrays_eq!(decoded, expected_slice);
}

Expand All @@ -439,7 +439,7 @@ mod tests {
let encoded = alp_encode(&array, None).unwrap();

assert!(encoded.patches().is_none());
let decoded = decompress_into_array(encoded);
let decoded = decompress_into_array(encoded).unwrap();
assert_arrays_eq!(decoded, array);
}

Expand All @@ -450,7 +450,7 @@ mod tests {
let encoded = alp_encode(&array, None).unwrap();

assert!(encoded.patches().is_none());
let decoded = decompress_into_array(encoded);
let decoded = decompress_into_array(encoded).unwrap();
assert_arrays_eq!(decoded, array);
}

Expand All @@ -467,7 +467,7 @@ mod tests {
let encoded = alp_encode(&array, None).unwrap();

assert!(encoded.patches().is_some());
let decoded = decompress_into_array(encoded);
let decoded = decompress_into_array(encoded).unwrap();
assert_arrays_eq!(decoded, array);
}

Expand All @@ -488,7 +488,7 @@ mod tests {
let encoded = alp_encode(&array, None).unwrap();

assert!(encoded.patches().is_some());
let decoded = decompress_into_array(encoded);
let decoded = decompress_into_array(encoded).unwrap();

for idx in 0..size {
let decoded_val = decoded.as_slice::<f64>()[idx];
Expand All @@ -515,7 +515,7 @@ mod tests {

let array = PrimitiveArray::from_option_iter(values);
let encoded = alp_encode(&array, None).unwrap();
let decoded = decompress_into_array(encoded);
let decoded = decompress_into_array(encoded).unwrap();

assert_arrays_eq!(decoded, array);
}
Expand All @@ -535,7 +535,7 @@ mod tests {

let array = PrimitiveArray::new(Buffer::from(values), validity);
let encoded = alp_encode(&array, None).unwrap();
let decoded = decompress_into_array(encoded);
let decoded = decompress_into_array(encoded).unwrap();

assert_arrays_eq!(decoded, array);
}
Expand Down
14 changes: 6 additions & 8 deletions encodings/alp/src/alp/decompress.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use crate::match_each_alp_float_ptype;
/// # Returns
///
/// A `PrimitiveArray` containing the decompressed floating-point values with all patches applied.
pub fn decompress_into_array(array: ALPArray) -> PrimitiveArray {
pub fn decompress_into_array(array: ALPArray) -> VortexResult<PrimitiveArray> {
let (encoded, exponents, patches, dtype) = array.into_parts();
if let Some(ref patches) = patches
&& let Some(chunk_offsets) = patches.chunk_offsets()
Expand All @@ -38,15 +38,15 @@ pub fn decompress_into_array(array: ALPArray) -> PrimitiveArray {
let patches_chunk_offsets = chunk_offsets.as_ref().to_primitive();
let patches_indices = patches.indices().to_primitive();
let patches_values = patches.values().to_primitive();
decompress_chunked_core(
Ok(decompress_chunked_core(
prim_encoded,
exponents,
&patches_indices,
&patches_values,
&patches_chunk_offsets,
patches,
dtype,
)
))
} else {
let encoded_prim = encoded.to_primitive();
// We need to drop ALPArray here in case converting encoded buffer into
Expand Down Expand Up @@ -86,9 +86,7 @@ pub fn execute_decompress(array: ALPArray, ctx: &mut ExecutionCtx) -> VortexResu
))
} else {
let encoded = encoded.execute::<PrimitiveArray>(ctx)?;
Ok(decompress_unchunked_core(
encoded, exponents, patches, dtype,
))
decompress_unchunked_core(encoded, exponents, patches, dtype)
}
}

Expand Down Expand Up @@ -157,7 +155,7 @@ fn decompress_unchunked_core(
exponents: Exponents,
patches: Option<Patches>,
dtype: DType,
) -> PrimitiveArray {
) -> VortexResult<PrimitiveArray> {
let validity = encoded.validity().clone();
let ptype = dtype.as_ptype();

Expand All @@ -171,6 +169,6 @@ fn decompress_unchunked_core(
if let Some(patches) = patches {
decoded.patch(&patches)
} else {
decoded
Ok(decoded)
}
}
Loading
Loading