Skip to content

Decode stored DecimalType and PType values strictly #10755

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
connortsui20 merged 4 commits into from
Oct 26, 2026
Merged

Decode stored DecimalType and PType values strictly #10755

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
0789a11
fix(array): decode decimal values_type strictly
connortsui20 Oct 23, 2026
1905512
fix(array): decode protobuf PType strictly
connortsui20 Oct 23, 2026
71df8d5
fix: decode PType metadata fields strictly
connortsui20 Oct 23, 2026
07368ee
Remove lenient prost enum methods from strict metadata
connortsui20 Oct 26, 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
52 changes: 50 additions & 2 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 @@ -66,12 +66,23 @@ pub struct ALPRDMetadata {
dict_len: u32,
#[prost(uint32, repeated, tag = "3")]
dict: Vec<u32>,
#[prost(enumeration = "PType", tag = "4")]

@connortsui20 connortsui20 Oct 26, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So in general I think we don't want to do this anymore

#[prost(int32, tag = "4")]
left_parts_ptype: i32,
#[prost(message, tag = "5")]
patches: Option<PatchesMetadata>,
}

impl ALPRDMetadata {
fn get_left_parts_ptype(&self) -> VortexResult<PType> {
PType::try_from(self.left_parts_ptype).map_err(|_| {
vortex_err!(
"ALPRDMetadata left_parts_ptype must be a known PType, got {}",
self.left_parts_ptype
)
})
}
}

