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

Add overflow-checking variants of arithmetic dyn kernels #2740

Merged
merged 17 commits into from
Sep 20, 2022
13 changes: 13 additions & 0 deletions arrow/src/array/array.rs
Expand Up @@ -356,6 +356,19 @@ pub trait ArrayAccessor: Array {
/// # Safety
/// Caller is responsible for ensuring that the index is within the bounds of the array
unsafe fn value_unchecked(&self, index: usize) -> Self::Item;

/// Returns a values accessor [`ArrayValuesAccessor`] for this [`ArrayAccessor`] if
/// it supports. Returns [`None`] if it doesn't support accessing values directly.
fn get_values_accessor(&self) -> Option<&dyn ArrayValuesAccessor<Item = Self::Item>> {
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the point of the dyn indirection here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't want to return a Option<&[Self::Item]> directly as the semantics look weird (i.e. some ArrayAccessor doesn't provide values). This is to return self as ArrayValuesAccessor so the caller can call values.

But I think I will remove it based on #2740 (comment).

None
}
}

/// A trait for accessing the values of an [`ArrayAccessor`] as a slice at once. Not all
/// [`ArrayAccessor`] implementations support this trait. Currently only [`PrimitiveArray`]
/// supports it.
pub trait ArrayValuesAccessor: ArrayAccessor {
fn values(&self) -> &[Self::Item];
}

/// Constructs an array using the input `data`.
Expand Down
12 changes: 11 additions & 1 deletion arrow/src/array/array_primitive.rs
Expand Up @@ -33,7 +33,7 @@ use crate::{
util::trusted_len_unzip,
};

use crate::array::array::ArrayAccessor;
use crate::array::array::{ArrayAccessor, ArrayValuesAccessor};
use half::f16;

/// Array whose elements are of primitive types.
Expand Down Expand Up @@ -204,6 +204,16 @@ impl<'a, T: ArrowPrimitiveType> ArrayAccessor for &'a PrimitiveArray<T> {
unsafe fn value_unchecked(&self, index: usize) -> Self::Item {
PrimitiveArray::value_unchecked(self, index)
}

fn get_values_accessor(&self) -> Option<&dyn ArrayValuesAccessor<Item = Self::Item>> {
Some(self)
}
}

impl<'a, T: ArrowPrimitiveType> ArrayValuesAccessor for &'a PrimitiveArray<T> {
fn values(&self) -> &[Self::Item] {
PrimitiveArray::values(self)
}
}

pub(crate) fn as_datetime<T: ArrowPrimitiveType>(v: i64) -> Option<NaiveDateTime> {
Expand Down