Skip to content

Commit e939edc

Browse files
connortsui20connortsui20
committed
fix bugs and clean up
Signed-off-by: Connor Tsui <[email protected]>
1 parent 1eed89b commit e939edc

File tree

16 files changed

+242
-226
lines changed

16 files changed

+242
-226
lines changed

‎vortex-array/src/arrays/bool/vtable/operator.rs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// SPDX-FileCopyrightText: Copyright the Vortex contributors
33

44
use vortex_compute::filter::Filter;
5-
use vortex_error::VortexResult;
5+
use vortex_error::{VortexExpect, VortexResult};
66
use vortex_vector::BoolVector;
77

88
use crate::ArrayRef;
@@ -27,7 +27,11 @@ impl OperatorVTable<BoolVTable> for BoolVTable {
2727
// Note that validity already has the mask applied so we only need to apply it to bits.
2828
let bits = bits.filter(&mask);
2929

30-
Ok(BoolVector::new(bits, validity).into())
30+
Ok(BoolVector::try_new(bits, validity)
31+
.vortex_expect(
32+
"`BoolVector` bits and validity somehow did not have the same length",
33+
)
34+
.into())
3135
}))
3236
}
3337
}

‎vortex-array/src/arrays/primitive/vtable/operator.rs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
use vortex_compute::filter::Filter;
55
use vortex_dtype::match_each_native_ptype;
6-
use vortex_error::VortexResult;
6+
use vortex_error::{VortexExpect, VortexResult};
77
use vortex_vector::PVector;
88

99
use crate::ArrayRef;
@@ -30,7 +30,11 @@ impl OperatorVTable<PrimitiveVTable> for PrimitiveVTable {
3030
// the elements.
3131
let elements = elements.filter(&mask);
3232

33-
Ok(PVector::new(elements, validity).into())
33+
Ok(PVector::try_new(elements, validity)
34+
.vortex_expect(
35+
"`PVector` elements and validity somehow did not have the same length",
36+
)
37+
.into())
3438
}))
3539
})
3640
}

‎vortex-compute/src/filter/bool.rs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
11
// SPDX-License-Identifier: Apache-2.0
22
// SPDX-FileCopyrightText: Copyright the Vortex contributors
33

4+
use vortex_error::VortexExpect;
45
use vortex_mask::Mask;
56
use vortex_vector::{BoolVector, VectorOps};
67

78
use crate::filter::Filter;
89

910
impl Filter for BoolVector {
1011
fn filter(&self, mask: &Mask) -> Self {
11-
Self::new(self.bits().filter(mask), self.validity().filter(mask))
12+
Self::try_new(self.bits().filter(mask), self.validity().filter(mask)).vortex_expect(
13+
"`BoolVector` bits and validity filter somehow did not have the same length",
14+
)
1215
}
1316
}

‎vortex-compute/src/logical/and.rs‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
use std::ops::BitAnd;
55

6+
use vortex_error::VortexExpect;
67
use vortex_vector::{BoolVector, VectorOps};
78

