Support casting `Date` to a time zone-specific timestamp #7141

friendlymatthew · 2025-02-16T05:00:35Z

Closes #7140. Related: apache/datafusion#14638

Currently, you can only cast Date32/Date64 into a Timestamp without a time zone. This commit updates the casting rules to support date to time-zone specific timestamps and remodels the casting logic to support this feature.

There are no user facing changes.

arrow-cast/src/cast/mod.rs

friendlymatthew · 2025-02-16T17:53:43Z

This is ready for review. The test cases comprehensively cover the entire Date × TimeUnit matrix. Also tested via the datafusion-cli. See apache/datafusion#14638 (comment)

cc/ @xudong963 @Omega359

Omega359

From a quick look over this looks nice

arrow-cast/src/cast/mod.rs

xudong963 · 2025-02-17T15:31:14Z

arrow-cast/src/cast/mod.rs

+    #[test]
+    fn test_cast_date32_to_timestamp_with_timezone() {
+        let tz = "+0545"; // UTC + 0545 is Asia/Kathmandu
+        let a = Date32Array::from(vec![Some(18628), Some(18993), None]); // 2021-1-1, 2022-1-1


So the default tz of date is UTC, right?

So the default tz of date is UTC, right?

Yes, but shouldn't we ensure Dates cast to timestamps with a particular timezone?

I'm also a bit confused by this comment. Are you implying that a is improperly constructed? (i.e. the third element shouldn't be None?) Here's a similar test case that tests for time zone aware timestamps:

arrow-rs/arrow-cast/src/cast/mod.rs

Lines 5277 to 5291 in 38d6e69

let array_with_tz =

TimestampMillisecondArray::from(vec![Some(864000003005), Some(1545696002001), None])

.with_timezone(tz.to_string());

let expected = vec![

Some("1997-05-19 05:45:03.005000"),

Some("2018-12-25 05:45:02.001000"),

None,

];

assert_cast_timestamp_to_string!(

array_with_tz,

DataType::Utf8View,

StringViewArray,

cast_options,

expected

);

Maybe I'm misunderstanding something. Would appreciate some clarification. Thank you!

The test is okay for me. Thanks!

I'm just curious about which particular timezone we should use. Should we depend on a time zone configuration or use the UTC directly?

I'm just curious about which particular timezone we should use. Should we depend on a time zone configuration or use the UTC directly?

Other test cases use random time zones like the +0545 mentioned above. I'd argue that for the sake of testing, we'd want to use a non-default time zone.

tustvold · 2025-02-19T09:42:31Z

Thank you for this PR, and especially for the tests which make it clear what this is doing.

I do have one question, is this the expected behaviour?

In particular when casting timestamps without a timezone to timestamps with a timezone, the conversion is done preserving the "local" time.

i.e.

2021-01-01 05:45:00.000000 -> 2021-01-01 05:45:00.000000 +0545

I therefore wonder if date conversions should behave similarly, i.e.

2021-01-01 -> 2021-01-01 00:00:00.000000 +0545

instead of what this PR implements which interprets the date as being in the UTC timezone

2021-01-01 -> 2021-01-01 05:45:00.000000 +0545

With the behaviour in this PR I think you would get different behaviour going Date32 -> Timestamp(_, None) -> Timestamp(_, Some(_))? (It might be worth testing this regardless)

friendlymatthew · 2025-02-19T19:07:59Z

I do have one question, is this the expected behaviour?

In particular when casting timestamps without a timezone to timestamps with a timezone, the conversion is done preserving the "local" time.

i.e.

2021-01-01 05:45:00.000000 -> 2021-01-01 05:45:00.000000 +0545

I therefore wonder if date conversions should behave similarly, i.e.

2021-01-01 -> 2021-01-01 00:00:00.000000 +0545

Interesting! Yeah, I agree that we should preserve the local time when casting. c19dbc5 resolves this.

Now, when we cast a date to a time zone aware timestamp, we do a intermediary cast into a regular Timestamp, then attempt to recast with the time zone.

/cc @tustvold @Omega359

friendlymatthew · 2025-02-19T19:11:10Z

