Skip to content

Commit 15573bb

Browse files
joseph-isaacsjoseph-isaacs
committed
fix
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
1 parent f87ffd5 commit 15573bb

7 files changed

Lines changed: 205 additions & 28 deletions

File tree

‎encodings/bytebool/src/array.rs‎

Lines changed: 57 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ use vortex_array::vtable::OperationsVTable;
2626
use vortex_array::vtable::VTable;
2727
use vortex_array::vtable::ValidityHelper;
2828
use vortex_array::vtable::ValidityVTableFromValidityHelper;
29+
use vortex_array::vtable::validity_to_child;
2930
use vortex_buffer::BitBuffer;
3031
use vortex_buffer::ByteBuffer;
3132
use vortex_error::VortexResult;
@@ -142,16 +143,20 @@ impl VTable for ByteBool {
142143
}
143144

144145
fn slot_name(_array: &ByteBoolArray, idx: usize) -> String {
145-
let _ = SLOT_NAMES;
146-
vortex_panic!("ByteBoolArray has no slots, requested index {idx}")
146+
SLOT_NAMES[idx].to_string()
147147
}
148148

149149
fn with_slots(array: &mut ByteBoolArray, slots: Vec<Option<ArrayRef>>) -> VortexResult<()> {
150150
vortex_ensure!(
151-
slots.is_empty(),
152-
"ByteBoolArray expects 0 slots, got {}",
151+
slots.len() == NUM_SLOTS,
152+
"ByteBoolArray expects {} slots, got {}",
153+
NUM_SLOTS,
153154
slots.len()
154155
);
156+
array.validity = match &slots[VALIDITY_SLOT] {
157+
Some(arr) => Validity::Array(arr.clone()),
158+
None => Validity::from(array.dtype.nullability()),
159+
};
155160
array.slots = slots;
156161
Ok(())
157162
}
@@ -182,7 +187,9 @@ impl VTable for ByteBool {
182187
}
183188
}
184189

185-
pub(super) const SLOT_NAMES: [&str; 0] = [];
190+
pub(super) const VALIDITY_SLOT: usize = 0;
191+
pub(super) const NUM_SLOTS: usize = 1;
192+
pub(super) const SLOT_NAMES: [&str; NUM_SLOTS] = ["validity"];
186193

