Skip to content

Commit

Permalink
Remove with_dyn and ArrayDef (#1503)
Browse files Browse the repository at this point in the history
Waiting on #1501
  • Loading branch information
gatesn authored Nov 29, 2024
1 parent 6696266 commit bbdb949
Show file tree
Hide file tree
Showing 27 changed files with 151 additions and 246 deletions.
10 changes: 5 additions & 5 deletions encodings/fastlanes/src/bitpacking/compute/search_sorted.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use vortex_array::compute::{
search_sorted_usize, IndexOrd, Len, SearchResult, SearchSorted, SearchSortedFn,
SearchSortedSide,
};
use vortex_array::stats::ArrayStatistics;
use vortex_array::validity::Validity;
use vortex_array::variants::PrimitiveArrayTrait;
use vortex_array::{ArrayDType, ArrayLen};
Expand Down Expand Up @@ -156,11 +157,10 @@ impl<'a, T: BitPacking + NativePType> BitPackedSearch<'a, T> {
Validity::AllInvalid => 0,
Validity::Array(varray) => {
// In sorted order, nulls come after all the non-null values.
varray.with_dyn(|a| {
a.statistics()
.compute_true_count()
.vortex_expect("Failed to compute true count")
})
varray
.statistics()
.compute_true_count()
.vortex_expect("Failed to compute true count")
}
};

Expand Down
10 changes: 4 additions & 6 deletions encodings/fsst/src/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,13 @@ use std::sync::Arc;

use fsst::{Decompressor, Symbol};
use serde::{Deserialize, Serialize};
use vortex_array::array::{VarBin, VarBinArray};
use vortex_array::encoding::ids;
use vortex_array::array::{VarBinArray, VarBinEncoding};
use vortex_array::encoding::{ids, Encoding};
use vortex_array::stats::{StatisticsVTable, StatsSet};
use vortex_array::validity::{ArrayValidity, LogicalValidity, Validity, ValidityVTable};
use vortex_array::variants::{BinaryArrayTrait, Utf8ArrayTrait, VariantsVTable};
use vortex_array::visitor::{ArrayVisitor, VisitorVTable};
use vortex_array::{
impl_encoding, ArrayDType, ArrayData, ArrayDef, ArrayLen, ArrayTrait, IntoCanonical,
};
use vortex_array::{impl_encoding, ArrayDType, ArrayData, ArrayLen, ArrayTrait, IntoCanonical};
use vortex_dtype::{DType, Nullability, PType};
use vortex_error::{vortex_bail, VortexExpect, VortexResult};

Expand Down Expand Up @@ -75,7 +73,7 @@ impl FSSTArray {
vortex_bail!(InvalidArgument: "uncompressed_lengths must have integer type and cannot be nullable");
}

if codes.encoding().id() != VarBin::ID {
if codes.encoding().id() != VarBinEncoding::ID {
vortex_bail!(
InvalidArgument: "codes must have varbin encoding, was {}",
codes.encoding().id()
Expand Down
9 changes: 5 additions & 4 deletions encodings/fsst/tests/fsst_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@
use vortex_array::array::builder::VarBinBuilder;
use vortex_array::array::PrimitiveArray;
use vortex_array::compute::{filter, scalar_at, slice, take, FilterMask, TakeOptions};
use vortex_array::encoding::Encoding;
use vortex_array::validity::Validity;
use vortex_array::{ArrayData, ArrayDef, IntoArrayData, IntoCanonical};
use vortex_array::{ArrayData, IntoArrayData, IntoCanonical};
use vortex_dtype::{DType, Nullability};
use vortex_fsst::{fsst_compress, fsst_train_compressor, FSST};
use vortex_fsst::{fsst_compress, fsst_train_compressor, FSSTEncoding};

macro_rules! assert_nth_scalar {
($arr:expr, $n:expr, $expected:expr) => {
Expand Down Expand Up @@ -55,7 +56,7 @@ fn test_fsst_array_ops() {

// test slice
let fsst_sliced = slice(&fsst_array, 1, 3).unwrap();
assert_eq!(fsst_sliced.encoding().id(), FSST::ENCODING.id());
assert_eq!(fsst_sliced.encoding().id(), FSSTEncoding::ID);
assert_eq!(fsst_sliced.len(), 2);
assert_nth_scalar!(
fsst_sliced,
Expand Down Expand Up @@ -87,7 +88,7 @@ fn test_fsst_array_ops() {
let mask = FilterMask::from_iter([false, true, false]);

let fsst_filtered = filter(&fsst_array, mask).unwrap();
assert_eq!(fsst_filtered.encoding().id(), FSST::ENCODING.id());
assert_eq!(fsst_filtered.encoding().id(), FSSTEncoding::ID);
assert_eq!(fsst_filtered.len(), 1);
assert_nth_scalar!(
fsst_filtered,
Expand Down
28 changes: 2 additions & 26 deletions vortex-array/src/data/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use owned::OwnedArrayData;
use viewed::ViewedArrayData;
use vortex_buffer::Buffer;
use vortex_dtype::DType;
use vortex_error::{vortex_err, vortex_panic, VortexExpect, VortexResult};
use vortex_error::{vortex_err, VortexExpect, VortexResult};
use vortex_scalar::Scalar;

use crate::array::{
Expand All @@ -22,7 +22,7 @@ use crate::stats::{ArrayStatistics, Stat, Statistics, StatsSet};
use crate::stream::{ArrayStream, ArrayStreamAdapter};
use crate::validity::{ArrayValidity, LogicalValidity, ValidityVTable};
use crate::{
ArrayChildrenIterator, ArrayDType, ArrayLen, ArrayMetadata, ArrayTrait, Context,
ArrayChildrenIterator, ArrayDType, ArrayLen, ArrayMetadata, Context,
TryDeserializeArrayMetadata,
};

Expand Down Expand Up @@ -335,30 +335,6 @@ impl ArrayData {
pub fn is_encoding(&self, id: EncodingId) -> bool {
self.encoding().id() == id
}

#[inline]
pub fn with_dyn<R, F>(&self, mut f: F) -> R
where
F: FnMut(&dyn ArrayTrait) -> R,
{
let mut result = None;

self.encoding()
.with_dyn(self, &mut |array| {
result = Some(f(array));
Ok(())
})
.unwrap_or_else(|err| {
vortex_panic!(
err,
"Failed to convert Array to {}",
std::any::type_name::<dyn ArrayTrait>()
)
});

// Now we unwrap the optional, which we know to be populated by the closure.
result.vortex_expect("Failed to get result from Array::with_dyn")
}
}

impl Display for ArrayData {
Expand Down
33 changes: 3 additions & 30 deletions vortex-array/src/encoding/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,12 @@ use std::any::Any;
use std::fmt::{Debug, Display, Formatter};
use std::hash::{Hash, Hasher};

use vortex_error::{vortex_panic, VortexResult};

use crate::compute::ComputeVTable;
use crate::stats::StatisticsVTable;
use crate::validity::ValidityVTable;
use crate::variants::VariantsVTable;
use crate::visitor::VisitorVTable;
use crate::{ArrayData, ArrayDef, ArrayMetadata, ArrayTrait, IntoCanonicalVTable, MetadataVTable};
use crate::{ArrayData, ArrayMetadata, IntoCanonicalVTable, MetadataVTable};

pub mod opaque;

Expand Down Expand Up @@ -63,6 +61,8 @@ impl AsRef<str> for EncodingId {

/// Marker trait for array encodings with their associated Array type.
pub trait Encoding: 'static {
const ID: EncodingId;

type Array;
type Metadata: ArrayMetadata;
}
Expand All @@ -86,13 +86,6 @@ pub trait EncodingVTable:
fn id(&self) -> EncodingId;

fn as_any(&self) -> &dyn Any;

/// Unwrap the provided array into an implementation of ArrayTrait
fn with_dyn(
&self,
array: &ArrayData,
f: &mut dyn for<'b> FnMut(&'b (dyn ArrayTrait + 'b)) -> VortexResult<()>,
) -> VortexResult<()>;
}

impl PartialEq for dyn EncodingVTable + '_ {
Expand All @@ -107,26 +100,6 @@ impl Hash for dyn EncodingVTable + '_ {
}
}

/// Non-object-safe extensions to the ArrayEncoding trait.
pub trait ArrayEncodingExt {
type D: ArrayDef;

fn with_dyn<R, F>(array: &ArrayData, mut f: F) -> R
where
F: for<'b> FnMut(&'b (dyn ArrayTrait + 'b)) -> R,
{
let typed = <<Self::D as ArrayDef>::Array as TryFrom<ArrayData>>::try_from(array.clone())
.unwrap_or_else(|err| {
vortex_panic!(
err,
"Failed to convert array to {}",
std::any::type_name::<<Self::D as ArrayDef>::Array>()
)
});
f(&typed)
}
}

pub trait ArrayEncodingRef {
fn encoding(&self) -> EncodingRef;
}
Expand Down
13 changes: 1 addition & 12 deletions vortex-array/src/encoding/opaque.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::validity::{LogicalValidity, ValidityVTable};
use crate::variants::VariantsVTable;
use crate::visitor::{ArrayVisitor, VisitorVTable};
use crate::{
ArrayData, ArrayMetadata, ArrayTrait, Canonical, IntoCanonicalVTable, MetadataVTable,
ArrayData, ArrayMetadata, Canonical, IntoCanonicalVTable, MetadataVTable,
TrySerializeArrayMetadata,
};

Expand All @@ -38,17 +38,6 @@ impl EncodingVTable for OpaqueEncoding {
fn as_any(&self) -> &dyn Any {
self
}

fn with_dyn(
&self,
_array: &ArrayData,
_f: &mut dyn for<'b> FnMut(&'b (dyn ArrayTrait + 'b)) -> VortexResult<()>,
) -> VortexResult<()> {
vortex_bail!(
"OpaqueEncoding: with_dyn cannot be called for opaque array ({})",
self.0
)
}
}

impl IntoCanonicalVTable for OpaqueEncoding {
Expand Down
1 change: 0 additions & 1 deletion vortex-array/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
pub use canonical::*;
pub use context::*;
pub use data::*;
pub use macros::*;
pub use metadata::*;
pub use paste;
use vortex_dtype::DType;
Expand Down
64 changes: 14 additions & 50 deletions vortex-array/src/macros.rs
Original file line number Diff line number Diff line change
@@ -1,22 +1,7 @@
//! The core Vortex macro to create new encodings and array types.
use vortex_error::VortexError;

use crate::encoding::{
ArrayEncodingExt, ArrayEncodingRef, EncodingId, EncodingRef, EncodingVTable,
};
use crate::{ArrayData, ArrayMetadata, ArrayTrait, ToArrayData, TryDeserializeArrayMetadata};

/// Trait the defines the set of types relating to an array.
/// Because it has associated types it can't be used as a trait object.
pub trait ArrayDef {
const ID: EncodingId;
const ENCODING: EncodingRef;

type Array: ArrayTrait + TryFrom<ArrayData, Error = VortexError>;
type Metadata: ArrayMetadata + Clone + for<'m> TryDeserializeArrayMetadata<'m>;
type Encoding: EncodingVTable + ArrayEncodingExt<D = Self>;
}
use crate::encoding::{ArrayEncodingRef, EncodingRef};
use crate::{ArrayData, ToArrayData};

impl<A: AsRef<ArrayData>> ToArrayData for A {
fn to_array(&self) -> ArrayData {
Expand All @@ -32,22 +17,6 @@ impl<A: AsRef<ArrayData>> ToArrayData for A {
macro_rules! impl_encoding {
($id:literal, $code:expr, $Name:ident) => {
$crate::paste::paste! {
/// The array definition trait
#[derive(std::fmt::Debug, Clone)]
pub struct $Name;
impl $crate::ArrayDef for $Name {
const ID: $crate::encoding::EncodingId = $crate::encoding::EncodingId::new($id, $code);
const ENCODING: $crate::encoding::EncodingRef = &[<$Name Encoding>];
type Array = [<$Name Array>];
type Metadata = [<$Name Metadata>];
type Encoding = [<$Name Encoding>];
}

impl $crate::encoding::Encoding for [<$Name Encoding>] {
type Array = [<$Name Array>];
type Metadata = [<$Name Metadata>];
}

#[derive(std::fmt::Debug, Clone)]
#[repr(transparent)]
pub struct [<$Name Array>]($crate::ArrayData);
Expand Down Expand Up @@ -95,11 +64,11 @@ macro_rules! impl_encoding {
type Error = vortex_error::VortexError;

fn try_from(data: $crate::ArrayData) -> vortex_error::VortexResult<Self> {
if data.encoding().id() != <$Name as $crate::ArrayDef>::ID {
if data.encoding().id() != <[<$Name Encoding>] as $crate::encoding::Encoding>::ID {
vortex_error::vortex_bail!(
"Mismatched encoding {}, expected {}",
data.encoding().id().as_ref(),
<$Name as $crate::ArrayDef>::ID,
<[<$Name Encoding>] as $crate::encoding::Encoding>::ID,
);
}
Ok(Self(data))
Expand All @@ -112,11 +81,11 @@ macro_rules! impl_encoding {
type Error = vortex_error::VortexError;

fn try_from(data: &'a $crate::ArrayData) -> vortex_error::VortexResult<Self> {
if data.encoding().id() != <$Name as $crate::ArrayDef>::ID {
if data.encoding().id() != <[<$Name Encoding>] as $crate::encoding::Encoding>::ID {
vortex_error::vortex_bail!(
"Mismatched encoding {}, expected {}",
data.encoding().id().as_ref(),
<$Name as $crate::ArrayDef>::ID,
<[<$Name Encoding>] as $crate::encoding::Encoding>::ID,
);
}
Ok(unsafe { std::mem::transmute::<&$crate::ArrayData, &[<$Name Array>]>(data) })
Expand All @@ -126,27 +95,22 @@ macro_rules! impl_encoding {
/// The array encoding
#[derive(std::fmt::Debug)]
pub struct [<$Name Encoding>];

impl $crate::encoding::Encoding for [<$Name Encoding>] {
const ID: $crate::encoding::EncodingId = $crate::encoding::EncodingId::new($id, $code);
type Array = [<$Name Array>];
type Metadata = [<$Name Metadata>];
}

impl $crate::encoding::EncodingVTable for [<$Name Encoding>] {
#[inline]
fn id(&self) -> $crate::encoding::EncodingId {
<$Name as $crate::ArrayDef>::ID
<[<$Name Encoding>] as $crate::encoding::Encoding>::ID
}

fn as_any(&self) -> &dyn std::any::Any {
self
}

#[inline]
fn with_dyn(
&self,
array: &$crate::ArrayData,
f: &mut dyn for<'b> FnMut(&'b (dyn $crate::ArrayTrait + 'b)) -> vortex_error::VortexResult<()>,
) -> vortex_error::VortexResult<()> {
<Self as $crate::encoding::ArrayEncodingExt>::with_dyn(array, f)
}
}
impl $crate::encoding::ArrayEncodingExt for [<$Name Encoding>] {
type D = $Name;
}

/// Implement ArrayMetadata
Expand Down
8 changes: 4 additions & 4 deletions vortex-sampling-compressor/src/compressors/alp.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use vortex_alp::{alp_encode_components, match_each_alp_float_ptype, ALPArray, ALPEncoding, ALP};
use vortex_alp::{alp_encode_components, match_each_alp_float_ptype, ALPArray, ALPEncoding};
use vortex_array::aliases::hash_set::HashSet;
use vortex_array::array::PrimitiveArray;
use vortex_array::encoding::EncodingRef;
use vortex_array::encoding::{Encoding, EncodingRef};
use vortex_array::stats::ArrayStatistics;
use vortex_array::variants::PrimitiveArrayTrait;
use vortex_array::{ArrayData, ArrayDef, IntoArrayData, IntoArrayVariant};
use vortex_array::{ArrayData, IntoArrayData, IntoArrayVariant};
use vortex_dtype::PType;
use vortex_error::VortexResult;

Expand All @@ -17,7 +17,7 @@ pub struct ALPCompressor;

impl EncodingCompressor for ALPCompressor {
fn id(&self) -> &str {
ALP::ID.as_ref()
ALPEncoding::ID.as_ref()
}

fn cost(&self) -> u8 {
Expand Down
8 changes: 4 additions & 4 deletions vortex-sampling-compressor/src/compressors/alp_rd.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
use std::any::Any;
use std::sync::Arc;

use vortex_alp::{match_each_alp_float_ptype, ALPRDEncoding, RDEncoder as ALPRDEncoder, ALPRD};
use vortex_alp::{match_each_alp_float_ptype, ALPRDEncoding, RDEncoder as ALPRDEncoder};
use vortex_array::aliases::hash_set::HashSet;
use vortex_array::array::PrimitiveArray;
use vortex_array::encoding::EncodingRef;
use vortex_array::encoding::{Encoding, EncodingRef};
use vortex_array::stats::ArrayStatistics;
use vortex_array::variants::PrimitiveArrayTrait;
use vortex_array::{ArrayData, ArrayDef, IntoArrayData, IntoArrayVariant};
use vortex_array::{ArrayData, IntoArrayData, IntoArrayVariant};
use vortex_dtype::PType;
use vortex_error::{vortex_bail, VortexResult};
use vortex_fastlanes::BitPackedEncoding;
Expand All @@ -26,7 +26,7 @@ impl EncoderMetadata for ALPRDEncoder {

impl EncodingCompressor for ALPRDCompressor {
fn id(&self) -> &str {
ALPRD::ID.as_ref()
ALPRDEncoding::ID.as_ref()
}

fn cost(&self) -> u8 {
Expand Down
Loading

0 comments on commit bbdb949

Please sign in to comment.