wiedld commented on code in PR #7614:
URL: https://github.com/apache/arrow-datafusion/pull/7614#discussion_r1334910072
##########
datafusion/physical-expr/src/datetime_expressions.rs:
##########
@@ -208,20 +209,25 @@ pub fn make_current_time(
move |_arg| Ok(ColumnarValue::Scalar(ScalarValue::Time64Nanosecond(nano)))
}
-fn quarter_month(date: &NaiveDateTime) -> u32 {
+fn quarter_month<T: TimeZone>(date: &DateTime<T>) -> u32 {
1 + 3 * ((date.month() - 1) / 3)
}
/// Tuncates the single `value`, expressed in nanoseconds since the
/// epoch, for granularities greater than 1 second, in taking into
/// account that some granularities are not uniform durations of time
/// (e.g. months are not always the same lengths, leap seconds, etc)
-fn date_trunc_coarse(granularity: &str, value: i64) -> Result<i64> {
- // Use chrono NaiveDateTime to clear the various fields
- // correctly accounting for non uniform granularities
- let value = timestamp_ns_to_datetime(value).ok_or_else(|| {
- DataFusionError::Execution(format!("Timestamp {value} out of range"))
- })?;
+fn date_trunc_coarse(
+ granularity: &str,
+ value: i64,
+ tz: &Option<Arc<str>>,
+) -> Result<i64> {
+ // Use chrono DateTime<Tz> to clear the various fields because need to
clear per timezone,
+ // and NaiveDateTime (ISO 8601) has no concept of timezones
+ let tz =
arrow_array::timezone::Tz::from_str(tz.as_deref().unwrap_or("+00"))?;
Review Comment:
That's an excellent idea. Tz parsing is [now applied per
batch](https://github.com/apache/arrow-datafusion/pull/7614/commits/9f4cd11bb583d83e93cd3c6be9a7c1e672d50e3a).
Additionally, there was an inference of Tz (when None) in order to use
`DateTime<Tz>`. That has been replaced with the use of generics to accept
either `DateTime<Tz>` or `NaiveDateTime`. Let me know how that looks!
--
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]