impl ArrayHash for ALPRDData {
fn array_hash<H: Hasher>(&self, state: &mut H, accuracy: EqMode) {
self.left_parts_dictionary.array_hash(state, accuracy);
Expand Down Expand Up @@ -172,14 +183,15 @@ impl VTable for ALPRD {
session: &VortexSession,
) -> VortexResult<ArrayParts<Self>> {
let metadata = ALPRDMetadata::decode(metadata)?;
let left_parts_ptype = metadata.get_left_parts_ptype()?;
if children.len() < 2 {
vortex_bail!(
"Expected at least 2 children for ALPRD encoding, found {}",
children.len()
);
}

let left_parts_dtype = DType::Primitive(metadata.left_parts_ptype(), dtype.nullability());
let left_parts_dtype = DType::Primitive(left_parts_ptype, dtype.nullability());
let left_parts = children.get(0, &left_parts_dtype, len)?;
let left_parts_dictionary: Buffer<u16> = metadata.dict.as_slice()
[0..metadata.dict_len as usize]
Expand Down Expand Up @@ -565,15 +577,21 @@ mod test {

use prost::Message;
use rstest::rstest;
use vortex_array::ArrayRef;
use vortex_array::VortexSessionExecute;
use vortex_array::arrays::PrimitiveArray;
use vortex_array::assert_arrays_eq;
use vortex_array::dtype::DType;
use vortex_array::dtype::NativePType;
use vortex_array::dtype::Nullability;
use vortex_array::dtype::PType;
use vortex_array::patches::PatchesMetadata;
use vortex_array::test_harness::check_metadata;
use vortex_array::vtable::VTable;
use vortex_error::VortexResult;
use vortex_session::VortexSession;

use super::ALPRD;
use super::ALPRDMetadata;
use crate::ALPRDFloat;
use crate::RDEncoderExt;
Expand Down Expand Up @@ -633,4 +651,34 @@ mod test {
.encode_to_vec(),
);
}

#[test]
fn test_deserialize_rejects_unknown_left_parts_ptype() -> VortexResult<()> {
let metadata = ALPRDMetadata {
left_parts_ptype: 11,
..Default::default()
}
.encode_to_vec();
let children: Vec<ArrayRef> = Vec::new();
let result = ALPRD.deserialize(
&DType::Primitive(PType::F64, Nullability::NonNullable),
0,
&metadata,
&[],
&children,
&SESSION,
);
assert!(result.is_err_and(|err| {
err.to_string()
.contains("ALPRDMetadata left_parts_ptype must be a known PType, got 11")
}));

let known = ALPRDMetadata {
left_parts_ptype: PType::U32 as i32,
..Default::default()
};
assert_eq!(known.get_left_parts_ptype()?, PType::U32);

Ok(())
}
}
78 changes: 70 additions & 8 deletions encodings/fastlanes/src/rle/vtable/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use vortex_array::vtable::VTable;
use vortex_error::VortexResult;
use vortex_error::vortex_ensure;
use vortex_error::vortex_ensure_eq;
use vortex_error::vortex_err;
use vortex_error::vortex_panic;
use vortex_session::VortexSession;
use vortex_session::registry::CachedId;
Expand All @@ -51,16 +52,36 @@ pub struct RLEMetadata {
pub values_len: u64,
#[prost(uint64, tag = "2")]
pub indices_len: u64,
#[prost(enumeration = "PType", tag = "3")]
#[prost(int32, tag = "3")]
pub indices_ptype: i32,
#[prost(uint64, tag = "4")]
pub values_idx_offsets_len: u64,
#[prost(enumeration = "PType", tag = "5")]
#[prost(int32, tag = "5")]
pub values_idx_offsets_ptype: i32,
#[prost(uint64, tag = "6", default = "0")]
pub offset: u64,
}

impl RLEMetadata {
fn get_indices_ptype(&self) -> VortexResult<PType> {
PType::try_from(self.indices_ptype).map_err(|_| {
vortex_err!(
"RLEMetadata indices_ptype must be a known PType, got {}",
self.indices_ptype
)
})
}

fn get_values_idx_offsets_ptype(&self) -> VortexResult<PType> {
PType::try_from(self.values_idx_offsets_ptype).map_err(|_| {
vortex_err!(
"RLEMetadata values_idx_offsets_ptype must be a known PType, got {}",
self.values_idx_offsets_ptype
)
})
}
}

impl ArrayHash for RLEData {
fn array_hash<H: Hasher>(&self, state: &mut H, _accuracy: EqMode) {
self.offset.hash(state);
Expand Down Expand Up @@ -161,12 +182,14 @@ impl VTable for RLE {
children: &dyn ArrayChildren,
_session: &VortexSession,
) -> VortexResult<ArrayParts<Self>> {
let metadata = RLEMetadata::decode(metadata)?;
let indices_ptype = metadata.get_indices_ptype()?;
let values_idx_offsets_ptype = metadata.get_values_idx_offsets_ptype()?;
vortex_ensure!(
buffers.is_empty(),
"RLEArray expects 0 buffers, got {}",
buffers.len()
);
let metadata = RLEMetadata::decode(metadata)?;
let values = children.get(
0,
&DType::Primitive(dtype.as_ptype(), Nullability::NonNullable),
Expand All @@ -175,16 +198,13 @@ impl VTable for RLE {

let indices = children.get(
1,
&DType::Primitive(metadata.indices_ptype(), dtype.nullability()),
&DType::Primitive(indices_ptype, dtype.nullability()),
usize::try_from(metadata.indices_len)?,
)?;

let values_idx_offsets = children.get(
2,
&DType::Primitive(
metadata.values_idx_offsets_ptype(),
Nullability::NonNullable,
),
&DType::Primitive(values_idx_offsets_ptype, Nullability::NonNullable),
usize::try_from(metadata.values_idx_offsets_len)?,
)?;

Expand Down Expand Up @@ -328,9 +348,18 @@ fn validate_parts(
#[cfg(test)]
mod tests {
use prost::Message;
use rstest::rstest;
use vortex_array::ArrayRef;
use vortex_array::dtype::DType;
use vortex_array::dtype::Nullability;
use vortex_array::dtype::PType;
use vortex_array::test_harness::check_metadata;
use vortex_array::vtable::VTable;
use vortex_error::VortexResult;

use super::RLE;
use super::RLEMetadata;
use crate::test::SESSION;

#[cfg_attr(miri, ignore)]
#[test]
Expand All @@ -348,4 +377,37 @@ mod tests {
.encode_to_vec(),
);
}

#[rstest]
#[case::indices_ptype(
|raw| RLEMetadata { indices_ptype: raw, ..Default::default() },
RLEMetadata::get_indices_ptype,
"RLEMetadata indices_ptype must be a known PType, got 11",
)]
#[case::values_idx_offsets_ptype(
|raw| RLEMetadata { values_idx_offsets_ptype: raw, ..Default::default() },
RLEMetadata::get_values_idx_offsets_ptype,
"RLEMetadata values_idx_offsets_ptype must be a known PType, got 11",
)]
fn test_deserialize_rejects_unknown_ptype(
#[case] with_ptype: fn(i32) -> RLEMetadata,
#[case] get_ptype: fn(&RLEMetadata) -> VortexResult<PType>,
#[case] expected: &str,
) -> VortexResult<()> {
let metadata = with_ptype(11).encode_to_vec();
let children: Vec<ArrayRef> = Vec::new();
let result = RLE.deserialize(
&DType::Primitive(PType::U32, Nullability::NonNullable),
0,
&metadata,
&[],
&children,
&SESSION,
);
assert!(result.is_err_and(|err| err.to_string().contains(expected)));

assert_eq!(get_ptype(&with_ptype(PType::U32 as i32))?, PType::U32);

Ok(())
}
}
51 changes: 49 additions & 2 deletions encodings/runend/src/array.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ use vortex_error::VortexResult;
use vortex_error::vortex_bail;
use vortex_error::vortex_ensure;
use vortex_error::vortex_ensure_eq;
use vortex_error::vortex_err;
use vortex_error::vortex_panic;
use vortex_session::VortexSession;
use vortex_session::registry::CachedId;
Expand All @@ -61,14 +62,25 @@ pub type RunEndArray = Array<RunEnd>;

#[derive(Clone, prost::Message)]
pub struct RunEndMetadata {
#[prost(enumeration = "PType", tag = "1")]
#[prost(int32, tag = "1")]
pub ends_ptype: i32,
#[prost(uint64, tag = "2")]
pub num_runs: u64,
#[prost(uint64, tag = "3")]
pub offset: u64,
}

impl RunEndMetadata {
fn get_ends_ptype(&self) -> VortexResult<PType> {
PType::try_from(self.ends_ptype).map_err(|_| {
vortex_err!(
"RunEndMetadata ends_ptype must be a known PType, got {}",
self.ends_ptype
)
})
}
}

impl ArrayHash for RunEndData {
fn array_hash<H: Hasher>(&self, state: &mut H, _accuracy: EqMode) {
self.offset.hash(state);
Expand Down Expand Up @@ -152,7 +164,8 @@ impl VTable for RunEnd {
session: &VortexSession,
) -> VortexResult<ArrayParts<Self>> {
let metadata = RunEndMetadata::decode(metadata)?;
let ends_dtype = DType::Primitive(metadata.ends_ptype(), Nullability::NonNullable);
let ends_ptype = metadata.get_ends_ptype()?;
let ends_dtype = DType::Primitive(ends_ptype, Nullability::NonNullable);
let runs = usize::try_from(metadata.num_runs).vortex_expect("Must be a valid usize");
let ends = children.get(0, &ends_dtype, runs)?;

Expand Down Expand Up @@ -575,6 +588,8 @@ mod tests {
use std::sync::Arc;
use std::sync::LazyLock;

use prost::Message;
use vortex_array::ArrayRef;
use vortex_array::IntoArray;
use vortex_array::VortexSessionExecute;
use vortex_array::arrays::DecimalArray;
Expand All @@ -591,10 +606,12 @@ mod tests {
use vortex_array::dtype::PType;
use vortex_array::dtype::i256;
use vortex_array::validity::Validity;
use vortex_array::vtable::VTable;
use vortex_buffer::buffer;
use vortex_error::VortexResult;
use vortex_session::VortexSession;

use super::RunEndMetadata;
use crate::RunEnd;

static SESSION: LazyLock<VortexSession> = LazyLock::new(|| {
Expand Down Expand Up @@ -934,4 +951,34 @@ mod tests {
assert_arrays_eq!(arr, expected, &mut ctx);
Ok(())
}

#[test]
fn test_deserialize_rejects_unknown_ends_ptype() -> VortexResult<()> {
let metadata = RunEndMetadata {
ends_ptype: 11,
..Default::default()
}
.encode_to_vec();
let children: Vec<ArrayRef> = Vec::new();
let result = RunEnd.deserialize(
&DType::Primitive(PType::I32, Nullability::NonNullable),
0,
&metadata,
&[],
&children,
&SESSION,
);
assert!(result.is_err_and(|err| {
err.to_string()
.contains("RunEndMetadata ends_ptype must be a known PType, got 11")
}));

let known = RunEndMetadata {
ends_ptype: PType::U32 as i32,
..Default::default()
};
assert_eq!(known.get_ends_ptype()?, PType::U32);

Ok(())
}
}
Loading
Loading