diff --git a/vortex-array/src/array/constant/compute/search_sorted.rs b/vortex-array/src/array/constant/compute/search_sorted.rs index fdafe4b22a..9e7bb1ffb4 100644 --- a/vortex-array/src/array/constant/compute/search_sorted.rs +++ b/vortex-array/src/array/constant/compute/search_sorted.rs @@ -1,11 +1,11 @@ use std::cmp::Ordering; -use vortex_error::VortexResult; +use vortex_error::{vortex_err, VortexResult}; use vortex_scalar::Scalar; use crate::array::{ConstantArray, ConstantEncoding}; use crate::compute::{SearchResult, SearchSortedFn, SearchSortedSide}; -use crate::ArrayLen; +use crate::{ArrayDType, ArrayLen}; impl SearchSortedFn for ConstantEncoding { fn search_sorted( @@ -14,7 +14,13 @@ impl SearchSortedFn for ConstantEncoding { value: &Scalar, side: SearchSortedSide, ) -> VortexResult { - match array.scalar().partial_cmp(value).unwrap_or(Ordering::Less) { + match array.scalar().partial_cmp(value).ok_or_else(|| { + vortex_err!( + "Cannot search sorted array type {} with value type {}", + array.dtype(), + value.dtype() + ) + })? { Ordering::Greater => Ok(SearchResult::NotFound(0)), Ordering::Less => Ok(SearchResult::NotFound(array.len())), Ordering::Equal => match side { diff --git a/vortex-array/src/array/sparse/mod.rs b/vortex-array/src/array/sparse/mod.rs index 39a5092ff4..3a758edfa1 100644 --- a/vortex-array/src/array/sparse/mod.rs +++ b/vortex-array/src/array/sparse/mod.rs @@ -247,9 +247,10 @@ mod test { use vortex_dtype::Nullability::Nullable; use vortex_dtype::{DType, PType}; use vortex_error::VortexError; - use vortex_scalar::Scalar; + use vortex_scalar::{PrimitiveScalar, Scalar}; use crate::array::sparse::SparseArray; + use crate::array::ConstantArray; use crate::compute::{scalar_at, slice, try_cast}; use crate::validity::ArrayValidity; use crate::{ArrayData, IntoArrayData, IntoArrayVariant}; @@ -295,6 +296,26 @@ mod test { assert_eq!(stop, 10); } + #[test] + pub fn test_scalar_at_again() { + let arr = SparseArray::try_new( + ConstantArray::new(10u32, 1).into_array(), + ConstantArray::new(Scalar::primitive(1234u32, Nullable), 1).into_array(), + 100, + Scalar::null(DType::Primitive(PType::U32, Nullable)), + ) + .unwrap(); + + assert_eq!( + PrimitiveScalar::try_from(&scalar_at(&arr, 10).unwrap()) + .unwrap() + .typed_value::(), + Some(1234) + ); + assert!(scalar_at(&arr, 0).unwrap().is_null()); + assert!(scalar_at(&arr, 99).unwrap().is_null()); + } + #[test] pub fn scalar_at_sliced() { let sliced = slice(sparse_array(nullable_fill()), 2, 7).unwrap(); diff --git a/vortex-array/src/compute/search_sorted.rs b/vortex-array/src/compute/search_sorted.rs index 4ff193bbae..3be8733072 100644 --- a/vortex-array/src/compute/search_sorted.rs +++ b/vortex-array/src/compute/search_sorted.rs @@ -213,7 +213,12 @@ pub fn search_sorted>( target: T, side: SearchSortedSide, ) -> VortexResult { - let scalar = target.into().cast(array.dtype())?; + let Ok(scalar) = target.into().cast(array.dtype()) else { + // If the cast fails, then the search value must be higher than the highest value in + // the array. + return Ok(SearchResult::NotFound(array.len())); + }; + if scalar.is_null() { vortex_bail!("Search sorted with null value is not supported"); } @@ -473,7 +478,11 @@ impl Len for [T] { #[cfg(test)] mod test { + use crate::array::PrimitiveArray; + use crate::compute::search_sorted; use crate::compute::search_sorted::{SearchResult, SearchSorted, SearchSortedSide}; + use crate::validity::Validity; + use crate::IntoArrayData; #[test] fn left_side_equal() { @@ -522,4 +531,15 @@ mod test { assert_eq!(arr[res.to_index() - 1], 9); assert_eq!(res, SearchResult::Found(13)); } + + #[test] + fn failed_cast() { + let arr = PrimitiveArray::from_vec( + vec![0u8, 1, 2, 3, 4, 5, 6, 7, 8, 9, 9, 9, 9], + Validity::NonNullable, + ) + .into_array(); + let res = search_sorted(&arr, 256, SearchSortedSide::Left).unwrap(); + assert_eq!(res, SearchResult::NotFound(arr.len())); + } } diff --git a/vortex-sampling-compressor/src/compressors/alp.rs b/vortex-sampling-compressor/src/compressors/alp.rs index df1f506e01..d0495897db 100644 --- a/vortex-sampling-compressor/src/compressors/alp.rs +++ b/vortex-sampling-compressor/src/compressors/alp.rs @@ -2,7 +2,6 @@ use vortex_alp::{alp_encode_components, match_each_alp_float_ptype, ALPArray, AL use vortex_array::aliases::hash_set::HashSet; use vortex_array::array::PrimitiveArray; use vortex_array::encoding::{Encoding, EncodingRef}; -use vortex_array::stats::ArrayStatistics; use vortex_array::variants::PrimitiveArrayTrait; use vortex_array::{ArrayData, IntoArrayData, IntoArrayVariant}; use vortex_dtype::PType; @@ -78,7 +77,7 @@ impl EncodingCompressor for ALPCompressor { compressed_patches.and_then(|p| p.path), ], )), - Some(array.statistics()), + array, )) } diff --git a/vortex-sampling-compressor/src/compressors/alp_rd.rs b/vortex-sampling-compressor/src/compressors/alp_rd.rs index 410afce9f6..c133acd3a7 100644 --- a/vortex-sampling-compressor/src/compressors/alp_rd.rs +++ b/vortex-sampling-compressor/src/compressors/alp_rd.rs @@ -5,7 +5,6 @@ use vortex_alp::{match_each_alp_float_ptype, ALPRDEncoding, RDEncoder as ALPRDEn use vortex_array::aliases::hash_set::HashSet; use vortex_array::array::PrimitiveArray; use vortex_array::encoding::{Encoding, EncodingRef}; -use vortex_array::stats::ArrayStatistics; use vortex_array::variants::PrimitiveArrayTrait; use vortex_array::{ArrayData, IntoArrayData, IntoArrayVariant}; use vortex_dtype::PType; @@ -68,7 +67,7 @@ impl EncodingCompressor for ALPRDCompressor { Ok(CompressedArray::compressed( encoded, Some(CompressionTree::new_with_metadata(self, vec![], encoder)), - Some(array.statistics()), + array, )) } diff --git a/vortex-sampling-compressor/src/compressors/bitpacked.rs b/vortex-sampling-compressor/src/compressors/bitpacked.rs index 888552408f..597c2533cc 100644 --- a/vortex-sampling-compressor/src/compressors/bitpacked.rs +++ b/vortex-sampling-compressor/src/compressors/bitpacked.rs @@ -127,7 +127,7 @@ impl EncodingCompressor for BitPackedCompressor { self, vec![patches.and_then(|p| p.path)], )), - Some(array.statistics()), + array, )) } diff --git a/vortex-sampling-compressor/src/compressors/chunked.rs b/vortex-sampling-compressor/src/compressors/chunked.rs index 3c38e94d07..9d713e7ec6 100644 --- a/vortex-sampling-compressor/src/compressors/chunked.rs +++ b/vortex-sampling-compressor/src/compressors/chunked.rs @@ -5,7 +5,6 @@ use vortex_array::aliases::hash_set::HashSet; use vortex_array::array::{ChunkedArray, ChunkedEncoding}; use vortex_array::compress::compute_precompression_stats; use vortex_array::encoding::{Encoding, EncodingRef}; -use vortex_array::stats::ArrayStatistics; use vortex_array::{ArrayDType, ArrayData, IntoArrayData}; use vortex_error::{vortex_bail, VortexExpect, VortexResult}; @@ -130,7 +129,7 @@ impl ChunkedCompressor { compressed_trees, Arc::new(ChunkedCompressorMetadata(ratio)), )), - Some(array.statistics()), + array, )) } } diff --git a/vortex-sampling-compressor/src/compressors/constant.rs b/vortex-sampling-compressor/src/compressors/constant.rs index 711bc23aa1..5f21e7075c 100644 --- a/vortex-sampling-compressor/src/compressors/constant.rs +++ b/vortex-sampling-compressor/src/compressors/constant.rs @@ -42,7 +42,7 @@ impl EncodingCompressor for ConstantCompressor { ) .into_array(), Some(CompressionTree::flat(self)), - Some(array.statistics()), + array, )) } diff --git a/vortex-sampling-compressor/src/compressors/date_time_parts.rs b/vortex-sampling-compressor/src/compressors/date_time_parts.rs index 6a2f3a467f..d7ae7c4c97 100644 --- a/vortex-sampling-compressor/src/compressors/date_time_parts.rs +++ b/vortex-sampling-compressor/src/compressors/date_time_parts.rs @@ -1,7 +1,6 @@ use vortex_array::aliases::hash_set::HashSet; use vortex_array::array::TemporalArray; use vortex_array::encoding::{Encoding, EncodingRef}; -use vortex_array::stats::ArrayStatistics; use vortex_array::{ArrayDType, ArrayData, IntoArrayData}; use vortex_datetime_dtype::TemporalMetadata; use vortex_datetime_parts::{ @@ -73,7 +72,7 @@ impl EncodingCompressor for DateTimePartsCompressor { self, vec![days.path, seconds.path, subsecond.path], )), - Some(array.statistics()), + array, )) } diff --git a/vortex-sampling-compressor/src/compressors/delta.rs b/vortex-sampling-compressor/src/compressors/delta.rs index d5107be7b9..a7aa6555fc 100644 --- a/vortex-sampling-compressor/src/compressors/delta.rs +++ b/vortex-sampling-compressor/src/compressors/delta.rs @@ -1,7 +1,6 @@ use vortex_array::aliases::hash_set::HashSet; use vortex_array::array::PrimitiveArray; use vortex_array::encoding::{Encoding, EncodingRef}; -use vortex_array::stats::ArrayStatistics; use vortex_array::variants::PrimitiveArrayTrait; use vortex_array::{ArrayData, IntoArrayData}; use vortex_error::VortexResult; @@ -58,7 +57,7 @@ impl EncodingCompressor for DeltaCompressor { DeltaArray::try_from_delta_compress_parts(bases.array, deltas.array, validity)? .into_array(), Some(CompressionTree::new(self, vec![bases.path, deltas.path])), - Some(array.statistics()), + array, )) } diff --git a/vortex-sampling-compressor/src/compressors/dict.rs b/vortex-sampling-compressor/src/compressors/dict.rs index 9eb305cee0..168296fb4e 100644 --- a/vortex-sampling-compressor/src/compressors/dict.rs +++ b/vortex-sampling-compressor/src/compressors/dict.rs @@ -80,7 +80,7 @@ impl EncodingCompressor for DictCompressor { Ok(CompressedArray::compressed( DictArray::try_new(codes.array, values.array)?.into_array(), Some(CompressionTree::new(self, vec![codes.path, values.path])), - Some(array.statistics()), + array, )) } diff --git a/vortex-sampling-compressor/src/compressors/for.rs b/vortex-sampling-compressor/src/compressors/for.rs index ab088db409..4f22266f3a 100644 --- a/vortex-sampling-compressor/src/compressors/for.rs +++ b/vortex-sampling-compressor/src/compressors/for.rs @@ -70,7 +70,7 @@ impl EncodingCompressor for FoRCompressor { ) .map(|a| a.into_array())?, Some(CompressionTree::new(self, vec![compressed_child.path])), - Some(array.statistics()), + array, )) } diff --git a/vortex-sampling-compressor/src/compressors/fsst.rs b/vortex-sampling-compressor/src/compressors/fsst.rs index 070b047f7e..0e570771ca 100644 --- a/vortex-sampling-compressor/src/compressors/fsst.rs +++ b/vortex-sampling-compressor/src/compressors/fsst.rs @@ -6,7 +6,6 @@ use fsst::Compressor; use vortex_array::aliases::hash_set::HashSet; use vortex_array::array::{VarBinEncoding, VarBinViewEncoding}; use vortex_array::encoding::{Encoding, EncodingRef}; -use vortex_array::stats::ArrayStatistics; use vortex_array::{ArrayDType, IntoArrayData}; use vortex_dtype::DType; use vortex_error::{vortex_bail, VortexResult}; @@ -128,7 +127,7 @@ impl EncodingCompressor for FSSTCompressor { vec![None, None, compressed_codes.path, uncompressed_lengths.path], compressor, )), - Some(array.statistics()), + array, )) } diff --git a/vortex-sampling-compressor/src/compressors/list.rs b/vortex-sampling-compressor/src/compressors/list.rs index 9501e1ba59..670e6cb244 100644 --- a/vortex-sampling-compressor/src/compressors/list.rs +++ b/vortex-sampling-compressor/src/compressors/list.rs @@ -1,7 +1,6 @@ use vortex_array::aliases::hash_set::HashSet; use vortex_array::array::{ListArray, ListEncoding}; use vortex_array::encoding::{Encoding, EncodingRef}; -use vortex_array::stats::ArrayStatistics; use vortex_array::{ArrayData, IntoArrayData}; use vortex_error::VortexResult; @@ -51,7 +50,7 @@ impl EncodingCompressor for ListCompressor { self, vec![compressed_elements.path, compressed_offsets.path, None], )), - Some(array.statistics()), + array, )) } diff --git a/vortex-sampling-compressor/src/compressors/mod.rs b/vortex-sampling-compressor/src/compressors/mod.rs index f78a3867ef..44963d17f7 100644 --- a/vortex-sampling-compressor/src/compressors/mod.rs +++ b/vortex-sampling-compressor/src/compressors/mod.rs @@ -6,9 +6,9 @@ use std::sync::Arc; use itertools::{EitherOrBoth, Itertools}; use vortex_array::aliases::hash_set::HashSet; use vortex_array::encoding::EncodingRef; -use vortex_array::stats::{ArrayStatistics, Statistics}; +use vortex_array::stats::ArrayStatistics; use vortex_array::tree::TreeFormatter; -use vortex_array::ArrayData; +use vortex_array::{ArrayDType, ArrayData}; use vortex_error::{vortex_panic, VortexExpect, VortexResult}; use crate::SamplingCompressor; @@ -209,17 +209,37 @@ impl<'a> CompressedArray<'a> { } pub fn compressed( - array: ArrayData, + compressed: ArrayData, path: Option>, - stats_to_inherit: Option<&dyn Statistics>, + uncompressed: impl AsRef, ) -> Self { - if let Some(stats) = stats_to_inherit { - // eagerly compute uncompressed size in bytes at compression time, since it's - // too expensive to compute after compression - let _ = stats.compute_uncompressed_size_in_bytes(); - array.inherit_statistics(stats); - } - let compressed = Self { array, path }; + let uncompressed = uncompressed.as_ref(); + + // Sanity check the compressed array + assert_eq!( + compressed.len(), + uncompressed.len(), + "Compressed array {} has different length to uncompressed", + compressed.encoding().id(), + ); + assert_eq!( + compressed.dtype(), + uncompressed.dtype(), + "Compressed array {} has different dtype to uncompressed", + compressed.encoding().id(), + ); + + // eagerly compute uncompressed size in bytes at compression time, since it's + // too expensive to compute after compression + let _ = uncompressed + .statistics() + .compute_uncompressed_size_in_bytes(); + compressed.inherit_statistics(uncompressed.statistics()); + + let compressed = Self { + array: compressed, + path, + }; compressed.validate(); compressed } diff --git a/vortex-sampling-compressor/src/compressors/roaring_bool.rs b/vortex-sampling-compressor/src/compressors/roaring_bool.rs index 7951b2ceb7..946bdd424e 100644 --- a/vortex-sampling-compressor/src/compressors/roaring_bool.rs +++ b/vortex-sampling-compressor/src/compressors/roaring_bool.rs @@ -1,7 +1,6 @@ use vortex_array::aliases::hash_set::HashSet; use vortex_array::array::BoolEncoding; use vortex_array::encoding::{Encoding, EncodingRef}; -use vortex_array::stats::ArrayStatistics; use vortex_array::{ArrayDType, ArrayData, IntoArrayData, IntoArrayVariant}; use vortex_dtype::DType; use vortex_dtype::Nullability::NonNullable; @@ -50,7 +49,7 @@ impl EncodingCompressor for RoaringBoolCompressor { Ok(CompressedArray::compressed( roaring_bool_encode(array.clone().into_bool()?)?.into_array(), Some(CompressionTree::flat(self)), - Some(array.statistics()), + array, )) } diff --git a/vortex-sampling-compressor/src/compressors/roaring_int.rs b/vortex-sampling-compressor/src/compressors/roaring_int.rs index ae4351c607..4dce6409cd 100644 --- a/vortex-sampling-compressor/src/compressors/roaring_int.rs +++ b/vortex-sampling-compressor/src/compressors/roaring_int.rs @@ -51,7 +51,7 @@ impl EncodingCompressor for RoaringIntCompressor { Ok(CompressedArray::compressed( roaring_int_encode(array.clone().into_primitive()?)?.into_array(), Some(CompressionTree::flat(self)), - Some(array.statistics()), + array, )) } diff --git a/vortex-sampling-compressor/src/compressors/runend.rs b/vortex-sampling-compressor/src/compressors/runend.rs index e250d14ecf..03933fd245 100644 --- a/vortex-sampling-compressor/src/compressors/runend.rs +++ b/vortex-sampling-compressor/src/compressors/runend.rs @@ -51,12 +51,12 @@ impl EncodingCompressor for RunEndCompressor { ctx: SamplingCompressor<'a>, ) -> VortexResult> { let primitive_array = array.clone().into_primitive()?; - let (ends, values) = runend_encode(&primitive_array); - let compressed_ends = ctx.auxiliary("ends").compress( - &downscale_integer_array(ends.into_array())?, - like.as_ref().and_then(|l| l.child(0)), - )?; + let ends = downscale_integer_array(ends.into_array())?.into_primitive()?; + + let compressed_ends = ctx + .auxiliary("ends") + .compress(&ends.into_array(), like.as_ref().and_then(|l| l.child(0)))?; let compressed_values = ctx .named("values") .excluding(self) @@ -73,7 +73,7 @@ impl EncodingCompressor for RunEndCompressor { self, vec![compressed_ends.path, compressed_values.path], )), - Some(array.statistics()), + array, )) } diff --git a/vortex-sampling-compressor/src/compressors/runend_bool.rs b/vortex-sampling-compressor/src/compressors/runend_bool.rs index d02aad067b..8a830c42dc 100644 --- a/vortex-sampling-compressor/src/compressors/runend_bool.rs +++ b/vortex-sampling-compressor/src/compressors/runend_bool.rs @@ -1,7 +1,6 @@ use vortex_array::aliases::hash_set::HashSet; use vortex_array::array::{BoolEncoding, PrimitiveArray}; use vortex_array::encoding::{Encoding, EncodingRef}; -use vortex_array::stats::ArrayStatistics as _; use vortex_array::{ArrayData, IntoArrayData, IntoArrayVariant}; use vortex_error::VortexResult; use vortex_runend_bool::compress::runend_bool_encode_slice; @@ -50,7 +49,7 @@ impl EncodingCompressor for RunEndBoolCompressor { RunEndBoolArray::try_new(compressed_ends.array, start, bool_array.validity())? .into_array(), Some(CompressionTree::new(self, vec![compressed_ends.path])), - Some(array.statistics()), + array, )) } diff --git a/vortex-sampling-compressor/src/compressors/sparse.rs b/vortex-sampling-compressor/src/compressors/sparse.rs index b597f32e4e..b3446e2efe 100644 --- a/vortex-sampling-compressor/src/compressors/sparse.rs +++ b/vortex-sampling-compressor/src/compressors/sparse.rs @@ -1,7 +1,6 @@ use vortex_array::aliases::hash_set::HashSet; use vortex_array::array::{SparseArray, SparseEncoding}; use vortex_array::encoding::{Encoding, EncodingRef}; -use vortex_array::stats::ArrayStatistics; use vortex_array::{ArrayData, ArrayLen, IntoArrayData}; use vortex_error::VortexResult; @@ -49,7 +48,7 @@ impl EncodingCompressor for SparseCompressor { )? .into_array(), Some(CompressionTree::new(self, vec![indices.path, values.path])), - Some(array.statistics()), + array, )) } diff --git a/vortex-sampling-compressor/src/compressors/struct_.rs b/vortex-sampling-compressor/src/compressors/struct_.rs index 454943594f..11ccb39939 100644 --- a/vortex-sampling-compressor/src/compressors/struct_.rs +++ b/vortex-sampling-compressor/src/compressors/struct_.rs @@ -3,7 +3,6 @@ use vortex_array::aliases::hash_set::HashSet; use vortex_array::array::{StructArray, StructEncoding}; use vortex_array::compress::compute_precompression_stats; use vortex_array::encoding::{Encoding, EncodingRef}; -use vortex_array::stats::ArrayStatistics; use vortex_array::variants::StructArrayTrait; use vortex_array::{ArrayDType, ArrayData, ArrayLen, IntoArrayData}; use vortex_dtype::DType; @@ -66,7 +65,7 @@ impl EncodingCompressor for StructCompressor { )? .into_array(), Some(CompressionTree::new(self, trees)), - Some(array.statistics()), + array, )) } diff --git a/vortex-sampling-compressor/src/compressors/varbin.rs b/vortex-sampling-compressor/src/compressors/varbin.rs index 20ec03308b..5b1e5fd182 100644 --- a/vortex-sampling-compressor/src/compressors/varbin.rs +++ b/vortex-sampling-compressor/src/compressors/varbin.rs @@ -1,7 +1,6 @@ use vortex_array::aliases::hash_set::HashSet; use vortex_array::array::{VarBinArray, VarBinEncoding}; use vortex_array::encoding::{Encoding, EncodingRef}; -use vortex_array::stats::ArrayStatistics; use vortex_array::{ArrayDType, ArrayData, IntoArrayData}; use vortex_error::VortexResult; @@ -45,7 +44,7 @@ impl EncodingCompressor for VarBinCompressor { )? .into_array(), Some(CompressionTree::new(self, vec![offsets.path, None, None])), - Some(array.statistics()), + array, )) } diff --git a/vortex-sampling-compressor/src/compressors/zigzag.rs b/vortex-sampling-compressor/src/compressors/zigzag.rs index 8f60b4bf70..659f676055 100644 --- a/vortex-sampling-compressor/src/compressors/zigzag.rs +++ b/vortex-sampling-compressor/src/compressors/zigzag.rs @@ -52,7 +52,7 @@ impl EncodingCompressor for ZigZagCompressor { Ok(CompressedArray::compressed( ZigZagArray::try_new(compressed.array)?.into_array(), Some(CompressionTree::new(self, vec![compressed.path])), - Some(array.statistics()), + array, )) } diff --git a/vortex-scalar/src/lib.rs b/vortex-scalar/src/lib.rs index 1f55d503de..87a5803cca 100644 --- a/vortex-scalar/src/lib.rs +++ b/vortex-scalar/src/lib.rs @@ -1,4 +1,5 @@ use std::cmp::Ordering; +use std::mem::discriminant; use std::sync::Arc; pub use scalar_type::ScalarType; @@ -208,7 +209,9 @@ impl PartialEq for Scalar { impl PartialOrd for Scalar { fn partial_cmp(&self, other: &Self) -> Option { - if self.dtype().eq_ignore_nullability(other.dtype()) { + // We check for DType equality, ignoring nullability, and allowing us to compare all + // primitive types to all other primitive types. + if discriminant(self.dtype()) == discriminant(other.dtype()) { self.value.0.partial_cmp(&other.value.0) } else { None diff --git a/vortex-scalar/src/pvalue.rs b/vortex-scalar/src/pvalue.rs index 5272163fca..88d2eb97b8 100644 --- a/vortex-scalar/src/pvalue.rs +++ b/vortex-scalar/src/pvalue.rs @@ -6,7 +6,7 @@ use num_traits::NumCast; use paste::paste; use vortex_dtype::half::f16; use vortex_dtype::{NativePType, PType}; -use vortex_error::{vortex_err, VortexError}; +use vortex_error::{vortex_err, VortexError, VortexExpect}; #[derive(Debug, Clone, Copy)] pub enum PValue { @@ -26,14 +26,14 @@ pub enum PValue { impl PartialEq for PValue { fn eq(&self, other: &Self) -> bool { match (self, other) { - (Self::U8(s), Self::U8(o)) => s.is_eq(*o), - (Self::U16(s), Self::U16(o)) => s.is_eq(*o), - (Self::U32(s), Self::U32(o)) => s.is_eq(*o), - (Self::U64(s), Self::U64(o)) => s.is_eq(*o), - (Self::I8(s), Self::I8(o)) => s.is_eq(*o), - (Self::I16(s), Self::I16(o)) => s.is_eq(*o), - (Self::I32(s), Self::I32(o)) => s.is_eq(*o), - (Self::I64(s), Self::I64(o)) => s.is_eq(*o), + (Self::U8(s), o) => o.as_u64().vortex_expect("upcast") == *s as u64, + (Self::U16(s), o) => o.as_u64().vortex_expect("upcast") == *s as u64, + (Self::U32(s), o) => o.as_u64().vortex_expect("upcast") == *s as u64, + (Self::U64(s), o) => o.as_u64().vortex_expect("upcast") == *s, + (Self::I8(s), o) => o.as_i64().vortex_expect("upcast") == *s as i64, + (Self::I16(s), o) => o.as_i64().vortex_expect("upcast") == *s as i64, + (Self::I32(s), o) => o.as_i64().vortex_expect("upcast") == *s as i64, + (Self::I64(s), o) => o.as_i64().vortex_expect("upcast") == *s, (Self::F16(s), Self::F16(o)) => s.is_eq(*o), (Self::F32(s), Self::F32(o)) => s.is_eq(*o), (Self::F64(s), Self::F64(o)) => s.is_eq(*o), @@ -45,14 +45,14 @@ impl PartialEq for PValue { impl PartialOrd for PValue { fn partial_cmp(&self, other: &Self) -> Option { match (self, other) { - (Self::U8(s), Self::U8(o)) => Some(s.total_compare(*o)), - (Self::U16(s), Self::U16(o)) => Some(s.total_compare(*o)), - (Self::U32(s), Self::U32(o)) => Some(s.total_compare(*o)), - (Self::U64(s), Self::U64(o)) => Some(s.total_compare(*o)), - (Self::I8(s), Self::I8(o)) => Some(s.total_compare(*o)), - (Self::I16(s), Self::I16(o)) => Some(s.total_compare(*o)), - (Self::I32(s), Self::I32(o)) => Some(s.total_compare(*o)), - (Self::I64(s), Self::I64(o)) => Some(s.total_compare(*o)), + (Self::U8(s), o) => Some((*s as u64).cmp(&o.as_u64().vortex_expect("upcast"))), + (Self::U16(s), o) => Some((*s as u64).cmp(&o.as_u64().vortex_expect("upcast"))), + (Self::U32(s), o) => Some((*s as u64).cmp(&o.as_u64().vortex_expect("upcast"))), + (Self::U64(s), o) => Some((*s).cmp(&o.as_u64().vortex_expect("upcast"))), + (Self::I8(s), o) => Some((*s as i64).cmp(&o.as_i64().vortex_expect("upcast"))), + (Self::I16(s), o) => Some((*s as i64).cmp(&o.as_i64().vortex_expect("upcast"))), + (Self::I32(s), o) => Some((*s as i64).cmp(&o.as_i64().vortex_expect("upcast"))), + (Self::I64(s), o) => Some((*s).cmp(&o.as_i64().vortex_expect("upcast"))), (Self::F16(s), Self::F16(o)) => Some(s.total_compare(*o)), (Self::F32(s), Self::F32(o)) => Some(s.total_compare(*o)), (Self::F64(s), Self::F64(o)) => Some(s.total_compare(*o)), @@ -310,6 +310,8 @@ impl Display for PValue { #[cfg(test)] mod test { + use std::cmp::Ordering; + use vortex_dtype::half::f16; use vortex_dtype::PType; @@ -332,4 +334,16 @@ mod test { assert!(!PValue::F16(f16::from_f32(10.0)).is_instance_of(&PType::U16)); assert!(!PValue::F16(f16::from_f32(10.0)).is_instance_of(&PType::I16)); } + + #[test] + fn test_compare_different_types() { + assert_eq!( + PValue::I8(4).partial_cmp(&PValue::I8(5)), + Some(Ordering::Less) + ); + assert_eq!( + PValue::I8(4).partial_cmp(&PValue::I64(5)), + Some(Ordering::Less) + ); + } }