This is an automated email from the ASF dual-hosted git repository.
maxgekk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new 9ec46a691a88 [SPARK-47087][SQL] Raise Spark's exception with an error
class in config value check
9ec46a691a88 is described below
commit 9ec46a691a880bac9f79b30c870dc99e9626101d
Author: Max Gekk <max.g...@gmail.com>
AuthorDate: Mon Feb 19 17:31:22 2024 +0300
[SPARK-47087][SQL] Raise Spark's exception with an error class in config
value check
### What changes were proposed in this pull request?
In the PR, I propose to extend the `TypedConfigBuilder` API by new method:
```scala
def checkValue(
validator: T => Boolean,
errorClass: String,
parameters: Map[String, String]): TypedConfigBuilder[T] = {
```
which raises `SparkIllegalArgumentException` with an error class when
`checkValue` fails.
As an example, I ported the check of the SQL config
`spark.sql.session.timeZone` on the new method.
### Why are the changes needed?
To improve user's experience with Spark SQL by migration on unified error
framework.
### Does this PR introduce _any_ user-facing change?
It can if user's code depends on particular format of error messages.
### How was this patch tested?
By running the affected test suites:
```
$ build/sbt "test:testOnly *SQLConfSuite"
$ build/sbt "core/testOnly *SparkThrowableSuite"
```
### Was this patch authored or co-authored using generative AI tooling?
No.
Closes #45156 from MaxGekk/fix-set-invalid-tz.
Authored-by: Max Gekk <max.g...@gmail.com>
Signed-off-by: Max Gekk <max.g...@gmail.com>
---
.../src/main/resources/error/error-classes.json | 13 ++++++++
.../spark/internal/config/ConfigBuilder.scala | 19 +++++++++++
...or-conditions-invalid-conf-value-error-class.md | 37 ++++++++++++++++++++++
docs/sql-error-conditions.md | 8 +++++
.../org/apache/spark/sql/internal/SQLConf.scala | 3 +-
.../sql-tests/analyzer-results/timezone.sql.out | 11 +++++--
.../resources/sql-tests/results/timezone.sql.out | 11 +++++--
.../apache/spark/sql/internal/SQLConfSuite.scala | 32 ++++++++++++-------
8 files changed, 117 insertions(+), 17 deletions(-)
diff --git a/common/utils/src/main/resources/error/error-classes.json
b/common/utils/src/main/resources/error/error-classes.json
index 38161ff87720..6c953174865f 100644
--- a/common/utils/src/main/resources/error/error-classes.json
+++ b/common/utils/src/main/resources/error/error-classes.json
@@ -1758,6 +1758,19 @@
],
"sqlState" : "42000"
},
+ "INVALID_CONF_VALUE" : {
+ "message" : [
+ "The value '<confValue>' in the config \"<confName>\" is invalid."
+ ],
+ "subClass" : {
+ "TIME_ZONE" : {
+ "message" : [
+ "Cannot resolve the given timezone."
+ ]
+ }
+ },
+ "sqlState" : "22022"
+ },
"INVALID_CURSOR" : {
"message" : [
"The cursor is invalid."
diff --git
a/core/src/main/scala/org/apache/spark/internal/config/ConfigBuilder.scala
b/core/src/main/scala/org/apache/spark/internal/config/ConfigBuilder.scala
index 954980dcb943..303d856ca2c5 100644
--- a/core/src/main/scala/org/apache/spark/internal/config/ConfigBuilder.scala
+++ b/core/src/main/scala/org/apache/spark/internal/config/ConfigBuilder.scala
@@ -22,6 +22,7 @@ import java.util.regex.PatternSyntaxException
import scala.util.matching.Regex
+import org.apache.spark.SparkIllegalArgumentException
import org.apache.spark.network.util.{ByteUnit, JavaUtils}
import org.apache.spark.util.Utils
@@ -111,6 +112,24 @@ private[spark] class TypedConfigBuilder[T](
}
}
+ /** Checks if the user-provided value for the config matches the validator.
+ * If it doesn't match, raise Spark's exception with the given error class.
*/
+ def checkValue(
+ validator: T => Boolean,
+ errorClass: String,
+ parameters: Map[String, String]): TypedConfigBuilder[T] = {
+ transform { v =>
+ if (!validator(v)) {
+ throw new SparkIllegalArgumentException(
+ errorClass = "INVALID_CONF_VALUE." + errorClass,
+ messageParameters = parameters ++ Map(
+ "confValue" -> v.toString,
+ "confName" -> parent.key))
+ }
+ v
+ }
+ }
+
/** Check that user-provided values for the config match a pre-defined set.
*/
def checkValues(validValues: Set[T]): TypedConfigBuilder[T] = {
transform { v =>
diff --git a/docs/sql-error-conditions-invalid-conf-value-error-class.md
b/docs/sql-error-conditions-invalid-conf-value-error-class.md
new file mode 100644
index 000000000000..ae0975e16116
--- /dev/null
+++ b/docs/sql-error-conditions-invalid-conf-value-error-class.md
@@ -0,0 +1,37 @@
+---
+layout: global
+title: INVALID_CONF_VALUE error class
+displayTitle: INVALID_CONF_VALUE error class
+license: |
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+---
+
+<!--
+ DO NOT EDIT THIS FILE.
+ It was generated automatically by `org.apache.spark.SparkThrowableSuite`.
+-->
+
+[SQLSTATE: 22022](sql-error-conditions-sqlstates.html#class-22-data-exception)
+
+The value '`<confValue>`' in the config "`<confName>`" is invalid.
+
+This error class has the following derived error classes:
+
+## TIME_ZONE
+
+Cannot resolve the given timezone.
+
+
diff --git a/docs/sql-error-conditions.md b/docs/sql-error-conditions.md
index f03a98e03d24..190cff3cbb1b 100644
--- a/docs/sql-error-conditions.md
+++ b/docs/sql-error-conditions.md
@@ -1080,6 +1080,14 @@ The datasource `<datasource>` cannot save the column
`<columnName>` because its
Column or field `<name>` is of type `<type>` while it's required to be
`<expectedType>`.
+###
[INVALID_CONF_VALUE](sql-error-conditions-invalid-conf-value-error-class.html)
+
+[SQLSTATE: 22022](sql-error-conditions-sqlstates.html#class-22-data-exception)
+
+The value '`<confValue>`' in the config "`<confName>`" is invalid.
+
+For more details see
[INVALID_CONF_VALUE](sql-error-conditions-invalid-conf-value-error-class.html)
+
### [INVALID_CURSOR](sql-error-conditions-invalid-cursor-error-class.html)
[SQLSTATE:
HY109](sql-error-conditions-sqlstates.html#class-HY-cli-specific-condition)
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 59db3e71a135..04b392d0c44f 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
@@ -2759,8 +2759,7 @@ object SQLConf {
"short names are not recommended to use because they can be ambiguous.")
.version("2.2.0")
.stringConf
- .checkValue(isValidTimezone, s"Cannot resolve the given timezone with" +
- " ZoneId.of(_, ZoneId.SHORT_IDS)")
+ .checkValue(isValidTimezone, errorClass = "TIME_ZONE", parameters =
Map.empty)
.createWithDefaultFunction(() => TimeZone.getDefault.getID)
val WINDOW_EXEC_BUFFER_IN_MEMORY_THRESHOLD =
diff --git
a/sql/core/src/test/resources/sql-tests/analyzer-results/timezone.sql.out
b/sql/core/src/test/resources/sql-tests/analyzer-results/timezone.sql.out
index 2ffbb963582a..9059f37f3607 100644
--- a/sql/core/src/test/resources/sql-tests/analyzer-results/timezone.sql.out
+++ b/sql/core/src/test/resources/sql-tests/analyzer-results/timezone.sql.out
@@ -48,8 +48,15 @@ org.apache.spark.sql.catalyst.parser.ParseException
-- !query
SET TIME ZONE 'invalid/zone'
-- !query analysis
-java.lang.IllegalArgumentException
-'invalid/zone' in spark.sql.session.timeZone is invalid. Cannot resolve the
given timezone with ZoneId.of(_, ZoneId.SHORT_IDS)
+org.apache.spark.SparkIllegalArgumentException
+{
+ "errorClass" : "INVALID_CONF_VALUE.TIME_ZONE",
+ "sqlState" : "22022",
+ "messageParameters" : {
+ "confName" : "spark.sql.session.timeZone",
+ "confValue" : "invalid/zone"
+ }
+}
-- !query
diff --git a/sql/core/src/test/resources/sql-tests/results/timezone.sql.out
b/sql/core/src/test/resources/sql-tests/results/timezone.sql.out
index 8c74d8e7f27d..d34599a49c5f 100644
--- a/sql/core/src/test/resources/sql-tests/results/timezone.sql.out
+++ b/sql/core/src/test/resources/sql-tests/results/timezone.sql.out
@@ -62,8 +62,15 @@ SET TIME ZONE 'invalid/zone'
-- !query schema
struct<>
-- !query output
-java.lang.IllegalArgumentException
-'invalid/zone' in spark.sql.session.timeZone is invalid. Cannot resolve the
given timezone with ZoneId.of(_, ZoneId.SHORT_IDS)
+org.apache.spark.SparkIllegalArgumentException
+{
+ "errorClass" : "INVALID_CONF_VALUE.TIME_ZONE",
+ "sqlState" : "22022",
+ "messageParameters" : {
+ "confName" : "spark.sql.session.timeZone",
+ "confValue" : "invalid/zone"
+ }
+}
-- !query
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 cc4669641a21..03f6b9719b9c 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
@@ -22,7 +22,7 @@ import java.util.TimeZone
import org.apache.hadoop.fs.Path
import org.apache.logging.log4j.Level
-import org.apache.spark.{SPARK_DOC_ROOT, SparkNoSuchElementException}
+import org.apache.spark.{SPARK_DOC_ROOT, SparkIllegalArgumentException,
SparkNoSuchElementException}
import org.apache.spark.sql._
import org.apache.spark.sql.catalyst.parser.ParseException
import org.apache.spark.sql.catalyst.util.DateTimeTestUtils.MIT
@@ -445,15 +445,19 @@ class SQLConfSuite extends QueryTest with
SharedSparkSession {
spark.conf.set(SQLConf.SESSION_LOCAL_TIMEZONE.key, "GMT+8:00")
assert(sql(s"set
${SQLConf.SESSION_LOCAL_TIMEZONE.key}").head().getString(1) === "GMT+8:00")
- intercept[IllegalArgumentException] {
+ intercept[SparkIllegalArgumentException] {
spark.conf.set(SQLConf.SESSION_LOCAL_TIMEZONE.key, "pst")
}
- val e = intercept[IllegalArgumentException] {
- spark.conf.set(SQLConf.SESSION_LOCAL_TIMEZONE.key, "Asia/shanghai")
- }
- assert(e.getMessage ===
- s"'Asia/shanghai' in ${SQLConf.SESSION_LOCAL_TIMEZONE.key} is invalid." +
- " Cannot resolve the given timezone with ZoneId.of(_,
ZoneId.SHORT_IDS)")
+
+ val invalidTz = "Asia/shanghai"
+ checkError(
+ exception = intercept[SparkIllegalArgumentException] {
+ spark.conf.set(SQLConf.SESSION_LOCAL_TIMEZONE.key, invalidTz)
+ },
+ errorClass = "INVALID_CONF_VALUE.TIME_ZONE",
+ parameters = Map(
+ "confValue" -> invalidTz,
+ "confName" -> SQLConf.SESSION_LOCAL_TIMEZONE.key))
}
test("set time zone") {
@@ -464,9 +468,15 @@ class SQLConfSuite extends QueryTest with
SharedSparkSession {
sql("set time zone local")
assert(spark.conf.get(SQLConf.SESSION_LOCAL_TIMEZONE) ===
TimeZone.getDefault.getID)
- val e1 = intercept[IllegalArgumentException](sql("set time zone
'invalid'"))
- assert(e1.getMessage === s"'invalid' in
${SQLConf.SESSION_LOCAL_TIMEZONE.key} is invalid." +
- " Cannot resolve the given timezone with ZoneId.of(_, ZoneId.SHORT_IDS)")
+ val tz = "Invalid TZ"
+ checkError(
+ exception = intercept[SparkIllegalArgumentException] {
+ sql(s"SET TIME ZONE '$tz'").collect()
+ },
+ errorClass = "INVALID_CONF_VALUE.TIME_ZONE",
+ parameters = Map(
+ "confValue" -> tz,
+ "confName" -> SQLConf.SESSION_LOCAL_TIMEZONE.key))
(-18 to 18).map(v => (v, s"interval '$v' hours")).foreach { case (i,
interval) =>
sql(s"set time zone $interval")
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org
