Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Array2: swap trait for struct with lifetime #215

Merged
merged 4 commits into from
Apr 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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