89
use crate::logical::LogicalAnd;
@@ -12,10 +13,11 @@ impl LogicalAnd for &BoolVector {
1213
type Output = BoolVector;
1314

1415
fn and(self, other: &BoolVector) -> BoolVector {
15-
BoolVector::new(
16+
BoolVector::try_new(
1617
self.bits().bitand(other.bits()),
1718
self.validity().bitand(other.validity()),
1819
)
20+
.vortex_expect("`BoolVector` bits and validity somehow did not have the same length")
1921
}
2022
}
2123

@@ -37,17 +39,17 @@ mod tests {
3739

3840
#[test]
3941
fn test_and_basic() {
40-
let left = BoolVector::new(bitbuffer![1 1 0 0], Mask::new_true(4));
41-
let right = BoolVector::new(bitbuffer![1 0 1 0], Mask::new_true(4));
42+
let left = BoolVector::try_new(bitbuffer![1 1 0 0], Mask::new_true(4)).unwrap();
43+
let right = BoolVector::try_new(bitbuffer![1 0 1 0], Mask::new_true(4)).unwrap();
4244

4345
let result = left.and(&right);
4446
assert_eq!(result.bits(), &bitbuffer![1 0 0 0]);
4547
}
4648

4749
#[test]
4850
fn test_and_with_nulls() {
49-
let left = BoolVector::new(bitbuffer![1 0], Mask::from(bitbuffer![1 0]));
50-
let right = BoolVector::new(bitbuffer![1 1], Mask::new_true(2));
51+
let left = BoolVector::try_new(bitbuffer![1 0], Mask::from(bitbuffer![1 0])).unwrap();
52+
let right = BoolVector::try_new(bitbuffer![1 1], Mask::new_true(2)).unwrap();
5153

5254
let result = left.and(&right);
5355
// Validity is AND'd, so if either side is null, result is null

‎vortex-compute/src/logical/and_kleene.rs‎

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
use std::ops::{BitAnd, BitOr, Not};
55

66
use vortex_buffer::BitBuffer;
7+
use vortex_error::VortexExpect;
78
use vortex_mask::Mask;
89
use vortex_vector::{BoolVector, VectorOps};
910

@@ -15,56 +16,74 @@ impl LogicalAndKleene for &BoolVector {
1516
fn and_kleene(self, rhs: Self) -> Self::Output {
1617
match (self.validity(), rhs.validity()) {
1718
(Mask::AllTrue(_), Mask::AllTrue(_)) => {
18-
BoolVector::new(self.bits().bitand(rhs.bits()), Mask::new_true(self.len()))
19+
BoolVector::try_new(self.bits().bitand(rhs.bits()), Mask::new_true(self.len()))
20+
.vortex_expect(
21+
"`BoolVector` bits and validity somehow did not have the same length",
22+
)
1923
}
2024
(Mask::AllTrue(_), Mask::AllFalse(_)) => {
2125
// self valid, rhs all null
2226
// Result: false where self is false (valid), null where self is true
2327
let result_bits = BitBuffer::new_unset(self.len());
2428
let validity = self.bits().not(); // valid where self is false
25-
BoolVector::new(result_bits, Mask::from(validity))
29+
BoolVector::try_new(result_bits, Mask::from(validity)).vortex_expect(
30+
"`BoolVector` bits and validity somehow did not have the same length",
31+
)
2632
}
2733
(Mask::AllFalse(_), Mask::AllTrue(_)) => {
2834
// self all null, rhs valid
2935
// Result: false where rhs is false (valid), null where rhs is true
3036
let result_bits = BitBuffer::new_unset(self.len());
3137
let validity = rhs.bits().not(); // valid where rhs is false
32-
BoolVector::new(result_bits, Mask::from(validity))
38+
BoolVector::try_new(result_bits, Mask::from(validity)).vortex_expect(
39+
"`BoolVector` bits and validity somehow did not have the same length",
40+
)
3341
}
3442
(Mask::AllFalse(_), Mask::AllFalse(_)) => {
3543
// All values are null
36-
BoolVector::new(
44+
BoolVector::try_new(
3745
BitBuffer::new_unset(self.len()),
3846
Mask::new_false(self.len()),
3947
)
48+
.vortex_expect(
49+
"`BoolVector` bits and validity somehow did not have the same length",
50+
)
4051
}
4152
(Mask::Values(lv), Mask::AllTrue(_)) => {
4253
// self partial validity, rhs all valid
4354
// Result valid where self valid OR self is null but rhs is false
4455
let result_bits = self.bits().bitand(rhs.bits());
4556
let validity = lv.bit_buffer().bitor(&rhs.bits().not());
46-
BoolVector::new(result_bits, Mask::from(validity))
57+
BoolVector::try_new(result_bits, Mask::from(validity)).vortex_expect(
58+
"`BoolVector` bits and validity somehow did not have the same length",
59+
)
4760
}
4861
(Mask::AllTrue(_), Mask::Values(rv)) => {
4962
// self all valid, rhs partial validity
5063
// Result valid where rhs valid OR rhs is null but self is false
5164
let result_bits = self.bits().bitand(rhs.bits());
5265
let validity = rv.bit_buffer().bitor(&self.bits().not());
53-
BoolVector::new(result_bits, Mask::from(validity))
66+
BoolVector::try_new(result_bits, Mask::from(validity)).vortex_expect(
67+
"`BoolVector` bits and validity somehow did not have the same length",
68+
)
5469
}
5570
(Mask::Values(lv), Mask::AllFalse(_)) => {
5671
// self partial validity, rhs all null
5772
// Result: false where self is false (valid), null otherwise
5873
let result_bits = BitBuffer::new_unset(self.len());
5974
let validity = lv.bit_buffer().bitand(&self.bits().not());
60-
BoolVector::new(result_bits, Mask::from(validity))
75+
BoolVector::try_new(result_bits, Mask::from(validity)).vortex_expect(
76+
"`BoolVector` bits and validity somehow did not have the same length",
77+
)
6178
}
6279
(Mask::AllFalse(_), Mask::Values(rv)) => {
6380
// self all null, rhs partial validity
6481
// Result: false where rhs is false (valid), null otherwise
6582
let result_bits = BitBuffer::new_unset(self.len());
6683
let validity = rv.bit_buffer().bitand(&rhs.bits().not());
67-
BoolVector::new(result_bits, Mask::from(validity))
84+
BoolVector::try_new(result_bits, Mask::from(validity)).vortex_expect(
85+
"`BoolVector` bits and validity somehow did not have the same length",
86+
)
6887
}
6988
(Mask::Values(lv), Mask::Values(rv)) => {
7089
// Both have partial validity
@@ -88,7 +107,9 @@ impl LogicalAndKleene for &BoolVector {
88107
let validity = both_valid
89108
.bitor(&self_null_rhs_false)
90109
.bitor(&rhs_null_self_false);
91-
BoolVector::new(result_bits, Mask::from(validity))
110+
BoolVector::try_new(result_bits, Mask::from(validity)).vortex_expect(
111+
"`BoolVector` bits and validity somehow did not have the same length",
112+
)
92113
}
93114
}
94115
}
@@ -113,8 +134,8 @@ mod tests {
113134
#[test]
114135
fn test_and_kleene_all_valid() {
115136
// When both sides are all valid, behaves like regular AND
116-
let left = BoolVector::new(bitbuffer![1 0 1], Mask::new_true(3));
117-
let right = BoolVector::new(bitbuffer![1 1 0], Mask::new_true(3));
137+
let left = BoolVector::try_new(bitbuffer![1 0 1], Mask::new_true(3)).unwrap();
138+
let right = BoolVector::try_new(bitbuffer![1 1 0], Mask::new_true(3)).unwrap();
118139

119140
let result = left.and_kleene(&right);
120141
assert_eq!(result.bits(), &bitbuffer![1 0 0]);
@@ -124,8 +145,8 @@ mod tests {
124145
#[test]
125146
fn test_and_kleene_all_null() {
126147
// When both are null, result is all null
127-
let left = BoolVector::new(bitbuffer![1 1], Mask::new_false(2));
128-
let right = BoolVector::new(bitbuffer![1 1], Mask::new_false(2));
148+
let left = BoolVector::try_new(bitbuffer![1 1], Mask::new_false(2)).unwrap();
149+
let right = BoolVector::try_new(bitbuffer![1 1], Mask::new_false(2)).unwrap();
129150

130151
let result = left.and_kleene(&right);
131152
assert_eq!(result.validity(), &Mask::new_false(2));
@@ -134,8 +155,8 @@ mod tests {
134155
#[test]
135156
fn test_and_kleene_false_and_null() {
136157
// false AND null = false (Kleene logic)
137-
let left = BoolVector::new(bitbuffer![0], Mask::new_true(1));
138-
let right = BoolVector::new(bitbuffer![1], Mask::new_false(1));
158+
let left = BoolVector::try_new(bitbuffer![0], Mask::new_true(1)).unwrap();
159+
let right = BoolVector::try_new(bitbuffer![1], Mask::new_false(1)).unwrap();
139160

140161
let result = left.and_kleene(&right);
141162
assert_eq!(result.bits(), &bitbuffer![0]);

‎vortex-compute/src/logical/and_not.rs‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
use std::ops::BitAnd;
55

6+
use vortex_error::VortexExpect;
67
use vortex_vector::{BoolVector, VectorOps};
78

89
use crate::logical::LogicalAndNot;
@@ -12,10 +13,11 @@ impl LogicalAndNot for &BoolVector {
1213
type Output = BoolVector;
1314

1415
fn and_not(self, other: &BoolVector) -> BoolVector {
15-
BoolVector::new(
16+
BoolVector::try_new(
1617
self.bits().bitand_not(other.bits()),
1718
self.validity().bitand(other.validity()),
1819
)
20+
.vortex_expect("`BoolVector` bits and validity somehow did not have the same length")
1921
}
2022
}
2123

@@ -38,8 +40,8 @@ mod tests {
3840
#[test]
3941
fn test_and_not_basic() {
4042
// left AND (NOT right)
41-
let left = BoolVector::new(bitbuffer![1 1 0 0], Mask::new_true(4));
42-
let right = BoolVector::new(bitbuffer![1 0 1 0], Mask::new_true(4));
43+
let left = BoolVector::try_new(bitbuffer![1 1 0 0], Mask::new_true(4)).unwrap();
44+
let right = BoolVector::try_new(bitbuffer![1 0 1 0], Mask::new_true(4)).unwrap();
4345

4446
let result = left.and_not(&right);
4547
// 1 & !1 = 0, 1 & !0 = 1, 0 & !1 = 0, 0 & !0 = 0
@@ -48,8 +50,8 @@ mod tests {
4850

4951
#[test]
5052
fn test_and_not_all_true() {
51-
let left = BoolVector::new(bitbuffer![1 1], Mask::new_true(2));
52-
let right = BoolVector::new(bitbuffer![1 1], Mask::new_true(2));
53+
let left = BoolVector::try_new(bitbuffer![1 1], Mask::new_true(2)).unwrap();
54+
let right = BoolVector::try_new(bitbuffer![1 1], Mask::new_true(2)).unwrap();
5355

5456
let result = left.and_not(&right);
5557
assert_eq!(result.bits(), &bitbuffer![0 0]);

‎vortex-compute/src/logical/not.rs‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
use std::ops::Not;
55

6+
use vortex_error::VortexExpect;
67
use vortex_vector::{BoolVector, BoolVectorMut, VectorOps};
78

89
use crate::logical::LogicalNot;
@@ -11,7 +12,8 @@ impl LogicalNot for &BoolVector {
1112
type Output = BoolVector;
1213

1314
fn not(self) -> <Self as LogicalNot>::Output {
14-
BoolVector::new(self.bits().not(), self.validity().clone())
15+
BoolVector::try_new(self.bits().not(), self.validity().clone())
16+
.vortex_expect("`BoolVector` bits and validity somehow did not have the same length")
1517
}
1618
}
1719

@@ -25,7 +27,8 @@ impl LogicalNot for BoolVector {
2527
Ok(bits) => bits.not().freeze(),
2628
Err(bits) => (&bits).not(),
2729
};
28-
BoolVector::new(bits, validity)
30+
BoolVector::try_new(bits, validity)
31+
.vortex_expect("`BoolVector` bits and validity somehow did not have the same length")
2932
}
3033
}
3134

@@ -49,7 +52,7 @@ mod tests {
4952

5053
#[test]
5154
fn test_not_basic() {
52-
let vec = BoolVector::new(bitbuffer![1 0 1 0], Mask::new_true(4));
55+
let vec = BoolVector::try_new(bitbuffer![1 0 1 0], Mask::new_true(4)).unwrap();
5356

5457
let result = vec.not();
5558
assert_eq!(result.bits(), &bitbuffer![0 1 0 1]);
@@ -58,7 +61,7 @@ mod tests {
5861

5962
#[test]
6063
fn test_not_owned() {
61-
let vec = BoolVector::new(bitbuffer![1 1], Mask::new_true(2));
64+
let vec = BoolVector::try_new(bitbuffer![1 1], Mask::new_true(2)).unwrap();
6265

6366
let result = vec.not();
6467
assert_eq!(result.bits(), &bitbuffer![0 0]);

‎vortex-compute/src/logical/or.rs‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
use std::ops::{BitAnd, BitOr};
55

6+
use vortex_error::VortexExpect;
67
use vortex_vector::{BoolVector, VectorOps};
78

89
use crate::logical::LogicalOr;
@@ -12,10 +13,11 @@ impl LogicalOr for &BoolVector {
1213
type Output = BoolVector;
1314

1415
fn or(self, other: &BoolVector) -> BoolVector {
15-
BoolVector::new(
16+
BoolVector::try_new(
1617
self.bits().bitor(other.bits()),
1718
self.validity().bitand(other.validity()),
1819
)
20+
.vortex_expect("`BoolVector` bits and validity somehow did not have the same length")
1921
}
2022
}
2123

@@ -37,17 +39,17 @@ mod tests {
3739

3840
#[test]
3941
fn test_or_basic() {
40-
let left = BoolVector::new(bitbuffer![1 1 0 0], Mask::new_true(4));
41-
let right = BoolVector::new(bitbuffer![1 0 1 0], Mask::new_true(4));
42+
let left = BoolVector::try_new(bitbuffer![1 1 0 0], Mask::new_true(4)).unwrap();
43+
let right = BoolVector::try_new(bitbuffer![1 0 1 0], Mask::new_true(4)).unwrap();
4244

4345
let result = left.or(&right);
4446
assert_eq!(result.bits(), &bitbuffer![1 1 1 0]);
4547
}
4648

4749
#[test]
4850
fn test_or_with_nulls() {
49-
let left = BoolVector::new(bitbuffer![0 1], Mask::from(bitbuffer![0 1]));
50-
let right = BoolVector::new(bitbuffer![0 0], Mask::new_true(2));
51+
let left = BoolVector::try_new(bitbuffer![0 1], Mask::from(bitbuffer![0 1])).unwrap();
52+
let right = BoolVector::try_new(bitbuffer![0 0], Mask::new_true(2)).unwrap();
5153

5254
let result = left.or(&right);
5355
// Validity is AND'd, so if either side is null, result is null

0 commit comments

Comments
 (0)