[SPARK-35975][SQL] New configuration `spark.sql.timestampType` for the default timestamp type #33176

gengliangwang · 2021-07-01T16:13:58Z

What changes were proposed in this pull request?

Add a new configuration spark.sql.timestampType, which configures the default timestamp type of Spark SQL, including SQL DDL and Cast clause. Setting the configuration as TIMESTAMP_NTZ will use TIMESTAMP WITHOUT TIME ZONE as the default type while putting it as TIMESTAMP_LTZ will use TIMESTAMP WITH LOCAL TIME ZONE.

The default value of the new configuration is TIMESTAMP_LTZ, which is consistent with previous Spark releases.

Why are the changes needed?

A new configuration for switching the default timestamp type as timestamp without time zone.

Does this PR introduce any user-facing change?

No, it's a new feature.

How was this patch tested?

Unit test

gengliangwang · 2021-07-01T16:15:44Z

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala

+
+  val TIMESTAMP_TYPE =
+    buildConf("spark.sql.timestampType")
+      .doc("Configures the default timestamp type of Spark SQL, including SQL DDL and Cast " +


The scope is bigger than this. It will affect timestamp literal, function to_timestamp and data source io, etc. We will get to that later.

MaxGekk · 2021-07-01T16:20:01Z

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala

+
+  val TIMESTAMP_TYPE =
+    buildConf("spark.sql.timestampType")
+      .doc("Configures the default timestamp type of Spark SQL, including SQL DDL and Cast " +


I wonder about types literals, for instance timestamp'2021-07-01 01:02:03'. Should the config influence on it too?

Yes, I mentioned in #33176 (comment). The type literal logic with TIMESTAMP_NTZ:

if there is no time zone part, return timestamp without time zone literal

otherwise, return timestamp with local time zone.

MaxGekk · 2021-07-01T16:21:29Z

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala

+  val TIMESTAMP_TYPE =
+    buildConf("spark.sql.timestampType")
+      .doc("Configures the default timestamp type of Spark SQL, including SQL DDL and Cast " +
+        "clause. Setting the configuration as TIMESTAMP_NTZ will use TIMESTAMP WITHOUT TIME " +


Can't you get the name from enum as you did that in .createWithDefault(TimestampTypes.TIMESTAMP_LTZ.toString). Maybe we will rename it again soon, who knows ;-)

MaxGekk · 2021-07-01T16:25:55Z

sql/core/src/test/scala/org/apache/spark/sql/TimestampTypeSuite.scala

+import org.apache.spark.util.ResetSystemProperties
+
+// Test suite for setting the default timestamp type of Spark SQL
+class TimestampTypeSuite extends QueryTest with SharedSparkSession with ResetSystemProperties {


We already have DataTypeSuite, so, could put the tests there, or you plan so many tests for TimestampType?

I think we can simply write tests in DataTypeParserSuite

if we apply the config in timestamp literals later, we should add tests in ExpressionParserSuite

This is end-to-end test with actual queries, which can't be done in DataTypeSuite.
I just renamed the file to SetDefaultTimestampTypeSuite to avoid the confusion.

@cloud-fan oh I missed your comments. Let me write tests in DataTypeParserSuite

MaxGekk · 2021-07-01T16:30:07Z

@gengliangwang Could you mention what is the default value of spark.sql.timestampType in the PR description.
In general it LGTM.

SparkQA · 2021-07-01T17:20:11Z

Kubernetes integration test unable to build dist.

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

cloud-fan · 2021-07-01T17:29:49Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala

@@ -2502,7 +2502,7 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with SQLConfHelper with Logg
      case ("float" | "real", Nil) => FloatType
      case ("double", Nil) => DoubleType
      case ("date", Nil) => DateType
-      case ("timestamp", Nil) => TimestampType
+      case ("timestamp", Nil) => SQLConf.get.timestampType


We need to support parsing type string for LTZ and NTZ specifically so that people can avoid relying on config in their queries.

I would prefer doing that in separate PRs to keep this one clean. We should support the new keyword both on data type parsing and the type literals.
https://issues.apache.org/jira/browse/SPARK-35977
https://issues.apache.org/jira/browse/SPARK-35978

MaxGekk · 2021-07-01T18:17:53Z

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala

+  val TIMESTAMP_TYPE =
+    buildConf("spark.sql.timestampType")
+      .doc("Configures the default timestamp type of Spark SQL, including SQL DDL and Cast " +
+        s"clause. Setting the configuration as ${TimestampTypes.TIMESTAMP_NTZ.toString} will " +


I thought the compiler automatically calls the toString method in string interpolations, so, you can skip it. Or I am wrong?

SparkQA · 2021-07-01T18:30:46Z

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

dongjoon-hyun

+1, LGTM (Pending CIs)

SparkQA · 2021-07-01T19:05:17Z

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

SparkQA · 2021-07-01T19:27:15Z

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

SparkQA · 2021-07-01T20:02:34Z

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

MaxGekk · 2021-07-01T20:24:36Z

+1, LGTM. Merging to master.
Thank you, @gengliangwang and @cloud-fan @dongjoon-hyun for reviews.

SparkQA · 2021-07-01T20:56:56Z

Test build #140527 has finished for PR 33176 at commit ad97b52.

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

dongjoon-hyun · 2021-07-01T20:58:01Z

Thank you, @gengliangwang and @MaxGekk !

SparkQA · 2021-07-01T21:57:57Z

Test build #140529 has finished for PR 33176 at commit a3c64e8.

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

SparkQA · 2021-07-01T23:15:02Z

Test build #140532 has finished for PR 33176 at commit 6c6bf3a.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
class DataTypeParserSuite extends SparkFunSuite with SQLHelper

new conf

ad97b52

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

gengliangwang requested review from MaxGekk, cloud-fan and dongjoon-hyun July 1, 2021 16:14

gengliangwang commented Jul 1, 2021

View reviewed changes

MaxGekk reviewed Jul 1, 2021

View reviewed changes

cloud-fan reviewed Jul 1, 2021

View reviewed changes

gengliangwang added 2 commits July 2, 2021 01:41

address comment

a3c64e8

update test

6c6bf3a

cloud-fan approved these changes Jul 1, 2021

View reviewed changes

MaxGekk reviewed Jul 1, 2021

View reviewed changes

MaxGekk approved these changes Jul 1, 2021

View reviewed changes

dongjoon-hyun approved these changes Jul 1, 2021

View reviewed changes

MaxGekk closed this in a643076 Jul 1, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-35975][SQL] New configuration `spark.sql.timestampType` for the default timestamp type #33176

[SPARK-35975][SQL] New configuration `spark.sql.timestampType` for the default timestamp type #33176

gengliangwang commented Jul 1, 2021 •

edited

gengliangwang Jul 1, 2021

MaxGekk Jul 1, 2021

gengliangwang Jul 1, 2021

MaxGekk Jul 1, 2021 •

edited

gengliangwang Jul 1, 2021

MaxGekk Jul 1, 2021

cloud-fan Jul 1, 2021

cloud-fan Jul 1, 2021

gengliangwang Jul 1, 2021

gengliangwang Jul 1, 2021

MaxGekk commented Jul 1, 2021

SparkQA commented Jul 1, 2021

cloud-fan Jul 1, 2021 •

edited

gengliangwang Jul 1, 2021 •

edited

MaxGekk Jul 1, 2021

SparkQA commented Jul 1, 2021

dongjoon-hyun left a comment

SparkQA commented Jul 1, 2021

SparkQA commented Jul 1, 2021

SparkQA commented Jul 1, 2021

MaxGekk commented Jul 1, 2021

SparkQA commented Jul 1, 2021

dongjoon-hyun commented Jul 1, 2021

SparkQA commented Jul 1, 2021

SparkQA commented Jul 1, 2021

[SPARK-35975][SQL] New configuration spark.sql.timestampType for the default timestamp type #33176

[SPARK-35975][SQL] New configuration spark.sql.timestampType for the default timestamp type #33176

Conversation

gengliangwang commented Jul 1, 2021 • edited

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?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaxGekk Jul 1, 2021 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaxGekk commented Jul 1, 2021

SparkQA commented Jul 1, 2021

cloud-fan Jul 1, 2021 • edited

Choose a reason for hiding this comment

gengliangwang Jul 1, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Jul 1, 2021

dongjoon-hyun left a comment

Choose a reason for hiding this comment

SparkQA commented Jul 1, 2021

SparkQA commented Jul 1, 2021

SparkQA commented Jul 1, 2021

MaxGekk commented Jul 1, 2021

SparkQA commented Jul 1, 2021

dongjoon-hyun commented Jul 1, 2021

SparkQA commented Jul 1, 2021

SparkQA commented Jul 1, 2021

[SPARK-35975][SQL] New configuration `spark.sql.timestampType` for the default timestamp type #33176

[SPARK-35975][SQL] New configuration `spark.sql.timestampType` for the default timestamp type #33176

gengliangwang commented Jul 1, 2021 •

edited

MaxGekk Jul 1, 2021 •

edited

cloud-fan Jul 1, 2021 •

edited

gengliangwang Jul 1, 2021 •

edited