benbellick commented on code in PR #25498:
URL: https://github.com/apache/datafusion/pull/25498#discussion_r4057571566


##########
datafusion/functions/src/datetime/date_trunc.rs:
##########
@@ -1441,6 +1468,98 @@ mod tests {
         }
     }
 
+    /// Evaluates `date_trunc(granularity, value)` once on a scalar and once 
on a
+    /// one-row column holding the same value.
+    fn date_trunc_scalar_and_array(
+        granularity: &str,
+        value: ScalarValue,
+    ) -> (
+        datafusion_common::Result<ScalarValue>,
+        datafusion_common::Result<ScalarValue>,
+    ) {
+        let invoke = |arg: ColumnarValue| {
+            let data_type = value.data_type();
+            let args = ScalarFunctionArgs {
+                args: 
vec![ColumnarValue::Scalar(ScalarValue::from(granularity)), arg],
+                arg_fields: vec![
+                    Field::new("a", DataType::Utf8, false).into(),
+                    Field::new("b", data_type.clone(), true).into(),
+                ],
+                number_rows: 1,
+                return_field: Field::new("f", data_type, true).into(),
+                config_options: Arc::new(ConfigOptions::default()),
+            };
+            match DateTruncFunc::new().invoke_with_args(args)? {
+                ColumnarValue::Scalar(result) => Ok(result),
+                ColumnarValue::Array(result) => 
ScalarValue::try_from_array(&result, 0),
+            }
+        };
+        (
+            invoke(ColumnarValue::Scalar(value.clone())),
+            invoke(ColumnarValue::Array(value.to_array().unwrap())),
+        )
+    }
+
+    /// A timestamp beyond the nanosecond range is truncated the same way as a
+    /// scalar and as a column: neither converts it to nanoseconds first.
+    #[test]
+    fn scalar_and_array_accept_timestamps_beyond_nanosecond_range() {
+        // 2286-11-20T17:46:40, after the last nanosecond timestamp in 2262
+        let seconds = 10_000_000_000;
+        let utc: Option<Arc<str>> = Some("UTC".into());
+        let cases = [
+            (
+                ScalarValue::TimestampSecond(Some(seconds + 59), None),
+                "minute",
+            ),
+            (
+                ScalarValue::TimestampSecond(Some(seconds + 1), None),
+                "hour",
+            ),
+            (ScalarValue::TimestampSecond(Some(seconds + 1), None), "day"),
+            (ScalarValue::TimestampSecond(Some(seconds), None), "second"),
+            (
+                ScalarValue::TimestampSecond(Some(seconds + 59), utc.clone()),
+                "minute",
+            ),
+            (
+                ScalarValue::TimestampMillisecond(Some(seconds * 1_000 + 999), 
None),
+                "second",
+            ),
+            (
+                ScalarValue::TimestampMicrosecond(Some(seconds * 1_000_000 + 
999), None),
+                "millisecond",
+            ),
+            (
+                ScalarValue::TimestampMicrosecond(Some(-seconds * 1_000_000 - 
1), utc),
+                "second",
+            ),
+        ];

Review Comment:
   Great tests! I think they could benefit from some comments or some named 
comments to make them a bit easier to recognize their intent on a quick glance. 



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

Reply via email to