benbellick commented on code in PR #25498:
URL: https://github.com/apache/datafusion/pull/25498#discussion_r4054239396
##########
datafusion/functions/src/datetime/date_trunc.rs:
##########
@@ -1441,6 +1472,96 @@ 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",
+ ),
+ ];
+ for (value, granularity) in cases {
+ let (scalar, array) = date_trunc_scalar_and_array(granularity,
value.clone());
+ let scalar = scalar.unwrap_or_else(|e| {
+ panic!("scalar date_trunc('{granularity}', {value:?}) failed:
{e}")
+ });
+ assert_eq!(
+ scalar,
+ array.unwrap(),
+ "date_trunc('{granularity}', {value:?})"
+ );
+ }
+
+ // The issue's example: `to_timestamp_seconds(10000000000)`
Review Comment:
This isn't going to make sense as a comment once this is merged. I would
either include the issue number for future reference or get rid of the mention
of the issue, whatever is the standard in this repo (I'm not sure). Thanks!
--
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]
