Support unix_timestamp and to_unix_timestamp with non-UTC timezones (non-DST) #9816

winningsix · 2023-11-21T12:05:03Z

TODOs:

Support ImprovedTimestamp path
Fix mis-matched CPU/GPU execution result

Signed-off-by: Ferdinand Xu <ferdinandx@nvidia.com>

res-life · 2023-11-22T07:29:39Z

integration_tests/src/main/python/date_time_test.py

@@ -262,6 +262,13 @@ def test_unsupported_fallback_to_unix_timestamp(data_gen):
        "to_unix_timestamp(a, b)"),
        "ToUnixTimestamp")

+# TODO: has another test for this called test_unix_timestamp
+@pytest.mark.parametrize('data_gen', date_n_time_gens, ids=idfn)


We should also test string gen.
Please refer to the Product plan doc, it requires String data type.

NVnavkumar · 2023-12-01T19:36:53Z

Also closes #9606

winningsix · 2023-12-08T13:11:07Z

build

sperlingxx · 2023-12-11T01:13:05Z

LGTM

res-life · 2023-12-11T02:28:59Z

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/datetimeExpressions.scala

+            failOnError)
+        }
+      case _: DateType =>
+        timeZoneId match {


Use zoneId defined in TimeZoneAwareExpression

@transient lazy val zoneId: ZoneId = DateTimeUtils.getZoneId(timeZoneId.get)

Still timeZoneId to match non case but use it later.

res-life · 2023-12-11T02:29:05Z

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/datetimeExpressions.scala

@@ -892,8 +909,20 @@ abstract class GpuToTimestampImproved extends GpuToTimestamp {
          failOnError)
      }
    } else if (lhs.dataType() == DateType){
-      lhs.getBase.asTimestampSeconds()
-    } else { // Timestamp
+      timeZoneId match {


Use zoneId defined in TimeZoneAwareExpression

@transient lazy val zoneId: ZoneId = DateTimeUtils.getZoneId(timeZoneId.get)

res-life · 2023-12-11T02:34:21Z

integration_tests/src/main/python/date_time_test.py

 def test_unix_timestamp(data_gen):
    assert_gpu_and_cpu_are_equal_collect(
-            lambda spark : unary_op_df(spark, data_gen).select(f.unix_timestamp(f.col('a'))))


Add cases for not supported Time Zone.

e.g.:

@pytest.mark.skipif(not is_supported_time_zone(), reason="not all time zones are supported now, refer to https://github.com/NVIDIA/spark-rapids/issues/6839, please update after all time zones are supported") def test_from_unixtime(data_gen, date_format): @pytest.mark.skipif(is_supported_time_zone(), reason="not all time zones are supported now, refer to https://github.com/NVIDIA/spark-rapids/issues/6839, please update after all time zones are supported") def test_from_unixtime_fall_back(data_gen, date_format):

And test TZs locally:
export TZ=Iran
export TZ= America/Los_Angeles

winningsix · 2023-12-11T15:17:26Z

build

res-life · 2023-12-12T02:03:28Z

LGTM

winningsix · 2023-12-13T12:49:03Z

build

NVnavkumar

LGTM. Eventually, need to do cleanup based on NVIDIA/spark-rapids-jni#1643

revans2

I know this just got merged, but we need to go back and fix the tests. I am fine if it is a fast follow on, otherwise we should revert it and do it right. The ProjectExec allowed to not be on the GPU in all test cases is not acceptable.

revans2 · 2023-12-13T16:07:16Z

integration_tests/src/main/python/date_time_test.py

-@allow_non_gpu(*non_utc_allow)
-def test_string_to_unix_timestamp(data_gen, date_form, ansi_enabled):
+@allow_non_gpu('ProjectExec')
+def test_string_to_unix_timestamp(data_gen, date_form, ansi_enabled, non_utc_timezone_enabled):


This parameter is not used and nonUTC.enabled is going away.

revans2 · 2023-12-13T16:09:05Z

integration_tests/src/main/python/date_time_test.py

 @pytest.mark.parametrize('data_gen,date_form', str_date_and_format_gen, ids=idfn)
-@allow_non_gpu(*non_utc_allow)
-def test_string_to_unix_timestamp(data_gen, date_form, ansi_enabled):
+@allow_non_gpu('ProjectExec')


I don't want to have a hard coded ProjectExec that does not know if we should fall back to the CPU or not!.

revans2 · 2023-12-13T16:09:21Z

integration_tests/src/main/python/date_time_test.py

 @pytest.mark.parametrize('data_gen,date_form', str_date_and_format_gen, ids=idfn)
-@allow_non_gpu(*non_utc_allow)
-def test_string_unix_timestamp(data_gen, date_form, ansi_enabled):
+@allow_non_gpu('ProjectExec')


winningsix added 9 commits November 21, 2023 17:25

Support current_date with timezone

3faacd8

Signed-off-by: Ferdinand Xu <ferdinandx@nvidia.com>

Remove type based timezone checker

9ed56ea

Add timezone checker for expressions

c7722f9

Add check for cast

a37d1ce

code style fix

e26e299

Fix premerge fails

3696466

Fix

ce90809

Refine comments

3566f1c

Support unix_timestamp with non-DST timezones

749cfbf

Signed-off-by: Ferdinand Xu <ferdinandx@nvidia.com>

winningsix changed the title ~~[WIP ] Support Unix_timestamp with non-UTC timezones (non-DST)~~ [WIP] Support Unix_timestamp with non-UTC timezones (non-DST) Nov 21, 2023

winningsix changed the title ~~[WIP] Support Unix_timestamp with non-UTC timezones (non-DST)~~ [WIP] Support Unix_timestamp with non-UTC timezones (non-DST) [skip ci] Nov 21, 2023

res-life reviewed Nov 22, 2023

View reviewed changes

winningsix changed the title ~~[WIP] Support Unix_timestamp with non-UTC timezones (non-DST) [skip ci]~~ [WIP] Support Unix_timestamp with non-UTC timezones (non-DST) Nov 22, 2023

sameerz added the feature request New feature or request label Nov 28, 2023

winningsix changed the base branch from branch-23.12 to branch-24.02 November 28, 2023 01:58

winningsix added 7 commits December 6, 2023 18:42

Merge branch 'branch-24.02' into unix_timestamp

f239624

Fix issue after merge

465f0f3

Fix

877c9eb

Merge branch 'branch-24.02' into unix_timestamp

83fee5b

Fix merge issue

fa439da

Update test cases

8ed6ff0

Refine

3b72923

winningsix marked this pull request as ready for review December 8, 2023 13:09

winningsix changed the title ~~[WIP] Support Unix_timestamp with non-UTC timezones (non-DST)~~ Support Unix_timestamp and to_unix_timestamp with non-UTC timezones (non-DST) Dec 8, 2023

winningsix changed the title ~~Support Unix_timestamp and to_unix_timestamp with non-UTC timezones (non-DST)~~ Support nix_timestamp and to_unix_timestamp with non-UTC timezones (non-DST) Dec 8, 2023

winningsix changed the title ~~Support nix_timestamp and to_unix_timestamp with non-UTC timezones (non-DST)~~ Support unix_timestamp and to_unix_timestamp with non-UTC timezones (non-DST) Dec 8, 2023

res-life reviewed Dec 11, 2023

View reviewed changes

Enable fallback case, address comments

003cf3a

res-life previously approved these changes Dec 12, 2023

View reviewed changes

NVnavkumar linked an issue Dec 13, 2023 that may be closed by this pull request

[FEA] Support unix_timestamp with CST(China Time Zone) support #9606

Closed

Merge branch 'branch-24.02' into unix_timestamp

0752ffc

winningsix dismissed res-life’s stale review via 0752ffc December 13, 2023 12:48

NVnavkumar approved these changes Dec 13, 2023

View reviewed changes

NVnavkumar merged commit 798682a into NVIDIA:branch-24.02 Dec 13, 2023
37 of 38 checks passed

revans2 requested changes Dec 13, 2023

View reviewed changes

revans2 mentioned this pull request Dec 13, 2023

Update tests to not have hard coded fallback when not needed #10042

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support unix_timestamp and to_unix_timestamp with non-UTC timezones (non-DST) #9816

Support unix_timestamp and to_unix_timestamp with non-UTC timezones (non-DST) #9816

winningsix commented Nov 21, 2023 •

edited

res-life Nov 22, 2023

NVnavkumar commented Dec 1, 2023

winningsix commented Dec 8, 2023

sperlingxx commented Dec 11, 2023

res-life Dec 11, 2023

winningsix Dec 11, 2023

res-life Dec 11, 2023

winningsix Dec 11, 2023

res-life Dec 11, 2023

winningsix Dec 11, 2023

winningsix commented Dec 11, 2023

res-life commented Dec 12, 2023

winningsix commented Dec 13, 2023

NVnavkumar left a comment

revans2 left a comment •

edited

revans2 Dec 13, 2023

revans2 Dec 13, 2023

revans2 Dec 13, 2023

Support unix_timestamp and to_unix_timestamp with non-UTC timezones (non-DST) #9816

Support unix_timestamp and to_unix_timestamp with non-UTC timezones (non-DST) #9816

Conversation

winningsix commented Nov 21, 2023 • edited

Choose a reason for hiding this comment

NVnavkumar commented Dec 1, 2023

winningsix commented Dec 8, 2023

sperlingxx commented Dec 11, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

winningsix commented Dec 11, 2023

res-life commented Dec 12, 2023

winningsix commented Dec 13, 2023

NVnavkumar left a comment

Choose a reason for hiding this comment

revans2 left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

winningsix commented Nov 21, 2023 •

edited

revans2 left a comment •

edited