arrow-cast/src/cast/mod.rs

+        (Date64, Timestamp(TimeUnit::Second, _)) => {
+            let array = array
                .as_primitive::<Date64Type>()
-                .unary::<_, TimestampSecondType>(|x| x / MILLISECONDS)
-                .with_timezone_opt(to_tz.clone()),
-        )),
-        (Date64, Timestamp(TimeUnit::Millisecond, to_tz)) => Ok(Arc::new(
-            array
+                .unary::<_, TimestampSecondType>(|x| x / MILLISECONDS);
+
+            cast_with_options(&array, to_type, cast_options)
+        }


We can split into two arms if we don't like how we're eagerly recasting a Timestamp.

(Date64, Timestamp(TimeUnit::Second, None)) => { // single cast here }, (Date64, Timestamp(TimeUnit::Second, Some(_))) => { // first cast to Timestamp, then recast with time zone }

friendlymatthew · 2025-02-19T19:11:53Z

arrow-cast/src/cast/mod.rs

+    #[test]
+    fn test_cast_date32_to_timestamp_and_timestamp_with_timezone() {
+        let tz = "+0545"; // UTC + 0545 is Asia/Kathmandu
+        let a = Date32Array::from(vec![Some(18628), None, None]); // 2021-1-1, 2022-1-1
+        let array = Arc::new(a) as ArrayRef;
+
+        let b = cast(
+            &array,
+            &DataType::Timestamp(TimeUnit::Second, Some(tz.into())),
+        )
+        .unwrap();
+        let c = b.as_primitive::<TimestampSecondType>();
+        let string_array = cast(&c, &DataType::Utf8).unwrap();
+        let result = string_array.as_string::<i32>();
+        assert_eq!("2021-01-01T00:00:00+05:45", result.value(0));
+
+        let b = cast(&array, &DataType::Timestamp(TimeUnit::Second, None)).unwrap();
+        let c = b.as_primitive::<TimestampSecondType>();
+        let string_array = cast(&c, &DataType::Utf8).unwrap();
+        let result = string_array.as_string::<i32>();
+        assert_eq!("2021-01-01T00:00:00", result.value(0));
+    }


Proof that casting a Date to a Timestamp with/without time zones result in the same string.

Not the biggest fan of this test case, would appreciate some comments if we would want to a test case that compares the cast results (with/without time zone).

friendlymatthew · 2025-02-21T02:27:26Z

Weird, clippy seems to complain about a completely different region of the codebase?

/cc @xudong963

xudong963 · 2025-02-21T02:31:56Z

Weird, clippy seems to complain about a completely different region of the codebase?

/cc @xudong963

FYI: #7167, maybe you can do an update.

friendlymatthew · 2025-02-21T03:50:41Z

Weird, clippy seems to complain about a completely different region of the codebase?
/cc @xudong963

FYI: #7167, maybe you can do an update.

Should be gtg

alamb

Thank you @friendlymatthew and @xudong963 and @Omega359 for the reviews

I think it is reasonable to preserve the timezone when casting from a Date (that doesn't have any timezone) to a timestamp with a timezone. I tried to think of some cases where this wouldn't make sense or we would have had issues with edge cases, but I could not.

Looks well tested as well 🏆

friendlymatthew · 2025-02-23T14:50:52Z

Thank you @friendlymatthew and @xudong963 and @Omega359 for the reviews

I think it is reasonable to preserve the timezone when casting from a Date (that doesn't have any timezone) to a timestamp with a timezone. I tried to think of some cases where this wouldn't make sense or we would have had issues with edge cases, but I could not.

Looks well tested as well 🏆

Thank you @tustvold for the review and clarifying the expected behavior!

github-actions bot added the arrow Changes to the arrow crate label Feb 16, 2025

friendlymatthew mentioned this pull request Feb 16, 2025

Feature: support cast date to timestamp with tz apache/datafusion#14638

Open

friendlymatthew commented Feb 16, 2025

View reviewed changes

arrow-cast/src/cast/mod.rs Outdated Show resolved Hide resolved

friendlymatthew force-pushed the friendlymatthew/cast-date-to-timestamp-tz branch from 717ed28 to 02a86ac Compare February 16, 2025 17:29

Omega359 reviewed Feb 16, 2025

View reviewed changes

xudong963 reviewed Feb 17, 2025

View reviewed changes

arrow-cast/src/cast/mod.rs Show resolved Hide resolved

xudong963 reviewed Feb 17, 2025

View reviewed changes

xudong963 requested a review from alamb February 18, 2025 05:03

friendlymatthew commented Feb 19, 2025

View reviewed changes

friendlymatthew added 3 commits February 20, 2025 22:48

Date can cast to tz-aware timestamp

c7af4b1

Cover other Date x TimeUnit cases

1b47275

Preserve local time when casting date to timestamp

7828b6d

friendlymatthew force-pushed the friendlymatthew/cast-date-to-timestamp-tz branch from c37b1a4 to 7828b6d Compare February 21, 2025 03:49

alamb approved these changes Feb 23, 2025

View reviewed changes

alamb changed the title ~~Date data types can cast to a time zone-specific timestamp~~ Support casting Date to a time zone-specific timestamp Feb 24, 2025

alamb merged commit 493d3ee into apache:main Feb 24, 2025
27 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support casting `Date` to a time zone-specific timestamp #7141

Support casting `Date` to a time zone-specific timestamp #7141

friendlymatthew commented Feb 16, 2025 •

edited

Loading

friendlymatthew commented Feb 16, 2025 •

edited

Loading

Omega359 left a comment

xudong963 Feb 17, 2025

friendlymatthew Feb 17, 2025

xudong963 Feb 18, 2025

friendlymatthew Feb 18, 2025

tustvold commented Feb 19, 2025 •

edited

Loading

friendlymatthew commented Feb 19, 2025

friendlymatthew Feb 19, 2025

friendlymatthew Feb 19, 2025

friendlymatthew commented Feb 21, 2025 •

edited

Loading

xudong963 commented Feb 21, 2025

friendlymatthew commented Feb 21, 2025

alamb left a comment

friendlymatthew commented Feb 23, 2025

	let array_with_tz =
	TimestampMillisecondArray::from(vec![Some(864000003005), Some(1545696002001), None])
	.with_timezone(tz.to_string());
	let expected = vec![
	Some("1997-05-19 05:45:03.005000"),
	Some("2018-12-25 05:45:02.001000"),
	None,
	];
	assert_cast_timestamp_to_string!(
	array_with_tz,
	DataType::Utf8View,
	StringViewArray,
	cast_options,
	expected
	);

Support casting Date to a time zone-specific timestamp #7141

Support casting Date to a time zone-specific timestamp #7141

Conversation

friendlymatthew commented Feb 16, 2025 • edited Loading

friendlymatthew commented Feb 16, 2025 • edited Loading

Omega359 left a comment

Choose a reason for hiding this comment

xudong963 Feb 17, 2025

Choose a reason for hiding this comment

friendlymatthew Feb 17, 2025

Choose a reason for hiding this comment

xudong963 Feb 18, 2025

Choose a reason for hiding this comment

friendlymatthew Feb 18, 2025

Choose a reason for hiding this comment

tustvold commented Feb 19, 2025 • edited Loading

friendlymatthew commented Feb 19, 2025

friendlymatthew Feb 19, 2025

Choose a reason for hiding this comment

friendlymatthew Feb 19, 2025

Choose a reason for hiding this comment

friendlymatthew commented Feb 21, 2025 • edited Loading

xudong963 commented Feb 21, 2025

friendlymatthew commented Feb 21, 2025

alamb left a comment

Choose a reason for hiding this comment

friendlymatthew commented Feb 23, 2025

Support casting `Date` to a time zone-specific timestamp #7141

Support casting `Date` to a time zone-specific timestamp #7141

friendlymatthew commented Feb 16, 2025 •

edited

Loading

friendlymatthew commented Feb 16, 2025 •

edited

Loading

tustvold commented Feb 19, 2025 •

edited

Loading

friendlymatthew commented Feb 21, 2025 •

edited

Loading