-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Closed
Labels
arrowChanges to the arrow crateChanges to the arrow crateenhancementAny new improvement worthy of a entry in the changelogAny new improvement worthy of a entry in the changelog
Description
Is your feature request related to a problem or challenge? Please describe what you are trying to do.
One thing I noticed while reviewing this code is that it will only support Timestamp + Interval not Interval + Timestamp - given this seems to be the case for Date + Interval as well,
Describe the solution you'd like
I would like the arrow add_dyn kernel to support Interval + Timestamp and Interval + Date in addition to Timestamp + Interval and Interval + Date
I would like code such as the following to work:
// timestamp second + interval day time
let a = TimestampSecondArray::from(vec![1, 2, 3, 4, 5]);
let b = IntervalDayTimeArray::from(vec![
Some(IntervalDayTimeType::make_value(1, 0)),
Some(IntervalDayTimeType::make_value(1, 0)),
Some(IntervalDayTimeType::make_value(1, 0)),
Some(IntervalDayTimeType::make_value(1, 0)),
Some(IntervalDayTimeType::make_value(1, 0)),
]);
let result = add_dyn(&a, &b).unwrap(); // <-------- this will work after https://github.com/apache/arrow-rs/pull/4038
let result2 = add_dyn(&b, &a).unwrap(); // <--------- this will not
assert_eq(&result, result2);
let result = result.as_primitive::<TimestampSecondType>();
let expected = TimestampSecondArray::from(vec![
1 + SECONDS_IN_DAY,
2 + SECONDS_IN_DAY,
3 + SECONDS_IN_DAY,
4 + SECONDS_IN_DAY,
5 + SECONDS_IN_DAY,
]);
assert_eq!(&expected, result);
Describe alternatives you've considered
Additional context
Noted on #4038 (comment)
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
arrowChanges to the arrow crateChanges to the arrow crateenhancementAny new improvement worthy of a entry in the changelogAny new improvement worthy of a entry in the changelog