alamb commented on code in PR #26686:
URL: https://github.com/apache/datafusion/pull/26686#discussion_r4372337981
##########
datafusion/functions/src/datetime/date_part.rs:
##########
@@ -564,48 +509,189 @@ 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`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(),
- )));
- }
+/// `nanosecond` returns Int64, because 60 seconds are 60 billion nanoseconds
Review Comment:
👍
##########
datafusion/spark/src/function/datetime/date_part.rs:
##########
@@ -77,14 +76,11 @@ impl ScalarUDFImpl for SparkDatePart {
&self.signature
}
+ // The default `return_field_from_args` makes the result nullable, like the
+ // integer parts of `date_part`, which this simplifies to, because they can
+ // be NULL for a non-null input
fn return_type(&self, _arg_types: &[DataType]) -> Result<DataType> {
Review Comment:
I think it would be better to explicitly implement return-field_from_args
here as it is more explicit
##########
datafusion/functions/src/datetime/date_part.rs:
##########
@@ -564,48 +509,189 @@ 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`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(),
- )));
- }
+/// `nanosecond` returns Int64, because 60 seconds are 60 billion nanoseconds
+/// and a duration can be up to `i64::MAX` nanoseconds long. `millisecond` and
+/// `microsecond` return Int32. A duration whose length overflows the return
+/// type is NULL.
+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
Review Comment:
We could probably constant fold this in `simplify` as well (return a
constant 0)
--
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]
