Skip to content

Commit 4c5742d

Browse files
committed
address comments
Signed-off-by: Andrew Duffy <andrew@a10y.dev>
1 parent af75dfb commit 4c5742d

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
@@ -56,7 +56,7 @@ impl ArrayPlugin for BitPackedPatchedPlugin {
5656
.deserialize(dtype, len, metadata, buffers, children, session)?
5757
.try_downcast::<BitPacked>()
5858
.map_err(|_| {
59-
vortex_err!("BitPacked plugin should only deserialize vortex.bitpacked")
59+
vortex_err!("BitPacked plugin should only deserialize fastlanes.bitpacked")
6060
})?;
6161

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

vortex-array/src/serde.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -278,16 +278,16 @@ pub trait ArrayChildren {
278278
}
279279
}
280280

281-
impl ArrayChildren for &[ArrayRef] {
281+
impl<T: AsRef<[ArrayRef]>> ArrayChildren for T {
282282
fn get(&self, index: usize, dtype: &DType, len: usize) -> VortexResult<ArrayRef> {
283-
let array = self[index].clone();
283+
let array = self.as_ref()[index].clone();
284284
assert_eq!(array.len(), len);
285285
assert_eq!(array.dtype(), dtype);
286286
Ok(array)
287287
}
288288

289289
fn len(&self) -> usize {
290-
<[_]>::len(self)
290+
self.as_ref().len()
291291
}
292292
}
293293

vortex-btrblocks/src/schemes/integer.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,7 @@ impl Scheme for BitPackingScheme {
364364
&p,
365365
&mut LEGACY_SESSION.create_execution_ctx(),
366366
)?
367+
.with_stats_set(packed_stats)
367368
.into_array(),
368369
}
369370
} else {

0 commit comments

Comments
 (0)