-
Notifications
You must be signed in to change notification settings - Fork 1k
Description
Is your feature request related to a problem or challenge? Please describe what you are trying to do.
The current support for arithmetic on temporal and decimal types is limited and fairly inconsistent:
- Binary kernels don't preserve input types, causing timezones, decimal scale and precision, etc... to be lost - Binary Arity Kernels Don't Preserve DataType #3307
- Timestamp subtraction should return duration - Subtracting
TimestampfromTimestampshould produce aDuration(notTimestamp) #3964 - Scalar kernels don't support arithmetic involving different types - RFC: Encode Scalars as
dyn Anyin Scalar dyn Kernels #2842 - Timestamps don't support arithmetic involving durations or intervals
- Decimal arithmetic doesn't compute the correct output scale and precision
- No support for temporal or decimal arithmetic in statically typed kernels
- Operations such as adding two timestamps are supported despite having unclear semantics
- Duplicated code for scalar/non-scalar, checked/unchecked, dyn/static variants of every operation
- No support for arithmetic between scalars
Describe the solution you'd like
It is not a fully formed design, but I would like to implement something inspired by the std::ops traits used for Rust arithmetic.
In particular something like
trait Add<Rhs> {
type Output = T;
fn add(&self, rhs: &Rhs) -> Result<Self::Output, ArrowError>;
fn add_checked(&self, rhs: &Rhs) -> Result<Self::Output, ArrowError>;
}
trait Scalar {
fn data_type(&self) -> &DataType;
fn as_any(&self) -> &dyn Any;
}
struct PrimitiveScalar<T: ArrowPrimitiveType>{
value: T::Native,
data_type: DataType,
}
impl <T: ArrowPrimitiveType> Scalar for PrimitiveScalar<T> {
}
impl <T: ArrowPrimitiveType> Add<PrimitiveArray<T>> for PrimitiveArray<T> {
...
}
impl <T: ArrowPrimitiveType> Add<PrimitiveScalar<T>> for PrimitiveArray<T> {
...
}
impl <T: ArrowPrimitiveType> Add<PrimitiveScalar<T>> for PrimitiveScalar<T> {
...
}
impl Add<dyn Array> for dyn Array {
...
}
impl Add<dyn Scalar> for dyn Array {
...
}
impl Add<dyn Scalar> for dyn Scalar {
...
}
Aside from solving the above problems, it would make for a more idiomatic design that is easier to extend and maintain. We would then deprecate and remove the old kernels.
This would also be a pattern that could be translated to support other kernels that run into similar issues with scalars, e.g. arrow-ord and arrow-string.
Describe alternatives you've considered
We could drop support for arithmetic with statically typed arguments, and add further kernels for performing arithmetic on dyn Scalar
Additional context