Skip to content

Commit 422848b

Browse files
committed
address comments
Signed-off-by: Andrew Duffy <andrew@a10y.dev>
1 parent a126ed2 commit 422848b

3 files changed

Lines changed: 153 additions & 4 deletions

File tree

encodings/fastlanes/src/bitpacking/plugin.rs

Lines changed: 149 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ impl ArrayPlugin for BitPackedPatchedPlugin {
4747
.deserialize(dtype, len, metadata, buffers, children, session)?
4848
.try_downcast::<BitPacked>()
4949
.map_err(|_| {
50-
vortex_err!("BitPacked plugin should only deserialize vortex.bitpacked")
50+
vortex_err!("BitPacked plugin should only deserialize fastlanes.bitpacked")
5151
})?;
5252

5353
// Create a new BitPackedArray without the interior patches installed.
@@ -74,3 +74,151 @@ impl ArrayPlugin for BitPackedPatchedPlugin {
7474
Ok(patched.into_array())
7575
}
7676
}
77+
78+
#[cfg(test)]
79+
mod tests {
80+
use std::sync::LazyLock;
81+
82+
use vortex_array::ArrayPlugin;
83+
use vortex_array::IntoArray;
84+
use vortex_array::arrays::PatchedArray;
85+
use vortex_array::arrays::PrimitiveArray;
86+
use vortex_array::arrays::patched::PatchedArrayExt;
87+
use vortex_array::buffer::BufferHandle;
88+
use vortex_array::session::ArraySession;
89+
use vortex_array::session::ArraySessionExt;
90+
use vortex_buffer::Buffer;
91+
use vortex_error::VortexResult;
92+
use vortex_error::vortex_err;
93+
use vortex_session::VortexSession;
94+
95+
use super::BitPackedPatchedPlugin;
96+
use crate::BitPacked;
97+
use crate::BitPackedArray;
98+
use crate::BitPackedArrayExt;
99+
use crate::BitPackedData;
100+
101+
static SESSION: LazyLock<VortexSession> = LazyLock::new(|| {
102+
let session = VortexSession::empty().with::<ArraySession>();
103+
session.arrays().register(BitPackedPatchedPlugin);
104+
session
105+
});
106+
107+
#[test]
108+
fn test_decode_bitpacked_patches() -> VortexResult<()> {
109+
// Create values where some exceed the bit width, causing patches.
110+
// With bit_width=9, max value is 511. Values >=512 become patches.
111+
let values: Buffer<i32> = (0i32..=512).collect();
112+
let parray = values.into_array();
113+
let bitpacked = BitPackedData::encode(&parray, 9)?;
114+
115+
assert!(
116+
bitpacked.patches().is_some(),
117+
"Expected BitPacked array to have patches"
118+
);
119+
120+
let array = bitpacked.as_array();
121+
122+
let metadata = array.metadata()?.unwrap_or_default();
123+
let children = array.children();
124+
let buffers = array
125+
.buffers()
126+
.into_iter()
127+
.map(BufferHandle::new_host)
128+
.collect::<Vec<_>>();
129+
130+
let deserialized = BitPackedPatchedPlugin.deserialize(
131+
array.dtype(),
132+
array.len(),
133+
&metadata,
134+
&buffers,
135+
&children,
136+
&SESSION )?;
137+
138+
let patched: PatchedArray = deserialized
139+
.try_downcast()
140+
.map_err(|a| vortex_err!("Expected Patched, got {}", a.encoding_id()))?;
141+
142+
let inner_bitpacked: BitPackedArray = patched
143+
.base_array()
144+
.clone()
145+
.try_downcast()
146+
.map_err(|a| vortex_err!("Expected inner BitPacked, got {}", a.encoding_id()))?;
147+
148+
assert!(
149+
inner_bitpacked.patches().is_none(),
150+
"Inner BitPacked should NOT have patches"
151+
);
152+
153+
Ok(())
154+
}
155+
156+
#[test]
157+
fn bitpacked_without_patches_stays_bitpacked() -> VortexResult<()> {
158+
// With bit_width=16, max value is 65535. All values 0..100 fit.
159+
let values: Buffer<i32> = (0i32..100).collect();
160+
let parray = values.into_array();
161+
let bitpacked = BitPackedData::encode(&parray, 16)?;
162+
163+
assert!(
164+
bitpacked.patches().is_none(),
165+
"Expected BitPacked array without patches"
166+
);
167+
168+
let array = bitpacked.as_array();
169+
170+
let metadata = array.metadata()?.unwrap_or_default();
171+
let children = array.children();
172+
let buffers = array
173+
.buffers()
174+
.into_iter()
175+
.map(BufferHandle::new_host)
176+
.collect::<Vec<_>>();
177+
178+
let deserialized = BitPackedPatchedPlugin.deserialize(
179+
array.dtype(),
180+
array.len(),
181+
&metadata,
182+
&buffers,
183+
&children,
184+
&SESSION,
185+
)?;
186+
187+
let result = deserialized
188+
.try_downcast::<BitPacked>()
189+
.map_err(|a| vortex_err!("Expected deserialize BitPacked, got {}", a.encoding_id()))?;
190+
191+
assert!(result.patches().is_none(), "Result should not have patches");
192+
193+
Ok(())
194+
}
195+
196+
#[test]
197+
fn primitive_array_returns_error() -> VortexResult<()> {
198+
let array = PrimitiveArray::from_iter([1i32, 2, 3]).into_array();
199+
200+
let metadata = array.metadata()?.unwrap_or_default();
201+
let children = array.children();
202+
let buffers = array
203+
.buffers()
204+
.into_iter()
205+
.map(BufferHandle::new_host)
206+
.collect::<Vec<_>>();
207+
208+
let result = BitPackedPatchedPlugin.deserialize(
209+
array.dtype(),
210+
array.len(),
211+
&metadata,
212+
&buffers,
213+
&children,
214+
&SESSION,
215+
);
216+
217+
assert!(
218+
result.is_err(),
219+
"Expected error when deserializing PrimitiveArray with BitPackedPatchedPlugin"
220+
);
221+
222+
Ok(())
223+
}
224+
}

vortex-array/src/serde.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -268,16 +268,16 @@ pub trait ArrayChildren {
268268
}
269269
}
270270

271-
impl ArrayChildren for &[ArrayRef] {
271+
impl<T: AsRef<[ArrayRef]>> ArrayChildren for T {
272272
fn get(&self, index: usize, dtype: &DType, len: usize) -> VortexResult<ArrayRef> {
273-
let array = self[index].clone();
273+
let array = self.as_ref()[index].clone();
274274
assert_eq!(array.len(), len);
275275
assert_eq!(array.dtype(), dtype);
276276
Ok(array)
277277
}
278278

279279
fn len(&self) -> usize {
280-
<[_]>::len(self)
280+
self.as_ref().len()
281281
}
282282
}
283283

vortex-btrblocks/src/schemes/integer.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,7 @@ impl Scheme for BitPackingScheme {
351351
&p,
352352
&mut LEGACY_SESSION.create_execution_ctx(),
353353
)?
354+
.with_stats_set(packed_stats)
354355
.into_array(),
355356
}
356357
} else {

0 commit comments

Comments
 (0)