mbutrovich commented on code in PR #26686:
URL: https://github.com/apache/datafusion/pull/26686#discussion_r4367248751
##########
datafusion/functions/src/datetime/date_part.rs:
##########
@@ -564,48 +504,83 @@ fn epoch(array: &dyn Array) -> Result<ArrayRef> {
Ok(Arc::new(f))
}
-/// Invoke [`date_part`] on an `array` (e.g. Timestamp) and convert the
-/// result to a total number of nanoseconds as an Int64 array.
+/// Return the seconds of `array`, with their fraction, as a number of
+/// milliseconds, microseconds or nanoseconds (`part`): the seconds of the
+/// minute for timestamps, times and intervals, and the total length for
+/// durations.
///
-/// This returns an Int64 rather than Int32 because there 1 billion
+/// `nanosecond` returns an Int64 rather than Int32 because there are 1 billion
/// `nanosecond`s in each second, so representing up to 60 seconds as
/// nanoseconds can be values up to 60 billion, which does not fit in Int32.
-fn seconds_ns(array: &dyn Array) -> Result<ArrayRef> {
- // Fast path for Date32 and Date64 - no nanoseconds
- if array.data_type() == &Date32 || array.data_type() == &Date64 {
- return Ok(Arc::new(Int64Array::from_iter_values_with_nulls(
- repeat_n(0, array.len()),
- array.nulls().cloned(),
- )));
- }
+fn subsecond_part(array: &dyn Array, part: DatePart) -> Result<ArrayRef> {
+ let len = array.len();
+ let nulls = array.nulls().cloned();
+ Ok(match (array.data_type(), part) {
+ // Fast path for Date32 and Date64 - no seconds
+ (Date32 | Date64, DatePart::Nanosecond) => {
+ Arc::new(Int64Array::new(vec![0; len].into(), nulls))
+ }
+ (Date32 | Date64, _) => Arc::new(Int32Array::new(vec![0; len].into(),
nulls)),
+ // arrow's `date_part(Nanosecond)` computes these, but returns Int32,
+ // which overflows past about 2.1 seconds
+ (Interval(ArrowIntervalUnit::DayTime), DatePart::Nanosecond) =>
Arc::new(
+ as_interval_dt_array(array)?
+ .unary::<_, Int64Type>(|x| (x.milliseconds % 60_000) as i64 *
1_000_000),
+ ),
+ (Interval(ArrowIntervalUnit::MonthDayNano), DatePart::Nanosecond) =>
Arc::new(
+ as_interval_mdn_array(array)?
+ .unary::<_, Int64Type>(|x| x.nanoseconds % 60_000_000_000),
+ ),
+ (Duration(Nanosecond), DatePart::Nanosecond) => {
+
Arc::new(as_duration_nanosecond_array(array)?.reinterpret_cast::<Int64Type>())
+ }
+ // arrow's cast returns NULL on overflow, like its `date_part`
+ (Duration(_), DatePart::Nanosecond) => Arc::new(
+ as_duration_nanosecond_array(&cast(array, &Duration(Nanosecond))?)?
+ .reinterpret_cast::<Int64Type>(),
+ ),
+ // arrow's sub-second parts of intervals and durations already include
+ // the whole seconds
+ (
+ Interval(ArrowIntervalUnit::DayTime |
ArrowIntervalUnit::MonthDayNano)
+ | Duration(_),
+ _,
+ ) => date_part(array, part)?,
Review Comment:
> Could this be `Interval(_) | Duration(_)`?
Done in
[`date_part.rs`](https://github.com/apache/datafusion/blob/48ff4d8da0d3a990f6ef216e13ef4e1f5cbe758f/datafusion/functions/src/datetime/date_part.rs#L545-L547).
`nanosecond` fell through the same way, so the remaining [`query
error`](https://github.com/apache/datafusion/blob/48ff4d8da0d3a990f6ef216e13ef4e1f5cbe758f/datafusion/sqllogictest/test_files/datetime/date_part.slt#L1385-L1386)
now checks for `Nanosecond does not support`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
