viirya commented on code in PR #6789:
URL: https://github.com/apache/arrow-datafusion/pull/6789#discussion_r1245691545


##########
datafusion/physical-expr/src/datetime_expressions.rs:
##########
@@ -219,42 +219,51 @@ fn quarter_month(date: &NaiveDateTime) -> u32 {
 /// 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> {
-    if granularity == "millisecond" || granularity == "microsecond" {
-        return Ok(value);
-    }
+    // 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"))
+    })?;
+
+    let value = Some(value);
 
-    let value = timestamp_ns_to_datetime(value)
-        .ok_or_else(|| {
-            DataFusionError::Execution(format!("Timestamp {value} out of 
range"))
-        })?
-        .with_nanosecond(0);
     let value = match granularity {
-        "second" => value,
-        "minute" => value.and_then(|d| d.with_second(0)),
+        "millisecond" => value,
+        "microsecond" => value,
+        "second" => value.and_then(|d| d.with_nanosecond(0)),

Review Comment:
   Yea, it is more clear.



-- 
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]

Reply via email to