Skip to content
12 changes: 2 additions & 10 deletions encodings/bytebool/src/stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ impl StatisticsVTable<ByteBoolArray> for ByteBoolEncoding {
#[cfg(test)]
mod tests {
use vortex_array::stats::ArrayStatistics;
use vortex_dtype::{DType, Nullability};
use vortex_scalar::Scalar;

use super::*;

Expand Down Expand Up @@ -90,14 +88,8 @@ mod tests {
assert!(!bool_arr.statistics().compute_is_strict_sorted().unwrap());
assert!(bool_arr.statistics().compute_is_sorted().unwrap());
assert!(bool_arr.statistics().compute_is_constant().unwrap());
assert_eq!(
bool_arr.statistics().compute(Stat::Min).unwrap(),
Scalar::null(DType::Bool(Nullability::Nullable))
);
assert_eq!(
bool_arr.statistics().compute(Stat::Max).unwrap(),
Scalar::null(DType::Bool(Nullability::Nullable))
);
assert_eq!(bool_arr.statistics().compute(Stat::Min), None);
assert_eq!(bool_arr.statistics().compute(Stat::Max), None);
assert_eq!(bool_arr.statistics().compute_run_count().unwrap(), 1);
assert_eq!(bool_arr.statistics().compute_true_count().unwrap(), 0);
}
Expand Down
14 changes: 3 additions & 11 deletions vortex-array/src/array/bool/stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,7 @@ impl BoolStatsAccumulator {
#[cfg(test)]
mod test {
use arrow_buffer::BooleanBuffer;
use vortex_dtype::Nullability::Nullable;
use vortex_dtype::{DType, Nullability};
use vortex_scalar::Scalar;
use vortex_dtype::Nullability;

use crate::array::BoolArray;
use crate::stats::{ArrayStatistics, Stat};
Expand Down Expand Up @@ -278,14 +276,8 @@ mod test {
assert!(!bool_arr.statistics().compute_is_strict_sorted().unwrap());
assert!(bool_arr.statistics().compute_is_sorted().unwrap());
assert!(bool_arr.statistics().compute_is_constant().unwrap());
assert_eq!(
bool_arr.statistics().compute(Stat::Min).unwrap(),
Scalar::null(DType::Bool(Nullable))
);
assert_eq!(
bool_arr.statistics().compute(Stat::Max).unwrap(),
Scalar::null(DType::Bool(Nullable))
);
assert_eq!(bool_arr.statistics().compute(Stat::Min), None);
assert_eq!(bool_arr.statistics().compute(Stat::Max), None);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were just wrong, min and max are never null.

assert_eq!(bool_arr.statistics().compute_run_count().unwrap(), 1);
assert_eq!(bool_arr.statistics().compute_true_count().unwrap(), 0);
assert_eq!(bool_arr.statistics().compute_null_count().unwrap(), 5);
Expand Down
6 changes: 2 additions & 4 deletions vortex-array/src/array/primitive/stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,6 @@ impl<T: PStatsType> BitWidthAccumulator<T> {

#[cfg(test)]
mod test {
use vortex_dtype::{DType, Nullability, PType};
use vortex_scalar::Scalar;

use crate::array::primitive::PrimitiveArray;
Expand Down Expand Up @@ -402,8 +401,7 @@ mod test {
let arr = PrimitiveArray::from_option_iter([Option::<i32>::None, None, None]);
let min: Option<Scalar> = arr.statistics().compute(Stat::Min);
let max: Option<Scalar> = arr.statistics().compute(Stat::Max);
let null_i32 = Scalar::null(DType::Primitive(PType::I32, Nullability::Nullable));
assert_eq!(min, Some(null_i32.clone()));
assert_eq!(max, Some(null_i32));
assert_eq!(min, None);
assert_eq!(max, None);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was just wrong. I'm not really sure why it worked.

}
}
4 changes: 2 additions & 2 deletions vortex-array/src/data/statistics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::sync::Arc;
use enum_iterator::all;
use itertools::Itertools;
use vortex_dtype::{DType, Nullability, PType};
use vortex_error::vortex_panic;
use vortex_error::{vortex_panic, VortexExpect as _};
use vortex_scalar::{Scalar, ScalarValue};

use crate::data::InnerArrayData;
Expand Down Expand Up @@ -118,7 +118,7 @@ impl Statistics for ArrayData {
let s = self
.encoding()
.compute_statistics(self, stat)
.ok()?
.vortex_expect("compute_statistics must not fail")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debatable but it's really annoying to debug compute_statistics errors without this.

.get(stat)
.cloned();

Expand Down
8 changes: 4 additions & 4 deletions vortex-array/src/stats/statsset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ impl StatsSet {
/// an array consisting entirely of [null](vortex_dtype::DType::Null) values.
pub fn nulls(len: usize, dtype: &DType) -> Self {
let mut stats = Self::new_unchecked(vec![
(Stat::Min, Scalar::null(dtype.clone())),
(Stat::Max, Scalar::null(dtype.clone())),
(Stat::RunCount, 1.into()),
(Stat::NullCount, len.into()),
]);
Expand Down Expand Up @@ -86,8 +84,10 @@ impl StatsSet {
stats.set(Stat::TrueCount, true_count);
}

stats.set(Stat::Min, scalar.clone());
stats.set(Stat::Max, scalar.clone());
if !scalar.is_null() {
stats.set(Stat::Min, scalar.clone());
stats.set(Stat::Max, scalar.clone());
}

stats
}
Expand Down
17 changes: 14 additions & 3 deletions vortex-scalar/src/binary.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use vortex_buffer::ByteBuffer;
use vortex_dtype::{DType, Nullability};
use vortex_error::{vortex_bail, vortex_err, VortexError, VortexResult};
use vortex_error::{vortex_bail, vortex_err, VortexError, VortexExpect as _, VortexResult};

use crate::value::{InnerScalarValue, ScalarValue};
use crate::Scalar;
Expand All @@ -20,8 +20,19 @@ impl<'a> BinaryScalar<'a> {
self.value.as_ref().cloned()
}

pub fn cast(&self, _dtype: &DType) -> VortexResult<Scalar> {
todo!()
pub(crate) fn cast(&self, dtype: &DType) -> VortexResult<Scalar> {
if !matches!(dtype, DType::Binary(..)) {
vortex_bail!("Can't cast binary to {}", dtype)
}
Ok(Scalar::new(
dtype.clone(),
ScalarValue(InnerScalarValue::Buffer(
self.value
.as_ref()
.vortex_expect("nullness handled in Scalar::cast")
.clone(),
)),
))
}
}

Expand Down
16 changes: 8 additions & 8 deletions vortex-scalar/src/bool.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use vortex_dtype::Nullability::NonNullable;
use vortex_dtype::{DType, Nullability};
use vortex_error::{vortex_bail, vortex_err, VortexError, VortexResult};
use vortex_error::{vortex_bail, vortex_err, VortexError, VortexExpect as _, VortexResult};

use crate::value::ScalarValue;
use crate::{InnerScalarValue, Scalar};
Expand All @@ -20,14 +20,14 @@ impl<'a> BoolScalar<'a> {
self.value
}

pub fn cast(&self, dtype: &DType) -> VortexResult<Scalar> {
match dtype {
DType::Bool(_) => Ok(Scalar::bool(
self.value().ok_or_else(|| vortex_err!("not a bool"))?,
dtype.nullability(),
)),
_ => vortex_bail!("Can't cast {} to bool", dtype),
pub(crate) fn cast(&self, dtype: &DType) -> VortexResult<Scalar> {
if !matches!(dtype, DType::Bool(..)) {
vortex_bail!("Can't cast bool to {}", dtype)
}
Ok(Scalar::bool(
self.value.vortex_expect("nullness handled in Scalar::cast"),
dtype.nullability(),
))
}

pub fn invert(self) -> BoolScalar<'a> {
Expand Down
51 changes: 33 additions & 18 deletions vortex-scalar/src/extension.rs
Original file line number Diff line number Diff line change
@@ -1,42 +1,57 @@
use std::sync::Arc;

use vortex_dtype::{DType, ExtDType};
use vortex_error::{vortex_bail, vortex_panic, VortexError, VortexResult};
use vortex_error::{vortex_bail, VortexError, VortexResult};

use crate::value::ScalarValue;
use crate::Scalar;

pub struct ExtScalar<'a> {
dtype: &'a DType,
ext_dtype: &'a ExtDType,
value: &'a ScalarValue,
}

impl<'a> ExtScalar<'a> {
pub fn try_new(dtype: &'a DType, value: &'a ScalarValue) -> VortexResult<Self> {
if !matches!(dtype, DType::Extension(..)) {
let DType::Extension(ext_dtype) = dtype else {
vortex_bail!("Expected extension scalar, found {}", dtype)
}

Ok(Self { dtype, value })
}
};

#[inline]
pub fn dtype(&self) -> &'a DType {
self.dtype
Ok(Self { ext_dtype, value })
}

/// Returns the storage scalar of the extension scalar.
pub fn storage(&self) -> Scalar {
let storage_dtype = if let DType::Extension(ext_dtype) = self.dtype() {
ext_dtype.storage_dtype().clone()
} else {
vortex_panic!("Expected extension DType: {}", self.dtype());
};
Scalar::new(storage_dtype, self.value.clone())
Scalar::new(self.ext_dtype.storage_dtype().clone(), self.value.clone())
}

pub fn cast(&self, _dtype: &DType) -> VortexResult<Scalar> {
todo!()
pub(crate) fn cast(&self, dtype: &DType) -> VortexResult<Scalar> {
if self.value.is_null() && !dtype.is_nullable() {
vortex_bail!(
"cannot cast extension dtype with id {} and storage type {} to {}",
self.ext_dtype.id(),
self.ext_dtype.storage_dtype(),
dtype
);
}

if self.ext_dtype.storage_dtype().eq_ignore_nullability(dtype) {
// Casting from an extension type to the underlying storage type is OK.
return Ok(Scalar::new(dtype.clone(), self.value.clone()));
}

if let DType::Extension(ext_dtype) = dtype {
if self.ext_dtype.eq_ignore_nullability(ext_dtype) {
return Ok(Scalar::new(dtype.clone(), self.value.clone()));
}
}

vortex_bail!(
"cannot cast extension dtype with id {} and storage type {} to {}",
self.ext_dtype.id(),
self.ext_dtype.storage_dtype(),
dtype
);
}
}

Expand Down
Loading