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