Skip to content

Commit b4d5826

Browse files
committed
change mutable struct fields to Arc<Box>
Signed-off-by: Connor Tsui <[email protected]>
1 parent 20c1cda commit b4d5826

File tree

2 files changed

+53
-26
lines changed

2 files changed

+53
-26
lines changed

vortex-vector/src/struct_/vector.rs

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
//! Definition and implementation of [`StructVector`].
55
6+
use std::sync::Arc;
7+
68
use vortex_error::{VortexExpect, VortexResult, vortex_ensure};
79
use vortex_mask::Mask;
810

@@ -17,7 +19,14 @@ use crate::{StructVectorMut, Vector, VectorMutOps, VectorOps};
1719
#[derive(Debug, Clone)]
1820
pub struct StructVector {
1921
/// The fields of the `StructVector`, each stored column-wise as a [`Vector`].
20-
pub(super) fields: Box<[Vector]>,
22+
///
23+
/// We store these as an [`Arc<Box<_>>`] because we need to call [`try_unwrap()`] in our
24+
/// [`try_into_mut()`] implementation, and since slices are unsized it is not implemented for
25+
/// [`Arc<[Vector]>`].
26+
///
27+
/// [`try_unwrap()`]: Arc::try_unwrap
28+
/// [`try_into_mut()`]: Self::try_into_mut
29+
pub(super) fields: Arc<Box<[Vector]>>,
2130

2231
/// The validity mask (where `true` represents an element is **not** null).
2332
pub(super) validity: Mask,
@@ -32,25 +41,31 @@ pub struct StructVector {
3241
impl StructVector {
3342
/// Creates a new [`StructVector`] from the given fields and validity mask.
3443
///
44+
/// Note that we take [`Arc<Box<[_]>>`] in order to enable easier conversion to
45+
/// [`StructVectorMut`] via [`try_into_mut()`](Self::try_into_mut).
46+
///
3547
/// # Panics
3648
///
3749
/// Panics if:
3850
///
3951
/// - Any field vector has a length that does not match the length of other fields.
4052
/// - The validity mask length does not match the field length.
41-
pub fn new(fields: Box<[Vector]>, validity: Mask) -> Self {
53+
pub fn new(fields: Arc<Box<[Vector]>>, validity: Mask) -> Self {
4254
Self::try_new(fields, validity).vortex_expect("Failed to create `StructVector`")
4355
}
4456

4557
/// Tries to create a new [`StructVector`] from the given fields and validity mask.
4658
///
59+
/// Note that we take [`Arc<Box<[_]>>`] in order to enable easier conversion to
60+
/// [`StructVectorMut`] via [`try_into_mut()`](Self::try_into_mut).
61+
///
4762
/// # Errors
4863
///
4964
/// Returns an error if:
5065
///
5166
/// - Any field vector has a length that does not match the length of other fields.
5267
/// - The validity mask length does not match the field length.
53-
pub fn try_new(fields: Box<[Vector]>, validity: Mask) -> VortexResult<Self> {
68+
pub fn try_new(fields: Arc<Box<[Vector]>>, validity: Mask) -> VortexResult<Self> {
5469
let len = if fields.is_empty() {
5570
validity.len()
5671
} else {
@@ -85,13 +100,16 @@ impl StructVector {
85100

86101
/// Creates a new [`StructVector`] from the given fields and validity mask without validation.
87102
///
103+
/// Note that we take [`Arc<Box<[_]>>`] in order to enable easier conversion to
104+
/// [`StructVectorMut`] via [`try_into_mut()`](Self::try_into_mut).
105+
///
88106
/// # Safety
89107
///
90108
/// The caller must ensure that:
91109
///
92110
/// - All field vectors have the same length.
93111
/// - The validity mask has a length equal to the field length.
94-
pub unsafe fn new_unchecked(fields: Box<[Vector]>, validity: Mask) -> Self {
112+
pub unsafe fn new_unchecked(fields: Arc<Box<[Vector]>>, validity: Mask) -> Self {
95113
let len = if fields.is_empty() {
96114
validity.len()
97115
} else {
@@ -110,7 +128,7 @@ impl StructVector {
110128
}
111129

112130
/// Decomposes the struct vector into its constituent parts (fields, validity, and length).
113-
pub fn into_parts(self) -> (Box<[Vector]>, Mask, usize) {
131+
pub fn into_parts(self) -> (Arc<Box<[Vector]>>, Mask, usize) {
114132
(self.fields, self.validity, self.len)
115133
}
116134

@@ -135,16 +153,26 @@ impl VectorOps for StructVector {
135153
where
136154
Self: Sized,
137155
{
156+
let len = self.len;
157+
let fields = match Arc::try_unwrap(self.fields) {
158+
Ok(fields) => fields,
159+
Err(fields) => return Err(StructVector { fields, ..self }),
160+
};
161+
138162
let validity = match self.validity.try_into_mut() {
139163
Ok(validity) => validity,
140164
Err(validity) => {
141-
return Err(StructVector { validity, ..self });
165+
return Err(StructVector {
166+
fields: Arc::new(fields),
167+
validity,
168+
len,
169+
});
142170
}
143171
};
144172

145173
// Convert all of the remaining fields to mutable, if possible.
146-
let mut mutable_fields = Vec::with_capacity(self.fields.len());
147-
let mut fields_iter = self.fields.into_iter();
174+
let mut mutable_fields = Vec::with_capacity(fields.len());
175+
let mut fields_iter = fields.into_iter();
148176

149177
while let Some(field) = fields_iter.next() {
150178
match field.try_into_mut() {
@@ -164,7 +192,7 @@ impl VectorOps for StructVector {
164192
all_fields.extend(fields_iter);
165193

166194
return Err(StructVector {
167-
fields: all_fields.into_boxed_slice(),
195+
fields: Arc::new(all_fields.into_boxed_slice()),
168196
len: self.len,
169197
validity: validity.freeze(),
170198
});

vortex-vector/src/struct_/vector_mut.rs

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
//! Definition and implementation of [`StructVectorMut`].
55
6+
use std::sync::Arc;
7+
68
use vortex_error::{VortexExpect, VortexResult, vortex_ensure, vortex_panic};
79
use vortex_mask::MaskMut;
810

@@ -24,11 +26,11 @@ use crate::{StructVector, Vector, VectorMut, VectorMutOps, VectorOps};
2426
/// use vortex_mask::MaskMut;
2527
///
2628
/// // Create a struct with three fields: nulls, booleans, and integers.
27-
/// let fields = vec![
29+
/// let fields = Box::new([
2830
/// NullVectorMut::new(3).into(),
2931
/// BoolVectorMut::from_iter([true, false, true]).into(),
3032
/// PVectorMut::<i32>::from_iter([10, 20, 30]).into(),
31-
/// ];
33+
/// ]);
3234
///
3335
/// let mut struct_vec = StructVectorMut::new(fields, MaskMut::new_true(3));
3436
/// assert_eq!(struct_vec.len(), 3);
@@ -45,10 +47,10 @@ use crate::{StructVector, Vector, VectorMut, VectorMutOps, VectorOps};
4547
/// };
4648
/// use vortex_mask::MaskMut;
4749
///
48-
/// let fields = vec![
50+
/// let fields = Box::new([
4951
/// NullVectorMut::new(6).into(),
5052
/// PVectorMut::<i32>::from_iter([1, 2, 3, 4, 5, 6]).into(),
51-
/// ];
53+
/// ]);
5254
///
5355
/// let mut struct_vec = StructVectorMut::new(fields, MaskMut::new_true(6));
5456
///
@@ -72,11 +74,11 @@ use crate::{StructVector, Vector, VectorMut, VectorMutOps, VectorOps};
7274
/// use vortex_mask::MaskMut;
7375
/// use vortex_dtype::PTypeDowncast;
7476
///
75-
/// let fields = vec![
77+
/// let fields = Box::new([
7678
/// NullVectorMut::new(3).into(),
7779
/// BoolVectorMut::from_iter([true, false, true]).into(),
7880
/// PVectorMut::<i32>::from_iter([10, 20, 30]).into(),
79-
/// ];
81+
/// ]);
8082
///
8183
/// let struct_vec = StructVectorMut::new(fields, MaskMut::new_true(3));
8284
///
@@ -287,7 +289,7 @@ impl VectorMutOps for StructVectorMut {
287289
.collect();
288290

289291
StructVector {
290-
fields: frozen_fields.into_boxed_slice(),
292+
fields: Arc::new(frozen_fields.into_boxed_slice()),
291293
len: self.len,
292294
validity: self.validity.freeze(),
293295
}
@@ -368,16 +370,15 @@ mod tests {
368370
#[test]
369371
fn test_try_into_mut_and_values() {
370372
let struct_vec = StructVector {
371-
fields: vec![
373+
fields: Arc::new(Box::new([
372374
NullVector::new(5).into(),
373375
BoolVectorMut::from_iter([true, false, true, false, true])
374376
.freeze()
375377
.into(),
376378
PVectorMut::<i32>::from_iter([10, 20, 30, 40, 50])
377379
.freeze()
378380
.into(),
379-
]
380-
.into_boxed_slice(),
381+
])),
381382
len: 5,
382383
validity: Mask::AllTrue(5),
383384
};
@@ -410,12 +411,11 @@ mod tests {
410411
let bool_field_clone = bool_field.clone();
411412

412413
let struct_vec = StructVector {
413-
fields: vec![
414+
fields: Arc::new(Box::new([
414415
NullVector::new(3).into(),
415416
bool_field_clone,
416417
PVectorMut::<i32>::from_iter([1, 2, 3]).freeze().into(),
417-
]
418-
.into_boxed_slice(),
418+
])),
419419
len: 3,
420420
validity: Mask::AllTrue(3),
421421
};
@@ -479,12 +479,11 @@ mod tests {
479479

480480
// Test extend.
481481
let to_extend = StructVector {
482-
fields: vec![
482+
fields: Arc::new(Box::new([
483483
NullVector::new(2).into(),
484484
BoolVectorMut::from_iter([false, true]).freeze().into(),
485485
PVectorMut::<i32>::from_iter([40, 50]).freeze().into(),
486-
]
487-
.into_boxed_slice(),
486+
])),
488487
len: 2,
489488
validity: Mask::AllTrue(2),
490489
};
@@ -651,7 +650,7 @@ mod tests {
651650
}
652651

653652
// Verify field data is preserved.
654-
let mut fields = frozen.into_parts().0.into_vec();
653+
let mut fields = Arc::try_unwrap(frozen.into_parts().0).unwrap().into_vec();
655654

656655
if let Vector::Primitive(prim_vec) = fields.pop().unwrap() {
657656
let prim_vec_mut = prim_vec.try_into_mut().unwrap();

0 commit comments

Comments
 (0)