[spark] branch branch-3.0 updated: [SPARK-31038][SQL] Add checkValue for spark.sql.session.timeZone

[we...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

wenchen pushed a commit to branch branch-3.0
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/branch-3.0 by this push:
     new 1c165ee  [SPARK-31038][SQL] Add checkValue for spark.sql.session.timeZone
1c165ee is described below

commit 1c165eecd3948dc12db19827dfd326d32eb4e475
Author: Kent Yao <ya...@hotmail.com>
AuthorDate: Thu Mar 5 19:38:20 2020 +0800

    [SPARK-31038][SQL] Add checkValue for spark.sql.session.timeZone
    
    ### What changes were proposed in this pull request?
    
    The `spark.sql.session.timeZone` config can accept any string value including invalid time zone ids, then it will fail other queries that rely on the time zone. We should do the value checking in the set phase and fail fast if the zone value is invalid.
    
    ### Why are the changes needed?
    
    improve configuration
    ### Does this PR introduce any user-facing change?
    
    yes, will fail fast if the value is a wrong timezone id
    ### How was this patch tested?
    
    add ut
    
    Closes #27792 from yaooqinn/SPARK-31038.
    
    Authored-by: Kent Yao <ya...@hotmail.com>
    Signed-off-by: Wenchen Fan <we...@databricks.com>
---
 .../scala/org/apache/spark/sql/internal/SQLConf.scala |  8 ++++++++
 .../org/apache/spark/sql/internal/SQLConfSuite.scala  | 19 +++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
index 68a89b2..cc9c2ae 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
@@ -24,6 +24,7 @@ import java.util.zip.Deflater
 
 import scala.collection.JavaConverters._
 import scala.collection.immutable
+import scala.util.Try
 import scala.util.matching.Regex
 
 import org.apache.hadoop.fs.Path
@@ -38,6 +39,7 @@ import org.apache.spark.sql.catalyst.analysis.{HintErrorLogger, Resolver}
 import org.apache.spark.sql.catalyst.expressions.CodegenObjectFactoryMode
 import org.apache.spark.sql.catalyst.expressions.codegen.CodeGenerator
 import org.apache.spark.sql.catalyst.plans.logical.HintErrorHandler
+import org.apache.spark.sql.catalyst.util.DateTimeUtils
 import org.apache.spark.sql.connector.catalog.CatalogManager.SESSION_CATALOG_NAME
 import org.apache.spark.unsafe.array.ByteArrayMethods
 import org.apache.spark.util.Utils
@@ -1483,10 +1485,16 @@ object SQLConf {
     .doubleConf
     .createWithDefault(0.9)
 
+  private def isValidTimezone(zone: String): Boolean = {
+    Try { DateTimeUtils.getZoneId(zone) }.isSuccess
+  }
+
   val SESSION_LOCAL_TIMEZONE =
     buildConf("spark.sql.session.timeZone")
       .doc("""The ID of session local timezone, e.g. "GMT", "America/Los_Angeles", etc.""")
       .stringConf
+      .checkValue(isValidTimezone, s"Cannot resolve the given timezone with" +
+        " ZoneId.of(_, ZoneId.SHORT_IDS)")
       .createWithDefaultFunction(() => TimeZone.getDefault.getID)
 
   val WINDOW_EXEC_BUFFER_IN_MEMORY_THRESHOLD =
diff --git a/sql/core/src/test/scala/org/apache/spark/sql/internal/SQLConfSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/internal/SQLConfSuite.scala
index 61be367..48be211 100644
--- a/sql/core/src/test/scala/org/apache/spark/sql/internal/SQLConfSuite.scala
+++ b/sql/core/src/test/scala/org/apache/spark/sql/internal/SQLConfSuite.scala
@@ -348,4 +348,23 @@ class SQLConfSuite extends QueryTest with SharedSparkSession {
     }
     check(config2)
   }
+
+  test("spark.sql.session.timeZone should only accept valid zone id") {
+    spark.conf.set(SQLConf.SESSION_LOCAL_TIMEZONE.key, "MIT")
+    assert(sql(s"set ${SQLConf.SESSION_LOCAL_TIMEZONE.key}").head().getString(1) === "MIT")
+    spark.conf.set(SQLConf.SESSION_LOCAL_TIMEZONE.key, "America/Chicago")
+    assert(sql(s"set ${SQLConf.SESSION_LOCAL_TIMEZONE.key}").head().getString(1) ===
+      "America/Chicago")
+
+    intercept[IllegalArgumentException] {
+      spark.conf.set(SQLConf.SESSION_LOCAL_TIMEZONE.key, "pst")
+    }
+    intercept[IllegalArgumentException] {
+      spark.conf.set(SQLConf.SESSION_LOCAL_TIMEZONE.key, "GMT+8:00")
+    }
+    val e = intercept[IllegalArgumentException] {
+      spark.conf.set(SQLConf.SESSION_LOCAL_TIMEZONE.key, "Asia/shanghai")
+    }
+    assert(e.getMessage === "Cannot resolve the given timezone with ZoneId.of(_, ZoneId.SHORT_IDS)")
+  }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org