[SPARK-35979][SQL] Return different timestamp literals based on the default timestamp type #33215

gengliangwang · 2021-07-05T08:49:12Z

What changes were proposed in this pull request?

For the timestamp literal, it should have the following behavior.

When spark.sql.timestampType is TIMESTAMP_NTZ: if there is no time zone part, return timestamp without time zone literal; otherwise, return timestamp with local time zone literal
When spark.sql.timestampType is TIMESTAMP_LTZ: return timestamp with local time zone literal

Why are the changes needed?

When the default timestamp type is TIMESTAMP_NTZ, the result of type literal should return TIMESTAMP_NTZ when there is no time zone part in the string.

From setion 5.3 "literal" of ANSI SQL standard 2011:

27) The declared type of a <timestamp literal> that does not specify <time zone interval> is TIMESTAMP(P) WITHOUT TIME ZONE, where P is the number of digits in <seconds fraction>, if specified, and 0 (zero) otherwise. The declared type of a <timestamp literal> that specifies <time zone interval> is TIMESTAMP(P) WITH TIME ZONE, where P is the number of digits in <seconds fraction>, if specified, and 0 (zero) otherwise.

Since we don't have "timestamp with time zone", we use timestamp with local time zone instead.

Does this PR introduce any user-facing change?

No, the new timestmap type and the default timestamp configuration is not released yet.

How was this patch tested?

Unit test

HyukjinKwon · 2021-07-05T09:41:54Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala

@@ -248,7 +248,7 @@ object DateTimeUtils {
   * @return timestamp segments, time zone id and whether the input is just time without a date. If
   *         the input string can't be parsed as timestamp, the result timestamp segments are empty.
   */
-  private def parseTimestampString(s: UTF8String): (Array[Int], Option[ZoneId], Boolean) = {
+  private[sql] def parseTimestampString(s: UTF8String): (Array[Int], Option[ZoneId], Boolean) = {


I think we can just remove "private[sql]" since we're in catalyst.

SparkQA · 2021-07-05T09:46:46Z

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45155/

SparkQA · 2021-07-05T10:49:06Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45161/

SparkQA · 2021-07-05T11:24:20Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45161/

SparkQA · 2021-07-05T11:51:31Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45168/

SparkQA · 2021-07-05T11:52:32Z

Test build #140643 has finished for PR 33215 at commit 043d99b.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-07-05T12:27:15Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45168/

cloud-fan · 2021-07-05T14:20:58Z

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/ExpressionParserSuite.scala

+
+      // If the timestamp string contains time zone, return a timestamp with local time zone literal
+      assertEqual("tImEstAmp '2019-01-16 20:50:00.567000+01:00'",
+        Literal(1547668200567000L, TimestampType))


This test is hard to read. Can we create the literal from a Instant?

Sure. It is from another test case in the same test suite.

SparkQA · 2021-07-05T14:23:09Z

Test build #140651 has finished for PR 33215 at commit 381db36.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-07-05T15:31:27Z

Test build #140657 has finished for PR 33215 at commit da7ce20.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-07-05T15:39:46Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45170/

SparkQA · 2021-07-05T16:12:38Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45170/

gengliangwang · 2021-07-05T16:54:20Z

Merging to master/branch-3.2

…efault timestamp type ### What changes were proposed in this pull request? For the timestamp literal, it should have the following behavior. 1. When `spark.sql.timestampType` is TIMESTAMP_NTZ: if there is no time zone part, return timestamp without time zone literal; otherwise, return timestamp with local time zone literal 2. When `spark.sql.timestampType` is TIMESTAMP_LTZ: return timestamp with local time zone literal ### Why are the changes needed? When the default timestamp type is TIMESTAMP_NTZ, the result of type literal should return TIMESTAMP_NTZ when there is no time zone part in the string. From setion 5.3 "literal" of ANSI SQL standard 2011: ``` 27) The declared type of a <timestamp literal> that does not specify <time zone interval> is TIMESTAMP(P) WITHOUT TIME ZONE, where P is the number of digits in <seconds fraction>, if specified, and 0 (zero) otherwise. The declared type of a <timestamp literal> that specifies <time zone interval> is TIMESTAMP(P) WITH TIME ZONE, where P is the number of digits in <seconds fraction>, if specified, and 0 (zero) otherwise. ``` Since we don't have "timestamp with time zone", we use timestamp with local time zone instead. ### Does this PR introduce _any_ user-facing change? No, the new timestmap type and the default timestamp configuration is not released yet. ### How was this patch tested? Unit test Closes #33215 from gengliangwang/tsLiteral. Authored-by: Gengliang Wang <gengliang@apache.org> Signed-off-by: Gengliang Wang <gengliang@apache.org> (cherry picked from commit 2fffec7) Signed-off-by: Gengliang Wang <gengliang@apache.org>

SparkQA · 2021-07-05T19:14:53Z

Test build #140659 has finished for PR 33215 at commit 4e90fb9.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

gengliangwang added 2 commits July 5, 2021 14:58

save for now

9cd9bb1

add tests

043d99b

gengliangwang requested review from MaxGekk and cloud-fan July 5, 2021 08:49

github-actions bot added the SQL label Jul 5, 2021

update conf

381db36

HyukjinKwon reviewed Jul 5, 2021

View reviewed changes

remove private[sql]

da7ce20

cloud-fan reviewed Jul 5, 2021

View reviewed changes

cloud-fan approved these changes Jul 5, 2021

View reviewed changes

update test case

4e90fb9

gengliangwang closed this in 2fffec7 Jul 5, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-35979][SQL] Return different timestamp literals based on the default timestamp type #33215

[SPARK-35979][SQL] Return different timestamp literals based on the default timestamp type #33215

gengliangwang commented Jul 5, 2021

HyukjinKwon Jul 5, 2021

gengliangwang Jul 5, 2021

SparkQA commented Jul 5, 2021

SparkQA commented Jul 5, 2021

SparkQA commented Jul 5, 2021

SparkQA commented Jul 5, 2021

SparkQA commented Jul 5, 2021

SparkQA commented Jul 5, 2021

cloud-fan Jul 5, 2021

gengliangwang Jul 5, 2021

SparkQA commented Jul 5, 2021

SparkQA commented Jul 5, 2021

SparkQA commented Jul 5, 2021

SparkQA commented Jul 5, 2021

gengliangwang commented Jul 5, 2021

SparkQA commented Jul 5, 2021

[SPARK-35979][SQL] Return different timestamp literals based on the default timestamp type #33215

[SPARK-35979][SQL] Return different timestamp literals based on the default timestamp type #33215

Conversation

gengliangwang commented Jul 5, 2021

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

HyukjinKwon Jul 5, 2021

Choose a reason for hiding this comment

gengliangwang Jul 5, 2021

Choose a reason for hiding this comment

SparkQA commented Jul 5, 2021

SparkQA commented Jul 5, 2021

SparkQA commented Jul 5, 2021

SparkQA commented Jul 5, 2021

SparkQA commented Jul 5, 2021

SparkQA commented Jul 5, 2021

cloud-fan Jul 5, 2021

Choose a reason for hiding this comment

gengliangwang Jul 5, 2021

Choose a reason for hiding this comment

SparkQA commented Jul 5, 2021

SparkQA commented Jul 5, 2021

SparkQA commented Jul 5, 2021

SparkQA commented Jul 5, 2021

gengliangwang commented Jul 5, 2021

SparkQA commented Jul 5, 2021