Support from_utc_timestamp on the GPU for non-UTC timezones (non-DST) #9810

NVnavkumar · 2023-11-21T06:31:24Z

Fixes #9802 and closes #6831

This uses the GpuTimeZoneDB from spark-rapids-jni to enable from_utc_timestamp on the GPU for non-UTC timezones (currently without DST).

Signed-off-by: Navin Kumar <[email protected]>

… executor, use this in tests Signed-off-by: Navin Kumar <[email protected]>

…u-timezone-non-repeating-transition

…nd hide behind a config flag when first expression using it is completed. Signed-off-by: Navin Kumar <[email protected]>

…ezone support Signed-off-by: Navin Kumar <[email protected]>

…n-DST timezones Signed-off-by: Navin Kumar <[email protected]>

integration_tests/src/main/python/date_time_test.py

sql-plugin/src/main/scala/com/nvidia/spark/rapids/RapidsConf.scala

winningsix · 2023-11-21T13:48:37Z

tests/src/test/scala/com/nvidia/spark/rapids/timezone/TimeZoneSuite.scala

-        inputCv,
-        ZoneId.of(zoneStr))
+    withResource(createColumnVector(epochSeconds)) { inputCv =>
+      val actualRet = if (useGPU) {


No need to compare CPU TimezoneDB result against Spark.

this includes part of another PR #9739 which should be merged first after NVIDIA/spark-rapids-jni#1553 is merged

Signed-off-by: Navin Kumar <[email protected]>

…u-from-utc-timestamp

NVnavkumar · 2023-11-22T20:05:01Z

build

NVnavkumar · 2023-11-22T20:06:33Z

build

winningsix

General looks good to me. A few minor comments left.

winningsix · 2023-11-23T08:43:02Z

tests/src/test/scala/com/nvidia/spark/rapids/timezone/TimeZoneSuite.scala

      val epochDays = getEpochDays(startYear, endYear)
      testFromDateToTimestamp(epochDays, zoneStr)
    }
  }
+


nit: revert this.

What did you indicate to revert?

extra line here.

Per style, we generally have a blank line at the end of every file btw.

winningsix · 2023-11-23T12:26:54Z

tests/src/test/scala/com/nvidia/spark/rapids/timezone/TimeZoneSuite.scala

