Skip to content

Commit

Permalink
Array2: swap trait for struct with lifetime (#215)
Browse files Browse the repository at this point in the history
Now we have an Array<'a> enum, we can swap out the array trait and
things get a little simpler.
  • Loading branch information
gatesn authored Apr 7, 2024
1 parent 47f6eb8 commit 228dc21
Show file tree
Hide file tree
Showing 12 changed files with 266 additions and 349 deletions.
2 changes: 1 addition & 1 deletion vortex-array2/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,4 @@ vortex-flatbuffers = { path = "../vortex-flatbuffers" }
vortex-schema = { path = "../vortex-schema" }

[lints]
workspace = true
# workspace = true
19 changes: 14 additions & 5 deletions vortex-array2/src/array/bool/compute.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,26 @@
use vortex::scalar::Scalar;
use vortex::scalar::{BoolScalar, Scalar};
use vortex_error::VortexResult;

use crate::array::bool::BoolArray;
use crate::compute::{ArrayCompute, ScalarAtFn};
use crate::validity::ArrayValidity;
use crate::ArrayTrait;

impl ArrayCompute for &dyn BoolArray {
impl ArrayCompute for BoolArray<'_> {
fn scalar_at(&self) -> Option<&dyn ScalarAtFn> {
Some(self)
}
}

impl ScalarAtFn for &dyn BoolArray {
fn scalar_at(&self, _index: usize) -> VortexResult<Scalar> {
todo!()
impl ScalarAtFn for BoolArray<'_> {
fn scalar_at(&self, index: usize) -> VortexResult<Scalar> {
if self.is_valid(index) {
let value = self.boolean_buffer().value(index);
Ok(Scalar::Bool(
BoolScalar::try_new(Some(value), self.dtype().nullability()).unwrap(),
))
} else {
Ok(Scalar::null(self.dtype()))
}
}
}
147 changes: 70 additions & 77 deletions vortex-array2/src/array/bool/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,12 @@ use arrow_buffer::{BooleanBuffer, Buffer};
use vortex_error::VortexResult;
use vortex_schema::DType;

use crate::impl_encoding;
use crate::validity::Validity;
use crate::validity::{ArrayValidity, ValidityMetadata};
use crate::{impl_encoding, IntoArray};
use crate::ArrayMetadata;
use crate::{ArrayData, TypedArrayData};
use crate::{ArrayMetadata, TryFromArrayMetadata};
use crate::{ArrayView, ToArrayData};
use crate::{ToArray, TypedArrayView};

impl_encoding!("vortex.bool", Bool);

Expand All @@ -20,23 +19,58 @@ pub struct BoolMetadata {
length: usize,
}

#[allow(clippy::len_without_is_empty)]
impl BoolMetadata {
pub fn validity(&self) -> &ValidityMetadata {
&self.validity
impl TryParseArrayMetadata for BoolMetadata {
fn try_parse_metadata(_metadata: Option<&[u8]>) -> VortexResult<Self> {
todo!()
}
}

pub struct BoolArray<'a> {
dtype: &'a DType,
buffer: &'a Buffer,
validity: Option<Validity<'a>>,
// TODO(ngates): unpack metadata?
metadata: &'a BoolMetadata,
// TODO(ngates): we support statistics by reference to a dyn trait.
// This trait is implemented for ArrayView and ArrayData and is passed into here as part
// of ArrayParts.
// e.g. stats: &dyn Statistics,
}

impl BoolArray<'_> {
pub fn buffer(&self) -> &Buffer {
self.buffer
}

pub fn validity(&self) -> Option<&Validity> {
self.validity.as_ref()
}

pub fn metadata(&self) -> &BoolMetadata {
self.metadata
}

pub fn len(&self) -> usize {
self.length
pub fn boolean_buffer(&self) -> BooleanBuffer {
BooleanBuffer::new(self.buffer.clone(), 0, self.metadata.length)
}
}

// TODO(ngates): I think this could be a struct?
#[allow(clippy::len_without_is_empty)]
pub trait BoolArray {
fn buffer(&self) -> &Buffer;
fn len(&self) -> usize;
fn validity(&self) -> Option<Validity>;
impl<'v> TryFromArrayParts<'v, BoolMetadata> for BoolArray<'v> {
fn try_from_parts(
parts: &'v dyn ArrayParts<'v>,
metadata: &'v BoolMetadata,
) -> VortexResult<Self> {
Ok(BoolArray {
dtype: parts.dtype(),
buffer: parts
.buffer(0)
.ok_or(vortex_err!("BoolArray requires a buffer"))?,
validity: metadata
.validity
.to_validity(metadata.length, parts.child(0, &Validity::DTYPE)),
metadata,
})
}
}

impl BoolData {
Expand All @@ -59,82 +93,41 @@ impl BoolData {
}
}

impl BoolArray for BoolData {
fn buffer(&self) -> &Buffer {
self.data().buffers().first().unwrap()
impl ArrayTrait for BoolArray<'_> {
fn dtype(&self) -> &DType {
self.dtype
}

fn len(&self) -> usize {
self.metadata().len()
}

fn validity(&self) -> Option<Validity> {
self.metadata().validity().to_validity(
self.metadata().len(),
self.data().child(0).map(|data| data.to_array()),
)
self.metadata().length
}
}

impl BoolArray for BoolView<'_> {
fn buffer(&self) -> &Buffer {
self.view()
.buffers()
.first()
.expect("BoolView must have a single buffer")
}

fn len(&self) -> usize {
self.metadata().len()
}

fn validity(&self) -> Option<Validity> {
self.metadata().validity().to_validity(
self.metadata().len(),
self.view()
.child(0, &Validity::DTYPE)
.map(|a| a.into_array()),
)
impl ArrayValidity for BoolArray<'_> {
fn is_valid(&self, index: usize) -> bool {
self.validity().map(|v| v.is_valid(index)).unwrap_or(true)
}
}

impl TryFromArrayMetadata for BoolMetadata {
fn try_from_metadata(_metadata: Option<&[u8]>) -> VortexResult<Self> {
impl ToArrayData for BoolArray<'_> {
fn to_array_data(&self) -> ArrayData {
todo!()
}
}

impl<'v> TryFromArrayView<'v> for BoolView<'v> {
fn try_from_view(view: &'v ArrayView<'v>) -> VortexResult<Self> {
// TODO(ngates): validate the view.
Ok(BoolView::new_unchecked(
view.clone(),
BoolMetadata::try_from_metadata(view.metadata())?,
))
}
}

impl TryFromArrayData for BoolData {
fn try_from_data(data: &ArrayData) -> VortexResult<Self> {
// TODO(ngates): validate the array data.
Ok(Self::from_data_unchecked(data.clone()))
}
}

impl ArrayTrait for &dyn BoolArray {
fn len(&self) -> usize {
(**self).len()
}
}
#[cfg(test)]
mod tests {
use crate::array::bool::BoolData;
use crate::compute::scalar_at;
use crate::IntoArray;

impl ArrayValidity for &dyn BoolArray {
fn is_valid(&self, index: usize) -> bool {
self.validity().map(|v| v.is_valid(index)).unwrap_or(true)
}
}
#[test]
fn bool_array() {
let arr = BoolData::try_new(vec![true, false, true].into(), None)
.unwrap()
.into_array();

impl ToArrayData for &dyn BoolArray {
fn to_array_data(&self) -> ArrayData {
todo!()
let scalar: bool = scalar_at(&arr, 0).unwrap().try_into().unwrap();
assert_eq!(scalar, true);
}
}
5 changes: 3 additions & 2 deletions vortex-array2/src/array/primitive/compute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@ use vortex_error::VortexResult;
use crate::array::primitive::PrimitiveArray;
use crate::compute::{ArrayCompute, ScalarAtFn};
use crate::validity::ArrayValidity;
use crate::ArrayTrait;

impl ArrayCompute for &dyn PrimitiveArray {
impl ArrayCompute for PrimitiveArray<'_> {
fn scalar_at(&self) -> Option<&dyn ScalarAtFn> {
Some(self)
}
}

impl ScalarAtFn for &dyn PrimitiveArray {
impl ScalarAtFn for PrimitiveArray<'_> {
fn scalar_at(&self, index: usize) -> VortexResult<Scalar> {
if self.is_valid(index) {
match_each_native_ptype!(self.ptype(), |$T| {
Expand Down
Loading

0 comments on commit 228dc21

Please sign in to comment.