This is an automated email from the ASF dual-hosted git repository.
dongjoon 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 3013d8b4d310 [SPARK-45857][SQL] Enforce the error classes in
sub-classes of `AnalysisException`
3013d8b4d310 is described below
commit 3013d8b4d310997c8a6f6021f860851cd4f3c32a
Author: Max Gekk <max.g...@gmail.com>
AuthorDate: Sun Nov 12 14:56:13 2023 -0800
[SPARK-45857][SQL] Enforce the error classes in sub-classes of
`AnalysisException`
### What changes were proposed in this pull request?
In the PR, I propose to enforce creation of `AnalysisException` sub-class
exceptions with an error class always. In particular, it converts the
constructor with a message to private one, so, callers have to create a
sub-class of `AnalysisException` with an error class.
### Why are the changes needed?
This simplifies migration on error classes.
### Does this PR introduce _any_ user-facing change?
No, since user code doesn't throw `AnalysisException` and its sub-classes
in regular cases.
### How was this patch tested?
By existing test suites, for instance:
```
$ build/sbt "sql/testOnly *QueryParsingErrorsSuite"
```
### Was this patch authored or co-authored using generative AI tooling?
No.
Closes #43731 from MaxGekk/ban-message-subclasses-AnalysisException.
Authored-by: Max Gekk <max.g...@gmail.com>
Signed-off-by: Dongjoon Hyun <dh...@apple.com>
---
.../src/main/resources/error/error-classes.json | 5 ++
.../connect/client/GrpcExceptionConverter.scala | 32 +++------
.../sql/catalyst/analysis/NonEmptyException.scala | 2 +-
.../catalyst/analysis/alreadyExistException.scala | 45 ++----------
.../catalyst/analysis/noSuchItemsExceptions.scala | 79 +++++++---------------
.../catalyst/analysis/AlreadyExistException.scala | 14 ----
.../catalyst/analysis/NoSuchItemException.scala | 14 ----
.../catalog/InvalidUDFClassException.scala | 2 +-
.../spark/sql/errors/QueryCompilationErrors.scala | 6 ++
.../org/apache/spark/sql/jdbc/H2Dialect.scala | 6 +-
.../spark/sql/hive/HiveSessionStateBuilder.scala | 4 +-
11 files changed, 60 insertions(+), 149 deletions(-)
diff --git a/common/utils/src/main/resources/error/error-classes.json
b/common/utils/src/main/resources/error/error-classes.json
index 3b7a3a6006ef..e3b9f3161b24 100644
--- a/common/utils/src/main/resources/error/error-classes.json
+++ b/common/utils/src/main/resources/error/error-classes.json
@@ -6334,6 +6334,11 @@
"Operation not allowed: <cmd> only works on table with location
provided: <tableIdentWithDB>"
]
},
+ "_LEGACY_ERROR_TEMP_2450" : {
+ "message" : [
+ "No handler for UDF/UDAF/UDTF '<clazz>'"
+ ]
+ },
"_LEGACY_ERROR_TEMP_3000" : {
"message" : [
"Unexpected Py4J server <class>."
diff --git
a/connector/connect/common/src/main/scala/org/apache/spark/sql/connect/client/GrpcExceptionConverter.scala
b/connector/connect/common/src/main/scala/org/apache/spark/sql/connect/client/GrpcExceptionConverter.scala
index 52bd276b0c4b..73e2db2f4ac7 100644
---
a/connector/connect/common/src/main/scala/org/apache/spark/sql/connect/client/GrpcExceptionConverter.scala
+++
b/connector/connect/common/src/main/scala/org/apache/spark/sql/connect/client/GrpcExceptionConverter.scala
@@ -204,34 +204,24 @@ private[client] object GrpcExceptionConverter {
messageParameters = params.messageParameters,
context = params.queryContext)),
errorConstructor(params =>
- new NamespaceAlreadyExistsException(
- params.message,
- params.errorClass,
- params.messageParameters)),
+ new NamespaceAlreadyExistsException(params.errorClass.orNull,
params.messageParameters)),
errorConstructor(params =>
new TableAlreadyExistsException(
- params.message,
- params.cause,
- params.errorClass,
- params.messageParameters)),
+ params.errorClass.orNull,
+ params.messageParameters,
+ params.cause)),
errorConstructor(params =>
new TempTableAlreadyExistsException(
- params.message,
- params.cause,
- params.errorClass,
- params.messageParameters)),
+ params.errorClass.orNull,
+ params.messageParameters,
+ params.cause)),
errorConstructor(params =>
new NoSuchDatabaseException(
- params.message,
- params.cause,
- params.errorClass,
- params.messageParameters)),
+ params.errorClass.orNull,
+ params.messageParameters,
+ params.cause)),
errorConstructor(params =>
- new NoSuchTableException(
- params.message,
- params.cause,
- params.errorClass,
- params.messageParameters)),
+ new NoSuchTableException(params.errorClass.orNull,
params.messageParameters, params.cause)),
errorConstructor[NumberFormatException](params =>
new SparkNumberFormatException(
params.message,
diff --git
a/sql/api/src/main/scala/org/apache/spark/sql/catalyst/analysis/NonEmptyException.scala
b/sql/api/src/main/scala/org/apache/spark/sql/catalyst/analysis/NonEmptyException.scala
index ecd57672b616..2aea9bac12fe 100644
---
a/sql/api/src/main/scala/org/apache/spark/sql/catalyst/analysis/NonEmptyException.scala
+++
b/sql/api/src/main/scala/org/apache/spark/sql/catalyst/analysis/NonEmptyException.scala
@@ -25,7 +25,7 @@ import org.apache.spark.sql.catalyst.util.QuotingUtils.quoted
* Thrown by a catalog when an item already exists. The analyzer will rethrow
the exception
* as an [[org.apache.spark.sql.AnalysisException]] with the correct position
information.
*/
-case class NonEmptyNamespaceException(
+case class NonEmptyNamespaceException private(
override val message: String,
override val cause: Option[Throwable] = None)
extends AnalysisException(message, cause = cause) {
diff --git
a/sql/api/src/main/scala/org/apache/spark/sql/catalyst/analysis/alreadyExistException.scala
b/sql/api/src/main/scala/org/apache/spark/sql/catalyst/analysis/alreadyExistException.scala
index 85eba2b24614..8932a0296428 100644
---
a/sql/api/src/main/scala/org/apache/spark/sql/catalyst/analysis/alreadyExistException.scala
+++
b/sql/api/src/main/scala/org/apache/spark/sql/catalyst/analysis/alreadyExistException.scala
@@ -32,7 +32,7 @@ class DatabaseAlreadyExistsException(db: String)
extends NamespaceAlreadyExistsException(Array(db))
// any changes to this class should be backward compatible as it may be used
by external connectors
-class NamespaceAlreadyExistsException private[sql](
+class NamespaceAlreadyExistsException private(
message: String,
errorClass: Option[String],
messageParameters: Map[String, String])
@@ -52,17 +52,10 @@ class NamespaceAlreadyExistsException private[sql](
this(errorClass = "SCHEMA_ALREADY_EXISTS",
Map("schemaName" -> quoteNameParts(namespace.toImmutableArraySeq)))
}
-
- def this(message: String) = {
- this(
- message,
- errorClass = Some("SCHEMA_ALREADY_EXISTS"),
- messageParameters = Map.empty[String, String])
- }
}
// any changes to this class should be backward compatible as it may be used
by external connectors
-class TableAlreadyExistsException private[sql](
+class TableAlreadyExistsException private(
message: String,
cause: Option[Throwable],
errorClass: Option[String],
@@ -106,21 +99,13 @@ class TableAlreadyExistsException private[sql](
messageParameters = Map("relationName" -> quoted(tableIdent)),
cause = None)
}
-
- def this(message: String, cause: Option[Throwable] = None) = {
- this(
- message,
- cause,
- errorClass = Some("TABLE_OR_VIEW_ALREADY_EXISTS"),
- messageParameters = Map.empty[String, String])
- }
}
-class TempTableAlreadyExistsException private[sql](
- message: String,
- cause: Option[Throwable],
- errorClass: Option[String],
- messageParameters: Map[String, String])
+class TempTableAlreadyExistsException private(
+ message: String,
+ cause: Option[Throwable],
+ errorClass: Option[String],
+ messageParameters: Map[String, String])
extends AnalysisException(
message,
cause = cause,
@@ -144,14 +129,6 @@ class TempTableAlreadyExistsException private[sql](
messageParameters = Map("relationName"
-> quoteNameParts(AttributeNameParser.parseAttributeName(table))))
}
-
- def this(message: String, cause: Option[Throwable]) = {
- this(
- message,
- cause,
- errorClass = Some("TEMP_TABLE_OR_VIEW_ALREADY_EXISTS"),
- messageParameters = Map.empty[String, String])
- }
}
// any changes to this class should be backward compatible as it may be used
by external connectors
@@ -203,12 +180,4 @@ class IndexAlreadyExistsException private(
def this(indexName: String, tableName: String, cause: Option[Throwable]) = {
this("INDEX_ALREADY_EXISTS", Map("indexName" -> indexName, "tableName" ->
tableName), cause)
}
-
- def this(message: String, cause: Option[Throwable] = None) = {
- this(
- message,
- cause,
- errorClass = Some("INDEX_ALREADY_EXISTS"),
- messageParameters = Map.empty[String, String])
- }
}
diff --git
a/sql/api/src/main/scala/org/apache/spark/sql/catalyst/analysis/noSuchItemsExceptions.scala
b/sql/api/src/main/scala/org/apache/spark/sql/catalyst/analysis/noSuchItemsExceptions.scala
index b7c8473c08c0..ac22d26ccfd1 100644
---
a/sql/api/src/main/scala/org/apache/spark/sql/catalyst/analysis/noSuchItemsExceptions.scala
+++
b/sql/api/src/main/scala/org/apache/spark/sql/catalyst/analysis/noSuchItemsExceptions.scala
@@ -27,21 +27,21 @@ import org.apache.spark.util.ArrayImplicits._
* Thrown by a catalog when an item cannot be found. The analyzer will rethrow
the exception
* as an [[org.apache.spark.sql.AnalysisException]] with the correct position
information.
*/
-class NoSuchDatabaseException private[sql](
- message: String,
- cause: Option[Throwable],
- errorClass: Option[String],
- messageParameters: Map[String, String])
+class NoSuchDatabaseException private(
+ message: String,
+ cause: Option[Throwable],
+ errorClass: Option[String],
+ messageParameters: Map[String, String])
extends AnalysisException(
message,
cause = cause,
errorClass = errorClass,
messageParameters = messageParameters) {
- def this(errorClass: String, messageParameters: Map[String, String]) = {
+ def this(errorClass: String, messageParameters: Map[String, String], cause:
Option[Throwable]) = {
this(
SparkThrowableHelper.getMessage(errorClass, messageParameters),
- cause = None,
+ cause = cause,
Some(errorClass),
messageParameters)
}
@@ -49,15 +49,8 @@ class NoSuchDatabaseException private[sql](
def this(db: String) = {
this(
errorClass = "SCHEMA_NOT_FOUND",
- messageParameters = Map("schemaName" -> quoteIdentifier(db)))
- }
-
- def this(message: String, cause: Option[Throwable]) = {
- this(
- message = message,
- cause = cause,
- errorClass = Some("SCHEMA_NOT_FOUND"),
- messageParameters = Map.empty[String, String])
+ messageParameters = Map("schemaName" -> quoteIdentifier(db)),
+ cause = None)
}
}
@@ -90,18 +83,10 @@ class NoSuchNamespaceException private(
this(errorClass = "SCHEMA_NOT_FOUND",
Map("schemaName" -> quoteNameParts(namespace.toImmutableArraySeq)))
}
-
- def this(message: String, cause: Option[Throwable] = None) = {
- this(
- message,
- cause,
- errorClass = Some("SCHEMA_NOT_FOUND"),
- messageParameters = Map.empty[String, String])
- }
}
// any changes to this class should be backward compatible as it may be used
by external connectors
-class NoSuchTableException private[sql](
+class NoSuchTableException private(
message: String,
cause: Option[Throwable],
errorClass: Option[String],
@@ -112,36 +97,34 @@ class NoSuchTableException private[sql](
errorClass = errorClass,
messageParameters = messageParameters) {
- def this(errorClass: String, messageParameters: Map[String, String]) = {
+ def this(errorClass: String, messageParameters: Map[String, String], cause:
Option[Throwable]) = {
this(
SparkThrowableHelper.getMessage(errorClass, messageParameters),
- cause = None,
+ cause = cause,
Some(errorClass),
messageParameters)
}
def this(db: String, table: String) = {
- this(errorClass = "TABLE_OR_VIEW_NOT_FOUND",
+ this(
+ errorClass = "TABLE_OR_VIEW_NOT_FOUND",
messageParameters = Map("relationName" ->
- (quoteIdentifier(db) + "." + quoteIdentifier(table))))
+ (quoteIdentifier(db) + "." + quoteIdentifier(table))),
+ cause = None)
}
def this(name : Seq[String]) = {
- this(errorClass = "TABLE_OR_VIEW_NOT_FOUND",
- messageParameters = Map("relationName" -> quoteNameParts(name)))
+ this(
+ errorClass = "TABLE_OR_VIEW_NOT_FOUND",
+ messageParameters = Map("relationName" -> quoteNameParts(name)),
+ cause = None)
}
def this(tableIdent: Identifier) = {
- this(errorClass = "TABLE_OR_VIEW_NOT_FOUND",
- messageParameters = Map("relationName" -> quoted(tableIdent)))
- }
-
- def this(message: String, cause: Option[Throwable] = None) = {
this(
- message,
- cause,
- errorClass = Some("TABLE_OR_VIEW_NOT_FOUND"),
- messageParameters = Map.empty[String, String])
+ errorClass = "TABLE_OR_VIEW_NOT_FOUND",
+ messageParameters = Map("relationName" -> quoted(tableIdent)),
+ cause = None)
}
}
@@ -186,14 +169,6 @@ class NoSuchFunctionException private(
def this(identifier: Identifier) = {
this(errorClass = "ROUTINE_NOT_FOUND", Map("routineName" ->
quoted(identifier)))
}
-
- def this(message: String, cause: Option[Throwable] = None) = {
- this(
- message,
- cause,
- errorClass = Some("ROUTINE_NOT_FOUND"),
- messageParameters = Map.empty[String, String])
- }
}
class NoSuchTempFunctionException(func: String)
@@ -225,12 +200,4 @@ class NoSuchIndexException private(
def this(indexName: String, tableName: String, cause: Option[Throwable]) = {
this("INDEX_NOT_FOUND", Map("indexName" -> indexName, "tableName" ->
tableName), cause)
}
-
- def this(message: String, cause: Option[Throwable] = None) = {
- this(
- message,
- cause,
- errorClass = Some("INDEX_NOT_FOUND"),
- messageParameters = Map.empty[String, String])
- }
}
diff --git
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/AlreadyExistException.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/AlreadyExistException.scala
index bbac5ab7db3e..4662f1c6ede6 100644
---
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/AlreadyExistException.scala
+++
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/AlreadyExistException.scala
@@ -55,13 +55,6 @@ class PartitionAlreadyExistsException private(
.map( kv => quoteIdentifier(s"${kv._2}") + s" = ${kv._1}").mkString(",
") + ")"),
"tableName" ->
quoteNameParts(UnresolvedAttribute.parseAttributeName(tableName))))
}
-
- def this(message: String) = {
- this(
- message,
- errorClass = Some("PARTITIONS_ALREADY_EXIST"),
- messageParameters = Map.empty[String, String])
- }
}
// any changes to this class should be backward compatible as it may be used
by external connectors
@@ -105,11 +98,4 @@ class PartitionsAlreadyExistException private(
def this(tableName: String, partitionIdent: InternalRow, partitionSchema:
StructType) =
this(tableName, Seq(partitionIdent), partitionSchema)
-
- def this(message: String) = {
- this(
- message,
- errorClass = Some("PARTITIONS_ALREADY_EXIST"),
- messageParameters = Map.empty[String, String])
- }
}
diff --git
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/NoSuchItemException.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/NoSuchItemException.scala
index 217c293900ec..5db713066ff9 100644
---
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/NoSuchItemException.scala
+++
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/NoSuchItemException.scala
@@ -56,13 +56,6 @@ class NoSuchPartitionException private(
.map( kv => quoteIdentifier(s"${kv._2}") + s" = ${kv._1}").mkString(",
") + ")"),
"tableName" ->
quoteNameParts(UnresolvedAttribute.parseAttributeName(tableName))))
}
-
- def this(message: String) = {
- this(
- message,
- errorClass = Some("PARTITIONS_NOT_FOUND"),
- messageParameters = Map.empty[String, String])
- }
}
// any changes to this class should be backward compatible as it may be used
by external connectors
@@ -98,11 +91,4 @@ class NoSuchPartitionsException private(
.mkString(", ")).mkString("), PARTITION (") + ")"),
"tableName" ->
quoteNameParts(UnresolvedAttribute.parseAttributeName(tableName))))
}
-
- def this(message: String) = {
- this(
- message,
- errorClass = Some("PARTITIONS_NOT_FOUND"),
- messageParameters = Map.empty[String, String])
- }
}
diff --git
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/InvalidUDFClassException.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/InvalidUDFClassException.scala
index 658ddb21c6d9..bfd8ba7d5a59 100644
---
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/InvalidUDFClassException.scala
+++
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/InvalidUDFClassException.scala
@@ -24,7 +24,7 @@ import org.apache.spark.sql.AnalysisException
* Thrown when a query failed for invalid function class, usually because a SQL
* function's class does not follow the rules of the UDF/UDAF/UDTF class
definition.
*/
-class InvalidUDFClassException private[sql](
+class InvalidUDFClassException private(
message: String,
errorClass: Option[String] = None,
messageParameters: Map[String, String] = Map.empty)
diff --git
a/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala
index 7399f6c621cc..c3249a4c02d8 100644
---
a/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala
+++
b/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala
@@ -3825,4 +3825,10 @@ private[sql] object QueryCompilationErrors extends
QueryErrorsBase with Compilat
messageParameters = Map("rowTag" -> toSQLId(optionName))
)
}
+
+ def invalidUDFClassError(invalidClass: String): Throwable = {
+ new InvalidUDFClassException(
+ errorClass = "_LEGACY_ERROR_TEMP_2450",
+ messageParameters = Map("invalidClass" -> invalidClass))
+ }
}
diff --git a/sql/core/src/main/scala/org/apache/spark/sql/jdbc/H2Dialect.scala
b/sql/core/src/main/scala/org/apache/spark/sql/jdbc/H2Dialect.scala
index 43888d0ffedd..9bed6a6f873e 100644
--- a/sql/core/src/main/scala/org/apache/spark/sql/jdbc/H2Dialect.scala
+++ b/sql/core/src/main/scala/org/apache/spark/sql/jdbc/H2Dialect.scala
@@ -197,8 +197,10 @@ private[sql] object H2Dialect extends JdbcDialect {
// TABLE_OR_VIEW_NOT_FOUND_1
case 42102 =>
val quotedName =
quoteNameParts(UnresolvedAttribute.parseAttributeName(message))
- throw new NoSuchTableException(errorClass =
"TABLE_OR_VIEW_NOT_FOUND",
- messageParameters = Map("relationName" -> quotedName))
+ throw new NoSuchTableException(
+ errorClass = "TABLE_OR_VIEW_NOT_FOUND",
+ messageParameters = Map("relationName" -> quotedName),
+ cause = Some(e))
// SCHEMA_NOT_FOUND_1
case 90079 =>
val regex = """"((?:[^"\\]|\\[\\"ntbrf])+)"""".r
diff --git
a/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionStateBuilder.scala
b/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionStateBuilder.scala
index 0b5e98d0a3e4..e991665e2887 100644
---
a/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionStateBuilder.scala
+++
b/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionStateBuilder.scala
@@ -30,6 +30,7 @@ import
org.apache.spark.sql.catalyst.catalog.{ExternalCatalogWithListener, Inval
import org.apache.spark.sql.catalyst.expressions.Expression
import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
import org.apache.spark.sql.catalyst.rules.Rule
+import org.apache.spark.sql.errors.QueryCompilationErrors
import org.apache.spark.sql.execution.SparkPlanner
import org.apache.spark.sql.execution.aggregate.ResolveEncodersInScalaAgg
import org.apache.spark.sql.execution.analysis.DetectAmbiguousSelfJoin
@@ -207,8 +208,7 @@ object HiveUDFExpressionBuilder extends
SparkUDFExpressionBuilder {
throw analysisException
}
udfExpr.getOrElse {
- throw new InvalidUDFClassException(
- s"No handler for UDF/UDAF/UDTF '${clazz.getCanonicalName}'")
+ throw QueryCompilationErrors.invalidUDFClassError(clazz.getCanonicalName)
}
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org
