diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 6da87c38ea1..d40dea14b86 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -25,6 +25,8 @@ jobs: toolchain: nightly-2021-10-24 override: true - uses: Swatinem/rust-cache@v1 + with: + key: key1 - name: Install Miri run: | rustup component add miri diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 43300789307..1dd6d1aaa9e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -77,6 +77,8 @@ jobs: toolchain: nightly-2021-10-24 override: true - uses: Swatinem/rust-cache@v1 + with: + key: key1 - name: Install Miri run: | rustup component add miri @@ -96,6 +98,8 @@ jobs: toolchain: nightly-2021-10-24 override: true - uses: Swatinem/rust-cache@v1 + with: + key: key1 - name: Install Miri run: | rustup component add miri diff --git a/src/compute/comparison/mod.rs b/src/compute/comparison/mod.rs index a3471f3b285..95ee88a17a9 100644 --- a/src/compute/comparison/mod.rs +++ b/src/compute/comparison/mod.rs @@ -240,58 +240,59 @@ macro_rules! compare_scalar { Boolean => { let lhs = lhs.as_any().downcast_ref().unwrap(); let rhs = rhs.as_any().downcast_ref::().unwrap(); - boolean::$op(lhs, rhs.value()) + // validity checked above + boolean::$op(lhs, rhs.value().unwrap()) } Int8 => { let lhs = lhs.as_any().downcast_ref().unwrap(); let rhs = rhs.as_any().downcast_ref::>().unwrap(); - primitive::$op::(lhs, rhs.value()) + primitive::$op::(lhs, rhs.value().unwrap()) } Int16 => { let lhs = lhs.as_any().downcast_ref().unwrap(); let rhs = rhs.as_any().downcast_ref::>().unwrap(); - primitive::$op::(lhs, rhs.value()) + primitive::$op::(lhs, rhs.value().unwrap()) } Int32 | Date32 | Time32(_) | Interval(IntervalUnit::YearMonth) => { let lhs = lhs.as_any().downcast_ref().unwrap(); let rhs = rhs.as_any().downcast_ref::>().unwrap(); - primitive::$op::(lhs, rhs.value()) + primitive::$op::(lhs, rhs.value().unwrap()) } Int64 | Timestamp(_, _) | Date64 | Time64(_) | Duration(_) => { let lhs = lhs.as_any().downcast_ref().unwrap(); let rhs = rhs.as_any().downcast_ref::>().unwrap(); - primitive::$op::(lhs, rhs.value()) + primitive::$op::(lhs, rhs.value().unwrap()) } UInt8 => { let lhs = lhs.as_any().downcast_ref().unwrap(); let rhs = rhs.as_any().downcast_ref::>().unwrap(); - primitive::$op::(lhs, rhs.value()) + primitive::$op::(lhs, rhs.value().unwrap()) } UInt16 => { let lhs = lhs.as_any().downcast_ref().unwrap(); let rhs = rhs.as_any().downcast_ref::>().unwrap(); - primitive::$op::(lhs, rhs.value()) + primitive::$op::(lhs, rhs.value().unwrap()) } UInt32 => { let lhs = lhs.as_any().downcast_ref().unwrap(); let rhs = rhs.as_any().downcast_ref::>().unwrap(); - primitive::$op::(lhs, rhs.value()) + primitive::$op::(lhs, rhs.value().unwrap()) } UInt64 => { let lhs = lhs.as_any().downcast_ref().unwrap(); let rhs = rhs.as_any().downcast_ref::>().unwrap(); - primitive::$op::(lhs, rhs.value()) + primitive::$op::(lhs, rhs.value().unwrap()) } Float16 => unreachable!(), Float32 => { let lhs = lhs.as_any().downcast_ref().unwrap(); let rhs = rhs.as_any().downcast_ref::>().unwrap(); - primitive::$op::(lhs, rhs.value()) + primitive::$op::(lhs, rhs.value().unwrap()) } Float64 => { let lhs = lhs.as_any().downcast_ref().unwrap(); let rhs = rhs.as_any().downcast_ref::>().unwrap(); - primitive::$op::(lhs, rhs.value()) + primitive::$op::(lhs, rhs.value().unwrap()) } Utf8 => { let lhs = lhs.as_any().downcast_ref().unwrap(); @@ -309,7 +310,7 @@ macro_rules! compare_scalar { .as_any() .downcast_ref::>() .unwrap(); - primitive::$op::(lhs, rhs.value()) + primitive::$op::(lhs, rhs.value().unwrap()) } Binary => { let lhs = lhs.as_any().downcast_ref().unwrap(); diff --git a/src/scalar/boolean.rs b/src/scalar/boolean.rs index 74df27534f3..47c3f9a9bc9 100644 --- a/src/scalar/boolean.rs +++ b/src/scalar/boolean.rs @@ -3,32 +3,21 @@ use crate::datatypes::DataType; use super::Scalar; /// The [`Scalar`] implementation of a boolean. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct BooleanScalar { - value: bool, - is_valid: bool, -} - -impl PartialEq for BooleanScalar { - fn eq(&self, other: &Self) -> bool { - self.is_valid == other.is_valid && ((!self.is_valid) | (self.value == other.value)) - } + value: Option, } impl BooleanScalar { /// Returns a new [`BooleanScalar`] #[inline] - pub fn new(v: Option) -> Self { - let is_valid = v.is_some(); - Self { - value: v.unwrap_or_default(), - is_valid, - } + pub fn new(value: Option) -> Self { + Self { value } } - /// The value irrespectively of the validity + /// The value #[inline] - pub fn value(&self) -> bool { + pub fn value(&self) -> Option { self.value } } @@ -41,7 +30,7 @@ impl Scalar for BooleanScalar { #[inline] fn is_valid(&self) -> bool { - self.is_valid + self.value.is_some() } #[inline] diff --git a/src/scalar/primitive.rs b/src/scalar/primitive.rs index 83c78fef643..2d2c827fdf7 100644 --- a/src/scalar/primitive.rs +++ b/src/scalar/primitive.rs @@ -8,26 +8,16 @@ use super::Scalar; /// The implementation of [`Scalar`] for primitive, semantically equivalent to [`Option`] /// with [`DataType`]. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct PrimitiveScalar { - // Not Option because this offers a stabler pointer offset on the struct - value: T, - is_valid: bool, + value: Option, data_type: DataType, } -impl PartialEq for PrimitiveScalar { - fn eq(&self, other: &Self) -> bool { - self.data_type == other.data_type - && self.is_valid == other.is_valid - && ((!self.is_valid) | (self.value == other.value)) - } -} - impl PrimitiveScalar { /// Returns a new [`PrimitiveScalar`]. #[inline] - pub fn new(data_type: DataType, v: Option) -> Self { + pub fn new(data_type: DataType, value: Option) -> Self { if !T::is_valid(&data_type) { Err(ArrowError::InvalidArgumentError(format!( "Type {} does not support logical type {}", @@ -36,17 +26,12 @@ impl PrimitiveScalar { ))) .unwrap() } - let is_valid = v.is_some(); - Self { - value: v.unwrap_or_default(), - is_valid, - data_type, - } + Self { value, data_type } } - /// Returns the value irrespectively of its validity. + /// Returns the optional value. #[inline] - pub fn value(&self) -> T { + pub fn value(&self) -> Option { self.value } @@ -54,12 +39,7 @@ impl PrimitiveScalar { /// # Panic /// This function panics if the `data_type` is not valid for self's physical type `T`. pub fn to(self, data_type: DataType) -> Self { - let v = if self.is_valid { - Some(self.value) - } else { - None - }; - Self::new(data_type, v) + Self::new(data_type, self.value) } } @@ -78,7 +58,7 @@ impl Scalar for PrimitiveScalar { #[inline] fn is_valid(&self) -> bool { - self.is_valid + self.value.is_some() } #[inline] diff --git a/tests/it/scalar/boolean.rs b/tests/it/scalar/boolean.rs index a98b72210ca..9882e5a77ee 100644 --- a/tests/it/scalar/boolean.rs +++ b/tests/it/scalar/boolean.rs @@ -20,7 +20,7 @@ fn equal() { fn basics() { let a = BooleanScalar::new(Some(true)); - assert!(a.value()); + assert_eq!(a.value(), Some(true)); assert_eq!(a.data_type(), &DataType::Boolean); assert!(a.is_valid()); diff --git a/tests/it/scalar/primitive.rs b/tests/it/scalar/primitive.rs index c248689f3ec..3cb26707eb1 100644 --- a/tests/it/scalar/primitive.rs +++ b/tests/it/scalar/primitive.rs @@ -20,9 +20,8 @@ fn equal() { fn basics() { let a = PrimitiveScalar::from(Some(2i32)); - assert_eq!(a.value(), 2i32); + assert_eq!(a.value(), Some(2i32)); assert_eq!(a.data_type(), &DataType::Int32); - assert!(a.is_valid()); let a = a.to(DataType::Date32); assert_eq!(a.data_type(), &DataType::Date32);