+class TimeZoneSuite extends SparkQueryCompareTestSuite with BeforeAndAfterAll {
+  private val useGPU = true
+  private val testAllTimezones = false
+  private val testAllYears = false


nit: parameterized this in test cases below?

…u-from-utc-timestamp

…st proper fallback Signed-off-by: Navin Kumar <[email protected]>

Signed-off-by: Navin Kumar <[email protected]>

NVnavkumar · 2023-11-28T00:37:42Z

build

NVnavkumar · 2023-11-28T00:39:46Z

build

revans2

In general this looks good, but I really want to see us starting to run automated tests in other time zones. Have you tested the date_time_test.py changes with TZ set to a non-UTC time zone?

revans2 · 2023-11-28T20:12:30Z

integration_tests/src/main/python/date_time_test.py


 @pytest.mark.parametrize('time_zone', ["UTC", "Asia/Shanghai", "EST", "MST", "VST"], ids=idfn)
 @pytest.mark.parametrize('data_gen', [timestamp_gen], ids=idfn)
 @pytest.mark.xfail(condition = is_not_utc(), reason = 'xfail non-UTC time zone tests because of https://github.com/NVIDIA/spark-rapids/issues/9653')
 def test_from_utc_timestamp_supported_timezones(data_gen, time_zone):
-    # Remove spark.rapids.test.CPU.timezone configuration when GPU kernel is ready to really test on GPU
+    # TODO: Remove spark.rapids.sql.nonUTC.enabled configuration 


Is there an issue for this?

Created #9881 and added to timezone epic

NVnavkumar · 2023-11-28T22:58:46Z

In general this looks good, but I really want to see us starting to run automated tests in other time zones. Have you tested the date_time_test.py changes with TZ set to a non-UTC time zone?

I have tested it with other timezones. from_utc_timestamp explicitly doesn't actually care about the session timezone so it behaves pretty much the same way as before. But again with Xfail, will need to figure out if it does fail in the same way.

NVnavkumar · 2023-11-28T23:23:52Z

In general this looks good, but I really want to see us starting to run automated tests in other time zones. Have you tested the date_time_test.py changes with TZ set to a non-UTC time zone?

I have tested it with other timezones. from_utc_timestamp explicitly doesn't actually care about the session timezone so it behaves pretty much the same way as before. But again with Xfail, will need to figure out if it does fail in the same way.

Automated testing of non-UTC timezones should be captured by #9855 and #9633

NVnavkumar added 7 commits November 15, 2023 13:12

Updates to test suite to start using GpuTimeZoneDB

1fed95a

Signed-off-by: Navin Kumar <[email protected]>

Initialize the GPU timezone database in the plugin initialization for…

bb3e8e0

… executor, use this in tests Signed-off-by: Navin Kumar <[email protected]>

Merge branch 'branch-23.12' of github.com:NVIDIA/spark-rapids into gp…

9220316

…u-timezone-non-repeating-transition

Merge branch 'branch-23.12' of github.com:NVIDIA/spark-rapids into gp…

ff64818

…u-timezone-non-repeating-transition

Remove GpuTimeZoneDB init from plugin for now, eventually add again a…

8811320

…nd hide behind a config flag when first expression using it is completed. Signed-off-by: Navin Kumar <[email protected]>

Add plugin initialization code and change config flag for non UTC tim…

93f5660

…ezone support Signed-off-by: Navin Kumar <[email protected]>

Use GpuTimeZoneDB to run from_utc_timestamp on the GPU for non-UTC no…

1d4184d

…n-DST timezones Signed-off-by: Navin Kumar <[email protected]>

NVnavkumar self-assigned this Nov 21, 2023

winningsix reviewed Nov 21, 2023

View reviewed changes

Fix formatting of includes here

031c6c9

Signed-off-by: Navin Kumar <[email protected]>

NVnavkumar marked this pull request as ready for review November 22, 2023 18:21

NVnavkumar changed the title ~~[WIP] Support from_utc_timestamp on the GPU for non-UTC timezones (non-DST)~~ Support from_utc_timestamp on the GPU for non-UTC timezones (non-DST) Nov 22, 2023

NVnavkumar mentioned this pull request Nov 22, 2023

Update timezone test framework to support both GPU and CPU POC #9739

Closed

Merge branch 'branch-23.12' of github.com:NVIDIA/spark-rapids into gp…

afd9328

…u-from-utc-timestamp

NVnavkumar requested a review from revans2 November 22, 2023 18:51

winningsix reviewed Nov 23, 2023

View reviewed changes

NVnavkumar added 3 commits November 27, 2023 16:05

Merge branch 'branch-23.12' of github.com:NVIDIA/spark-rapids into gp…

c9ddf49

…u-from-utc-timestamp

Add some Olson timezones to fallback test and add config option to te…

e5deb4a

…st proper fallback Signed-off-by: Navin Kumar <[email protected]>

Add fallback test for when config option is not enabled

74dcf39

Signed-off-by: Navin Kumar <[email protected]>

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

winningsix approved these changes Nov 28, 2023

View reviewed changes

revans2 reviewed Nov 28, 2023

View reviewed changes

NVnavkumar merged commit 9af612f into NVIDIA:branch-23.12 Nov 28, 2023
37 checks passed

NVnavkumar mentioned this pull request Nov 28, 2023

[FEA] Support timestamp transitions to and from UTC for single time zones with no repeating rules #6831

Closed

NVnavkumar mentioned this pull request Nov 28, 2023

[FEA] Support FromUTCTimestamp on the GPU with a non-UTC time zone #9802

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support from_utc_timestamp on the GPU for non-UTC timezones (non-DST) #9810

Support from_utc_timestamp on the GPU for non-UTC timezones (non-DST) #9810

NVnavkumar commented Nov 21, 2023 •

edited

Loading

winningsix Nov 21, 2023

NVnavkumar Nov 21, 2023

NVnavkumar commented Nov 22, 2023

NVnavkumar commented Nov 22, 2023

winningsix left a comment

winningsix Nov 23, 2023

NVnavkumar Nov 27, 2023

winningsix Nov 28, 2023

NVnavkumar Nov 28, 2023

winningsix Nov 23, 2023

NVnavkumar commented Nov 28, 2023

NVnavkumar commented Nov 28, 2023

revans2 left a comment

revans2 Nov 28, 2023

NVnavkumar Nov 28, 2023

NVnavkumar commented Nov 28, 2023

NVnavkumar commented Nov 28, 2023 •

edited

Loading

Support from_utc_timestamp on the GPU for non-UTC timezones (non-DST) #9810

Support from_utc_timestamp on the GPU for non-UTC timezones (non-DST) #9810

Conversation

NVnavkumar commented Nov 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NVnavkumar commented Nov 22, 2023

NVnavkumar commented Nov 22, 2023

winningsix left a 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

Choose a reason for hiding this comment

NVnavkumar commented Nov 28, 2023

NVnavkumar commented Nov 28, 2023

revans2 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NVnavkumar commented Nov 28, 2023

NVnavkumar commented Nov 28, 2023 • edited Loading

NVnavkumar commented Nov 21, 2023 •

edited

Loading

NVnavkumar commented Nov 28, 2023 •

edited

Loading