Support unix_timestamp on GPU for subset of formats #1113

andygrove · 2020-11-13T15:31:28Z

Signed-off-by: Andy Grove andygrove@nvidia.com

This PR makes improvements to unix_timestamp so that a small number of date/timestamp formats can now be supported on GPU without requiring that incompatibleOps=true because they are now compatible with Spark (with the exception of #1117).

Specific changes in this PR:

unix_timestamp and to_unix_timestamp no longer return bad data for invalid inputs when using one of the compatible formats because it first validates the contents of a column using is_timestamp and will either throw an exception or convert invalid inputs to null, depending on the value specified for legacyTimeParserPolicy.
A new configuration option spark.rapids.sql.incompatibleDateFormats.enabled has been added.
unix_timestamp now supports the special dates epoch, now, today, yesterday, and tomorrow.
unix_timestamp will fall back to CPU if legacyTimeParserPolicy is LEGACY, or if the supplied format is not supported on GPU and spark.rapids.sql.incompatibleDateFormats.enabled=false.
As a side effect of these changes, to_date is now supported on GPU because it gets translated to an expression that is a combination of CAST and unix_timestamp, using one of the compatible formats.

This PR closes #1098, #1094, and largely addresses #50. I filed a new issue #1111 for the remainder of the work to support legacyTimeParserPolicy.

Signed-off-by: Andy Grove <andygrove@nvidia.com>

andygrove · 2020-11-13T15:32:45Z

build

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

Signed-off-by: Andy Grove <andygrove@nvidia.com>

…Ops to be set Signed-off-by: Andy Grove <andygrove@nvidia.com>

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

Signed-off-by: Andy Grove <andygrove@nvidia.com>

andygrove · 2020-11-16T21:25:39Z

Thanks for the review @revans2. I have now addressed the remaining issues.

revans2

looks great

revans2 · 2020-11-16T22:03:41Z

build

andygrove · 2020-11-16T23:16:58Z

The PySpark tests failed. I will either need to modify the config to enable incompatible formats or add more formats to the list of compatible formats.

revans2 · 2020-11-17T15:09:20Z

The PySpark tests failed. I will either need to modify the config to enable incompatible formats or add more formats to the list of compatible formats.

There are only two formats that are being tested,

'yyyy/MM' and 'yyyy/MM/dd'

One of the tests is marked as xfail because we allow invalid dates to be inserted.

The tests are already marked with incompat because to_unix_timestamp is not compatible so I am fine with either fix you suggested. I personally would like to see more formats being marked as compatible. Perhaps a disallow list in the parser would be better so we can support everything except for the one variable width one we know of.

…mestamp Signed-off-by: Andy Grove <andygrove@nvidia.com>

Signed-off-by: Andy Grove <andygrove@nvidia.com>

andygrove · 2020-11-17T17:36:04Z

integration_tests/src/main/python/date_time_test.py

 @pytest.mark.parametrize('data_gen,date_form', str_date_and_format_gen, ids=idfn)
 def test_string_to_unix_timestamp(data_gen, date_form):
-    print("date: " + date_form)
+    conf = {"spark.rapids.sql.improvedTimeOps.enabled": "true"}


The test fails if I don't add this config and I don't understand why yet. I am debugging this still.

Signed-off-by: Andy Grove <andygrove@nvidia.com>

andygrove · 2020-11-17T19:55:20Z

build

Signed-off-by: Andy Grove <andygrove@nvidia.com>

andygrove · 2020-11-17T21:54:52Z

@revans2 This is ready for review now. I had misunderstood the correct behavior for legacyTimeParserPolicy=EXCEPTION. We should not throw exceptions for values that we fail to parse for a given format unless those inputs would have been valid in Spark versions prior to 3.0.0. This is not true of any of the formats that we currently support on GPU.

andygrove · 2020-11-18T03:44:36Z

build

andygrove · 2020-11-18T14:35:11Z

It looks like the behavior has changed in Spark 3.1.0 and unix_timestamp gets translated to cast(gettimestamp(c0#20108, yyyy-MM-dd, Some(UTC)) as date) which we do not currently support on GPU, which is why the tests are failing against 3.1.0 currently.

revans2

The code looks fine, but if we know that there are failures in 3.1.0 then we should at least file a follow-on issue for them and either skip the tests for spark 3.1.0+ or mark them as xfail

Signed-off-by: Andy Grove <andygrove@nvidia.com>

andygrove · 2020-11-18T15:15:44Z

build

* Support unix_timestamp on GPU for subset of formats Signed-off-by: Andy Grove <andygrove@nvidia.com> * close scalar value Signed-off-by: Andy Grove <andygrove@nvidia.com> * compatible formats will now run on GPU without requiring incompatibleOps to be set Signed-off-by: Andy Grove <andygrove@nvidia.com> * code cleanup and address more review comments Signed-off-by: Andy Grove <andygrove@nvidia.com> * add specific config option for enabling incompatible date formats on GPU * update documentation Signed-off-by: Andy Grove <andygrove@nvidia.com> * improve docs Signed-off-by: Andy Grove <andygrove@nvidia.com> * use constants for special dates Signed-off-by: Andy Grove <andygrove@nvidia.com> * Add support for more date formats and remove incompat from to_unix_timestamp Signed-off-by: Andy Grove <andygrove@nvidia.com> * remove debug print Signed-off-by: Andy Grove <andygrove@nvidia.com> * Revert unnecessary change Signed-off-by: Andy Grove <andygrove@nvidia.com> * Make ToUnixTimestamp consistent with UnixTimestamp Signed-off-by: Andy Grove <andygrove@nvidia.com> * refactor to remove duplicate code Signed-off-by: Andy Grove <andygrove@nvidia.com> * fix resource leaks and fix regressions in python tests Signed-off-by: Andy Grove <andygrove@nvidia.com> * scalstyle Signed-off-by: Andy Grove <andygrove@nvidia.com> * update docs Signed-off-by: Andy Grove <andygrove@nvidia.com> * fix error in handling of legacyTimeParserPolicy=EXCEPTION Signed-off-by: Andy Grove <andygrove@nvidia.com> * fix test failures against Spark 3.1.0 Signed-off-by: Andy Grove <andygrove@nvidia.com>

…IDIA#1113) Signed-off-by: spark-rapids automation <70000568+nvauto@users.noreply.github.com>

Support unix_timestamp on GPU for subset of formats

ab0d61f

Signed-off-by: Andy Grove <andygrove@nvidia.com>

andygrove self-assigned this Nov 13, 2020

revans2 reviewed Nov 13, 2020

View reviewed changes

close scalar value

d0768c7

Signed-off-by: Andy Grove <andygrove@nvidia.com>

sameerz added the feature request New feature or request label Nov 15, 2020

sameerz added this to the Nov 9 - Nov 20 milestone Nov 15, 2020

compatible formats will now run on GPU without requiring incompatible…

1c43f8d

…Ops to be set Signed-off-by: Andy Grove <andygrove@nvidia.com>

revans2 reviewed Nov 16, 2020

View reviewed changes

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/datetimeExpressions.scala Outdated Show resolved Hide resolved

andygrove added 3 commits November 16, 2020 12:02

code cleanup and address more review comments

4a8a922

Signed-off-by: Andy Grove <andygrove@nvidia.com>

add specific config option for enabling incompatible date formats on GPU

6b0e654

update documentation

1d15e6b

Signed-off-by: Andy Grove <andygrove@nvidia.com>

andygrove changed the title ~~[WIP] Support unix_timestamp on GPU for subset of formats~~ Support unix_timestamp on GPU for subset of formats Nov 16, 2020

andygrove added 2 commits November 16, 2020 14:22

improve docs

900e0ba

Signed-off-by: Andy Grove <andygrove@nvidia.com>

use constants for special dates

9021145

Signed-off-by: Andy Grove <andygrove@nvidia.com>

revans2 previously approved these changes Nov 16, 2020

View reviewed changes

Add support for more date formats and remove incompat from to_unix_ti…

94bfbf9

…mestamp Signed-off-by: Andy Grove <andygrove@nvidia.com>

andygrove dismissed revans2’s stale review via 94bfbf9 November 17, 2020 17:28

remove debug print

d6c7971

Signed-off-by: Andy Grove <andygrove@nvidia.com>

revans2 previously approved these changes Nov 17, 2020

View reviewed changes

Revert unnecessary change

b93fe90

Signed-off-by: Andy Grove <andygrove@nvidia.com>

andygrove dismissed revans2’s stale review via b93fe90 November 17, 2020 17:34

andygrove commented Nov 17, 2020

View reviewed changes

andygrove mentioned this pull request Nov 17, 2020

[FEA] Support wider range of date/time formats with unix_timestamp #1142

Open

andygrove added 5 commits November 17, 2020 11:45

Make ToUnixTimestamp consistent with UnixTimestamp

df64036

Signed-off-by: Andy Grove <andygrove@nvidia.com>

refactor to remove duplicate code

dda37b1

Signed-off-by: Andy Grove <andygrove@nvidia.com>

fix resource leaks and fix regressions in python tests

b6545be

Signed-off-by: Andy Grove <andygrove@nvidia.com>

scalstyle

ddf154a

Signed-off-by: Andy Grove <andygrove@nvidia.com>

update docs

85cfef7

Signed-off-by: Andy Grove <andygrove@nvidia.com>

fix error in handling of legacyTimeParserPolicy=EXCEPTION

30aa765

Signed-off-by: Andy Grove <andygrove@nvidia.com>

revans2 reviewed Nov 18, 2020

View reviewed changes

fix test failures against Spark 3.1.0

dbafb3a

Signed-off-by: Andy Grove <andygrove@nvidia.com>

revans2 approved these changes Nov 18, 2020

View reviewed changes

andygrove merged commit fd6b613 into NVIDIA:branch-0.3 Nov 18, 2020

andygrove deleted the unix_timestamp branch November 18, 2020 16:17

This was referenced Nov 18, 2020

[BUG] unix_timestamp on GPU returns invalid data for special dates #1094

Closed

[FEA] support spark.sql.legacy.timeParserPolicy #50

Closed

tgravescs pushed a commit to tgravescs/spark-rapids that referenced this pull request Nov 30, 2023

Update submodule cudf to ed4c021e5938be3b24daa2c84b08b6e410c2ccd5 (NV…

b54770c

…IDIA#1113) Signed-off-by: spark-rapids automation <70000568+nvauto@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support unix_timestamp on GPU for subset of formats #1113

Support unix_timestamp on GPU for subset of formats #1113

andygrove commented Nov 13, 2020 •

edited

Loading

andygrove commented Nov 13, 2020

andygrove commented Nov 16, 2020

revans2 left a comment

revans2 commented Nov 16, 2020

andygrove commented Nov 16, 2020

revans2 commented Nov 17, 2020

andygrove Nov 17, 2020

andygrove commented Nov 17, 2020

andygrove commented Nov 17, 2020 •

edited

Loading

andygrove commented Nov 18, 2020

andygrove commented Nov 18, 2020

revans2 left a comment

andygrove commented Nov 18, 2020

Support unix_timestamp on GPU for subset of formats #1113

Support unix_timestamp on GPU for subset of formats #1113

Conversation

andygrove commented Nov 13, 2020 • edited Loading

andygrove commented Nov 13, 2020

andygrove commented Nov 16, 2020

revans2 left a comment

Choose a reason for hiding this comment

revans2 commented Nov 16, 2020

andygrove commented Nov 16, 2020

revans2 commented Nov 17, 2020

andygrove Nov 17, 2020

Choose a reason for hiding this comment

andygrove commented Nov 17, 2020

andygrove commented Nov 17, 2020 • edited Loading

andygrove commented Nov 18, 2020

andygrove commented Nov 18, 2020

revans2 left a comment

Choose a reason for hiding this comment

andygrove commented Nov 18, 2020

andygrove commented Nov 13, 2020 •

edited

Loading

andygrove commented Nov 17, 2020 •

edited

Loading