mbutrovich commented on code in PR #26686:
URL: https://github.com/apache/datafusion/pull/26686#discussion_r4367247937
##########
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>(),
+ ),
Review Comment:
> These two arms can be one:
Done in
[`date_part.rs`](https://github.com/apache/datafusion/blob/48ff4d8da0d3a990f6ef216e13ef4e1f5cbe758f/datafusion/functions/src/datetime/date_part.rs#L540-L544).
It costs about 300 ns per batch on `nanosecond` of `Duration(ns)`, from 227 ns
in the [earlier
run](https://github.com/apache/datafusion/pull/26686#issuecomment-6375740311)
to 542 ns in the
[latest](https://github.com/apache/datafusion/pull/26686#issuecomment-6384580919)
on Neoverse-V2 at 1000 rows. The cost is fixed per batch, so I kept the
simpler form.
--
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]