187194
#[derive(Clone, Debug)]
188195
pub struct ByteBoolArray {
@@ -201,6 +208,10 @@ impl ByteBool {
201208
}
202209

203210
impl ByteBoolArray {
211+
fn make_slots(validity: &Validity, len: usize) -> Vec<Option<ArrayRef>> {
212+
vec![validity_to_child(validity, len)]
213+
}
214+
204215
pub fn new(buffer: BufferHandle, validity: Validity) -> Self {
205216
let length = buffer.len();
206217
if let Some(vlen) = validity.maybe_len()
@@ -212,11 +223,12 @@ impl ByteBoolArray {
212223
vlen
213224
);
214225
}
226+
let slots = Self::make_slots(&validity, length);
215227
Self {
216228
dtype: DType::Bool(validity.nullability()),
217229
buffer,
218230
validity,
219-
slots: vec![],
231+
slots,
220232
stats_set: Default::default(),
221233
}
222234
}
@@ -273,6 +285,17 @@ impl From<Vec<Option<bool>>> for ByteBoolArray {
273285

274286
#[cfg(test)]
275287
mod tests {
288+
use vortex_array::ArrayContext;
289+
use vortex_array::IntoArray;
290+
use vortex_array::assert_arrays_eq;
291+
use vortex_array::serde::ArrayParts;
292+
use vortex_array::serde::SerializeOptions;
293+
use vortex_array::session::ArraySession;
294+
use vortex_array::session::ArraySessionExt;
295+
use vortex_buffer::ByteBufferMut;
296+
use vortex_session::VortexSession;
297+
use vortex_session::registry::ReadContext;
298+
276299
use super::*;
277300

278301
#[test]
@@ -305,4 +328,32 @@ mod tests {
305328
}
306329
assert_eq!(arr.len(), 2);
307330
}
331+
332+
#[test]
333+
fn test_nullable_bytebool_serde_roundtrip() {
334+
let array = ByteBoolArray::from(vec![Some(true), None, Some(false), None]);
335+
let dtype = array.dtype().clone();
336+
let len = array.len();
337+
let session = VortexSession::empty().with::<ArraySession>();
338+
session.arrays().register(ByteBool::ID, ByteBool);
339+
340+
let ctx = ArrayContext::empty();
341+
let serialized = array
342+
.clone()
343+
.into_array()
344+
.serialize(&ctx, &SerializeOptions::default())
345+
.unwrap();
346+
347+
let mut concat = ByteBufferMut::empty();
348+
for buf in serialized {
349+
concat.extend_from_slice(buf.as_ref());
350+
}
351+
352+
let parts = ArrayParts::try_from(concat.freeze()).unwrap();
353+
let decoded = parts
354+
.decode(&dtype, len, &ReadContext::new(ctx.to_ids()), &session)
355+
.unwrap();
356+
357+
assert_arrays_eq!(decoded, array);
358+
}
308359
}

‎vortex-array/public-api.lock‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4950,7 +4950,7 @@ pub fn vortex_array::arrays::Variant::build(dtype: &vortex_array::dtype::DType,
49504950

49514951
pub fn vortex_array::arrays::Variant::child(array: &Self::Array, idx: usize) -> vortex_array::ArrayRef
49524952

4953-
pub fn vortex_array::arrays::Variant::child_name(_array: &Self::Array, idx: usize) -> alloc::string::String
4953+
pub fn vortex_array::arrays::Variant::child_name(array: &Self::Array, idx: usize) -> alloc::string::String
49544954

49554955
pub fn vortex_array::arrays::Variant::deserialize(_bytes: &[u8], _dtype: &vortex_array::dtype::DType, _len: usize, _buffers: &[vortex_array::buffer::BufferHandle], _session: &vortex_session::VortexSession) -> vortex_error::VortexResult<Self::Metadata>
49564956

@@ -4968,17 +4968,21 @@ pub fn vortex_array::arrays::Variant::metadata(_array: &Self::Array) -> vortex_e
49684968

49694969
pub fn vortex_array::arrays::Variant::nbuffers(_array: &Self::Array) -> usize
49704970

4971-
pub fn vortex_array::arrays::Variant::nchildren(_array: &Self::Array) -> usize
4971+
pub fn vortex_array::arrays::Variant::nchildren(array: &Self::Array) -> usize
49724972

49734973
pub fn vortex_array::arrays::Variant::reduce(array: &Self::Array) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
49744974

49754975
pub fn vortex_array::arrays::Variant::reduce_parent(array: &Self::Array, parent: &vortex_array::ArrayRef, child_idx: usize) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
49764976

49774977
pub fn vortex_array::arrays::Variant::serialize(_metadata: Self::Metadata) -> vortex_error::VortexResult<core::option::Option<alloc::vec::Vec<u8>>>
49784978

4979+
pub fn vortex_array::arrays::Variant::slot_name(_array: &Self::Array, idx: usize) -> alloc::string::String
4980+
4981+
pub fn vortex_array::arrays::Variant::slots(array: &Self::Array) -> &[core::option::Option<vortex_array::ArrayRef>]
4982+
49794983
pub fn vortex_array::arrays::Variant::stats(array: &Self::Array) -> vortex_array::stats::StatsSetRef<'_>
49804984

4981-
pub fn vortex_array::arrays::Variant::with_children(array: &mut Self::Array, children: alloc::vec::Vec<vortex_array::ArrayRef>) -> vortex_error::VortexResult<()>
4985+
pub fn vortex_array::arrays::Variant::with_slots(array: &mut Self::Array, slots: alloc::vec::Vec<core::option::Option<vortex_array::ArrayRef>>) -> vortex_error::VortexResult<()>
49824986

49834987
impl vortex_array::vtable::ValidityVTable<vortex_array::arrays::Variant> for vortex_array::arrays::Variant
49844988

@@ -8178,7 +8182,7 @@ pub fn vortex_array::arrays::Variant::build(dtype: &vortex_array::dtype::DType,
81788182

81798183
pub fn vortex_array::arrays::Variant::child(array: &Self::Array, idx: usize) -> vortex_array::ArrayRef
81808184

8181-
pub fn vortex_array::arrays::Variant::child_name(_array: &Self::Array, idx: usize) -> alloc::string::String
8185+
pub fn vortex_array::arrays::Variant::child_name(array: &Self::Array, idx: usize) -> alloc::string::String
81828186

81838187
pub fn vortex_array::arrays::Variant::deserialize(_bytes: &[u8], _dtype: &vortex_array::dtype::DType, _len: usize, _buffers: &[vortex_array::buffer::BufferHandle], _session: &vortex_session::VortexSession) -> vortex_error::VortexResult<Self::Metadata>
81848188

@@ -8196,17 +8200,21 @@ pub fn vortex_array::arrays::Variant::metadata(_array: &Self::Array) -> vortex_e
81968200

81978201
pub fn vortex_array::arrays::Variant::nbuffers(_array: &Self::Array) -> usize
81988202

8199-
pub fn vortex_array::arrays::Variant::nchildren(_array: &Self::Array) -> usize
8203+
pub fn vortex_array::arrays::Variant::nchildren(array: &Self::Array) -> usize
82008204

82018205
pub fn vortex_array::arrays::Variant::reduce(array: &Self::Array) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
82028206

82038207
pub fn vortex_array::arrays::Variant::reduce_parent(array: &Self::Array, parent: &vortex_array::ArrayRef, child_idx: usize) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
82048208

82058209
pub fn vortex_array::arrays::Variant::serialize(_metadata: Self::Metadata) -> vortex_error::VortexResult<core::option::Option<alloc::vec::Vec<u8>>>
82068210

8211+
pub fn vortex_array::arrays::Variant::slot_name(_array: &Self::Array, idx: usize) -> alloc::string::String
8212+
8213+
pub fn vortex_array::arrays::Variant::slots(array: &Self::Array) -> &[core::option::Option<vortex_array::ArrayRef>]
8214+
82078215
pub fn vortex_array::arrays::Variant::stats(array: &Self::Array) -> vortex_array::stats::StatsSetRef<'_>
82088216

8209-
pub fn vortex_array::arrays::Variant::with_children(array: &mut Self::Array, children: alloc::vec::Vec<vortex_array::ArrayRef>) -> vortex_error::VortexResult<()>
8217+
pub fn vortex_array::arrays::Variant::with_slots(array: &mut Self::Array, slots: alloc::vec::Vec<core::option::Option<vortex_array::ArrayRef>>) -> vortex_error::VortexResult<()>
82108218

82118219
impl vortex_array::vtable::ValidityVTable<vortex_array::arrays::Variant> for vortex_array::arrays::Variant
82128220

@@ -21614,7 +21622,7 @@ pub fn vortex_array::arrays::Variant::build(dtype: &vortex_array::dtype::DType,
2161421622

2161521623
pub fn vortex_array::arrays::Variant::child(array: &Self::Array, idx: usize) -> vortex_array::ArrayRef
2161621624

21617-
pub fn vortex_array::arrays::Variant::child_name(_array: &Self::Array, idx: usize) -> alloc::string::String
21625+
pub fn vortex_array::arrays::Variant::child_name(array: &Self::Array, idx: usize) -> alloc::string::String
2161821626

2161921627
pub fn vortex_array::arrays::Variant::deserialize(_bytes: &[u8], _dtype: &vortex_array::dtype::DType, _len: usize, _buffers: &[vortex_array::buffer::BufferHandle], _session: &vortex_session::VortexSession) -> vortex_error::VortexResult<Self::Metadata>
2162021628

@@ -21632,17 +21640,21 @@ pub fn vortex_array::arrays::Variant::metadata(_array: &Self::Array) -> vortex_e
2163221640

2163321641
pub fn vortex_array::arrays::Variant::nbuffers(_array: &Self::Array) -> usize
2163421642

21635-
pub fn vortex_array::arrays::Variant::nchildren(_array: &Self::Array) -> usize
21643+
pub fn vortex_array::arrays::Variant::nchildren(array: &Self::Array) -> usize
2163621644

2163721645
pub fn vortex_array::arrays::Variant::reduce(array: &Self::Array) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
2163821646

2163921647
pub fn vortex_array::arrays::Variant::reduce_parent(array: &Self::Array, parent: &vortex_array::ArrayRef, child_idx: usize) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
2164021648

2164121649
pub fn vortex_array::arrays::Variant::serialize(_metadata: Self::Metadata) -> vortex_error::VortexResult<core::option::Option<alloc::vec::Vec<u8>>>
2164221650

21651+
pub fn vortex_array::arrays::Variant::slot_name(_array: &Self::Array, idx: usize) -> alloc::string::String
21652+
21653+
pub fn vortex_array::arrays::Variant::slots(array: &Self::Array) -> &[core::option::Option<vortex_array::ArrayRef>]
21654+
2164321655
pub fn vortex_array::arrays::Variant::stats(array: &Self::Array) -> vortex_array::stats::StatsSetRef<'_>
2164421656

21645-
pub fn vortex_array::arrays::Variant::with_children(array: &mut Self::Array, children: alloc::vec::Vec<vortex_array::ArrayRef>) -> vortex_error::VortexResult<()>
21657+
pub fn vortex_array::arrays::Variant::with_slots(array: &mut Self::Array, slots: alloc::vec::Vec<core::option::Option<vortex_array::ArrayRef>>) -> vortex_error::VortexResult<()>
2164621658

2164721659
impl vortex_array::vtable::VTable for vortex_array::arrays::dict::Dict
2164821660

‎vortex-array/src/arrays/decimal/array.rs‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,11 @@ use crate::patches::Patches;
2828
use crate::stats::ArrayStats;
2929
use crate::validity::Validity;
3030
use crate::vtable::ValidityHelper;
31+
use crate::vtable::validity_to_child;
3132

32-
pub(super) const SLOT_NAMES: [&str; 0] = [];
33+
pub(super) const VALIDITY_SLOT: usize = 0;
34+
pub(super) const NUM_SLOTS: usize = 1;
35+
pub(super) const SLOT_NAMES: [&str; NUM_SLOTS] = ["validity"];
3336

3437
/// A decimal array that stores fixed-precision decimal numbers with configurable scale.
3538
///
@@ -106,6 +109,10 @@ pub struct DecimalArrayParts {
106109
}
107110

108111
impl DecimalArray {
112+
fn make_slots(validity: &Validity, len: usize) -> Vec<Option<ArrayRef>> {
113+
vec![validity_to_child(validity, len)]
114+
}
115+
109116
/// Creates a new [`DecimalArray`] using a host-native buffer.
110117
///
111118
/// # Panics
@@ -226,8 +233,9 @@ impl DecimalArray {
226233
.vortex_expect("[Debug Assertion]: Invalid `DecimalArray` parameters");
227234
}
228235

236+
let len = values.len() / values_type.byte_width();
229237
Self {
230-
slots: vec![],
238+
slots: Self::make_slots(&validity, len),
231239
values,
232240
values_type,
233241
dtype: DType::Decimal(decimal_dtype, validity.nullability()),

‎vortex-array/src/arrays/decimal/vtable/mod.rs‎

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@ use crate::IntoArray;
1717
use crate::ProstMetadata;
1818
use crate::SerializeMetadata;
1919
use crate::arrays::DecimalArray;
20+
use crate::arrays::decimal::array::NUM_SLOTS;
2021
use crate::arrays::decimal::array::SLOT_NAMES;
22+
use crate::arrays::decimal::array::VALIDITY_SLOT;
2123
use crate::buffer::BufferHandle;
2224
use crate::dtype::DType;
2325
use crate::dtype::DecimalType;
@@ -174,16 +176,20 @@ impl VTable for Decimal {
174176
}
175177

176178
fn slot_name(_array: &DecimalArray, idx: usize) -> String {
177-
let _ = SLOT_NAMES;
178-
vortex_panic!("DecimalArray has no slots, requested index {idx}")
179+
SLOT_NAMES[idx].to_string()
179180
}
180181

181182
fn with_slots(array: &mut DecimalArray, slots: Vec<Option<ArrayRef>>) -> VortexResult<()> {
182183
vortex_ensure!(
183-
slots.is_empty(),
184-
"DecimalArray expects 0 slots, got {}",
184+
slots.len() == NUM_SLOTS,
185+
"DecimalArray expects {} slots, got {}",
186+
NUM_SLOTS,
185187
slots.len()
186188
);
189+
array.validity = match &slots[VALIDITY_SLOT] {
190+
Some(arr) => Validity::Array(arr.clone()),
191+
None => Validity::from(array.dtype.nullability()),
192+
};
187193
array.slots = slots;
188194
Ok(())
189195
}
@@ -228,6 +234,7 @@ mod tests {
228234
use crate::LEGACY_SESSION;
229235
use crate::arrays::Decimal;
230236
use crate::arrays::DecimalArray;
237+
use crate::assert_arrays_eq;
231238
use crate::dtype::DecimalDType;
232239
use crate::serde::ArrayParts;
233240
use crate::serde::SerializeOptions;
@@ -261,4 +268,38 @@ mod tests {
261268
.unwrap();
262269
assert!(decoded.is::<Decimal>());
263270
}
271+
272+
#[test]
273+
fn test_nullable_decimal_serde_roundtrip() {
274+
let array = DecimalArray::new(
275+
buffer![1234567i32, 0i32, -9999999i32],
276+
DecimalDType::new(7, 3),
277+
Validity::from_iter([true, false, true]),
278+
);
279+
let dtype = array.dtype().clone();
280+
let len = array.len();
281+
282+
let ctx = ArrayContext::empty();
283+
let out = array
284+
.clone()
285+
.into_array()
286+
.serialize(&ctx, &SerializeOptions::default())
287+
.unwrap();
288+
let mut concat = ByteBufferMut::empty();
289+
for buf in out {
290+
concat.extend_from_slice(buf.as_ref());
291+
}
292+
293+
let parts = ArrayParts::try_from(concat.freeze()).unwrap();
294+
let decoded = parts
295+
.decode(
296+
&dtype,
297+
len,
298+
&ReadContext::new(ctx.to_ids()),
299+
&LEGACY_SESSION,
300+
)
301+
.unwrap();
302+
303+
assert_arrays_eq!(decoded, array);
304+
}
264305
}

‎vortex-array/src/arrays/varbinview/array.rs‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,11 @@ use crate::dtype::DType;
2222
use crate::dtype::Nullability;
2323
use crate::stats::ArrayStats;
2424
use crate::validity::Validity;
25+
use crate::vtable::validity_to_child;
2526

26-
pub(super) const SLOT_NAMES: [&str; 0] = [];
27+
pub(super) const VALIDITY_SLOT: usize = 0;
28+
pub(super) const NUM_SLOTS: usize = 1;
29+
pub(super) const SLOT_NAMES: [&str; NUM_SLOTS] = ["validity"];
2730

2831
/// A variable-length binary view array that stores strings and binary data efficiently.
2932
///
@@ -102,6 +105,10 @@ pub struct VarBinViewArrayParts {
102105
}
103106

104107
impl VarBinViewArray {
108+
fn make_slots(validity: &Validity, len: usize) -> Vec<Option<ArrayRef>> {
109+
vec![validity_to_child(validity, len)]
110+
}
111+
105112
/// Creates a new [`VarBinViewArray`].
106113
///
107114
/// # Panics
@@ -248,8 +255,9 @@ impl VarBinViewArray {
248255
dtype: DType,
249256
validity: Validity,
250257
) -> Self {
258+
let len = views.len() / size_of::<BinaryView>();
251259
Self {
252-
slots: vec![],
260+
slots: Self::make_slots(&validity, len),
253261
views,
254262
buffers,
255263
dtype,

0 commit comments

Comments
 (0)