Skip to content

Commit e6558ac

Browse files
myrrcmyrrc
committed
Don't validate Primitive/Bool scalars
Signed-off-by: Mikhail Kot <mikhail@spiraldb.com>
1 parent 5c1671e commit e6558ac

4 files changed

Lines changed: 51 additions & 39 deletions

File tree

‎vortex-array/src/scalar/convert/into_scalar.rs‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -35,21 +35,26 @@ macro_rules! impl_into_scalar {
3535

3636
impl From<$ty> for Scalar {
3737
fn from(value: $ty) -> Self {
38-
Self::try_new(
39-
DType::$variant(Nullability::NonNullable),
40-
Some(ScalarValue::from(value)),
41-
)
42-
.vortex_expect("unable to construct a `Scalar`")
38+
// SAFETY: dtype and value are built from $variant
39+
unsafe {
40+
Self::new_unchecked(
41+
DType::$variant(Nullability::NonNullable),
42+
Some(ScalarValue::from(value)),
43+
)
44+
}
4345
}
4446
}
4547

4648
impl From<Option<$ty>> for Scalar {
4749
fn from(value: Option<$ty>) -> Self {
48-
Self::try_new(
49-
DType::$variant(Nullability::Nullable),
50-
value.map(ScalarValue::from),
51-
)
52-
.vortex_expect("unable to construct a `Scalar`")
50+
// SAFETY: dtype and value are built from $variant. DType is
51+
// nullable, so None is valid
52+
unsafe {
53+
Self::new_unchecked(
54+
DType::$variant(Nullability::Nullable),
55+
value.map(ScalarValue::from),
56+
)
57+
}
5358
}
5459
}
5560
};

‎vortex-array/src/scalar/convert/primitive.rs‎

Lines changed: 32 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
//! Conversions for [`PrimitiveScalar`]s.
55
66
use vortex_error::VortexError;
7-
use vortex_error::VortexExpect;
87
use vortex_error::VortexResult;
98
use vortex_error::vortex_err;
109

@@ -104,26 +103,29 @@ macro_rules! primitive_scalar {
104103
/// Non-nullable `Into<Scalar>` implementation for T.
105104
impl From<$T> for Scalar {
106105
fn from(value: $T) -> Self {
107-
Scalar::try_new(
108-
DType::Primitive(<$T>::PTYPE, Nullability::NonNullable),
109-
Some(ScalarValue::Primitive(value.into())),
110-
)
111-
.vortex_expect(
112-
"somehow unable to construct a primitive `Scalar` from a native type",
113-
)
106+
// SAFETY: Primitive dtype of T::PTYPE by definition always
107+
// matches Primitive value built from T
108+
unsafe {
109+
Scalar::new_unchecked(
110+
DType::Primitive(<$T>::PTYPE, Nullability::NonNullable),
111+
Some(ScalarValue::Primitive(value.into())),
112+
)
113+
}
114114
}
115115
}
116116

117117
/// Nullable `Into<Scalar>` implementation for T.
118118
impl From<Option<$T>> for Scalar {
119119
fn from(value: Option<$T>) -> Self {
120-
Scalar::try_new(
121-
DType::Primitive(<$T>::PTYPE, Nullability::Nullable),
122-
value.map(|value| ScalarValue::Primitive(value.into())),
123-
)
124-
.vortex_expect(
125-
"somehow unable to construct a primitive `Scalar` from a native type",
126-
)
120+
// SAFETY: Primitive dtype of T::PTYPE by definition always
121+
// matches Primitive value built from T. dtype is nullable so
122+
// None is valid
123+
unsafe {
124+
Scalar::new_unchecked(
125+
DType::Primitive(<$T>::PTYPE, Nullability::Nullable),
126+
value.map(|value| ScalarValue::Primitive(value.into())),
127+
)
128+
}
127129
}
128130
}
129131
};
@@ -199,20 +201,25 @@ impl From<usize> for ScalarValue {
199201

200202
impl From<usize> for Scalar {
201203
fn from(value: usize) -> Self {
202-
Scalar::try_new(
203-
DType::Primitive(PType::U64, Nullability::NonNullable),
204-
Some(ScalarValue::Primitive((value as u64).into())),
205-
)
206-
.vortex_expect("somehow unable to construct a primitive `Scalar` from a native type")
204+
// SAFETY: U64 always matches a primitive value built from u64
205+
unsafe {
206+
Scalar::new_unchecked(
207+
DType::Primitive(PType::U64, Nullability::NonNullable),
208+
Some(ScalarValue::Primitive((value as u64).into())),
209+
)
210+
}
207211
}
208212
}
209213

210214
impl From<Option<usize>> for Scalar {
211215
fn from(value: Option<usize>) -> Self {
212-
Scalar::try_new(
213-
DType::Primitive(PType::U64, Nullability::Nullable),
214-
value.map(|value| ScalarValue::Primitive((value as u64).into())),
215-
)
216-
.vortex_expect("somehow unable to construct a primitive `Scalar` from a native type")
216+
// SAFETY: U64 always matches a primitive value built from u64. Dtype
217+
// is nullable so None is valid.
218+
unsafe {
219+
Scalar::new_unchecked(
220+
DType::Primitive(PType::U64, Nullability::Nullable),
221+
value.map(|value| ScalarValue::Primitive((value as u64).into())),
222+
)
223+
}
217224
}
218225
}

‎vortex-mask/src/lib.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -543,9 +543,9 @@ impl Mask {
543543
Bound::Unbounded => self.len(),
544544
};
545545

546-
assert!(start <= end);
547-
assert!(start <= self.len());
548-
assert!(end <= self.len());
546+
debug_assert!(start <= end);
547+
debug_assert!(start <= self.len());
548+
debug_assert!(end <= self.len());
549549
let len = end - start;
550550

551551
// Slicing the whole mask is the identity. `Self` is `Arc`-backed, so the clone is cheap

‎vortex-session/src/session.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
//! read never takes a lock, it can never deadlock, and because it never holds a lock across the
1212
//! returned reference there is no reader/writer contention.
1313
//!
14-
//! * **Writes** ([`VortexSession::with_some`], [`SessionExt::get`] on a missing default) are
14+
//! * **Writes** ([`VortexSession::with_some`], [`SessionExt::get`] on a missing default are
1515
//! copy-on-write: the map is cloned, the change applied to the private copy, and the new map
1616
//! atomically published. The value is constructed *before* the map is updated, so no user code
1717
//! (in particular, no `Default::default` implementation) ever runs while a lock is held. This is

0 commit comments

Comments
 (0)