Implement date_part for intervals #6071
Conversation
Signed-off-by: Nick Cameron <[email protected]>
|
apache/datafusion#11501 demonstrates how this change supports |
No problem @nrc -- I am sorry for the delay in reviewing. I think @tustvold has less time to review PRs than he used to, and the rest of us are just trying to work through reviews as much as possible. Sadly, review bandwidth is almost always our most limited resource I am checking it out now |
| impl ExtractDatePartExt for PrimitiveArray<IntervalYearMonthType> { | ||
| fn date_part(&self, part: DatePart) -> Result<Int32Array, ArrowError> { | ||
| match part { | ||
| DatePart::Year => Ok(self.unary_opt(|d| Some(d / 12))), |
There was a problem hiding this comment.
Makes sense as IntervalYearMonth stores the number of whole months: https://docs.rs/arrow/latest/arrow/datatypes/enum.IntervalUnit.html
👍
| @@ -1500,4 +1595,203 @@ mod tests { | |||
| ensure_returns_error(&Time64MicrosecondArray::from(vec![0])); | |||
| ensure_returns_error(&Time64NanosecondArray::from(vec![0])); | |||
| } | |||
|
|
|||
| // IntervalDayTimeType week, day, hour, miute, second, mili, u, nano; invalid month, year; ignores the other part | |||
There was a problem hiding this comment.
these comments seem like they belong with the other tests test_interval_day_time_array and test_interval_month_day_nano_array 🤔
| assert_eq!(0, actual.value(0)); | ||
| assert_eq!(42, actual.value(1)); | ||
| assert_eq!(1042, actual.value(2)); | ||
| assert_eq!(MILLISECONDS_IN_DAY as i32 + 1, actual.value(3)); |
There was a problem hiding this comment.
I find it somewhat strange that only for MILLISECONDS_IN_DAY we have a defined constant 🤔
| assert_eq!(10, actual.value(1)); | ||
| assert_eq!(35, actual.value(2)); | ||
|
|
||
| // Times follow from nanos, but are not affected by months or dats. |
There was a problem hiding this comment.
| // Times follow from nanos, but are not affected by months or dats. | |
| // Times follow from nanos, but are not affected by months or days. |
|
I took the liberty of merging up from main to pick up #6116 |
Dandandan
changed the title
|
Thanks again @nrc |
Which issue does this PR close?
Addresses apache/datafusion#6327
Mentioned in #4969.
Closes #6113
Rationale for this change
Will support using
extractwith intervals in DataFusion. It's also desirable functionality in Arrow for it's own sake, although I don't have a directly motivating use case.Are there any user-facing changes?
The
date_partfunction will now work with intervals