You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/09/07 21:04:49 UTC
[GitHub] [spark] karenfeng commented on a change in pull request #33878: [SPARK-36303][SQL] Refactor fourthteenth set of 20 query execution errors to use error classes
karenfeng commented on a change in pull request #33878:
URL: https://github.com/apache/spark/pull/33878#discussion_r703786886
##########
File path: core/src/main/resources/error/error-classes.json
##########
@@ -1,8 +1,61 @@
{
+ "ADD_FILES_WITH_ABSOLUTE_PATH_UNSUPPORTED" : {
+ "message" : [ "%s does not support adding files with an absolute path" ],
+ "sqlState" : "0A000"
+ },
"AMBIGUOUS_FIELD_NAME" : {
"message" : [ "Field name %s is ambiguous and has %s matching fields in the struct." ],
"sqlState" : "42000"
},
+ "BATCH_METADATA_FILE_NOT_FOUND_ERROR" : {
Review comment:
As error classes always represent errors, we can minimize redundancy and clutter by removing the `_ERROR` suffix here (and below).
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogNotFoundException.scala
##########
@@ -26,3 +26,13 @@ class CatalogNotFoundException(message: String, cause: Throwable)
def this(message: String) = this(message, null)
}
+
+class SparkCatalogNotFoundException(errorClass: String, messageParameters: Array[String])
Review comment:
This should also be package private.
##########
File path: core/src/main/resources/error/error-classes.json
##########
@@ -1,8 +1,61 @@
{
+ "ADD_FILES_WITH_ABSOLUTE_PATH_UNSUPPORTED" : {
+ "message" : [ "%s does not support adding files with an absolute path" ],
+ "sqlState" : "0A000"
+ },
"AMBIGUOUS_FIELD_NAME" : {
"message" : [ "Field name %s is ambiguous and has %s matching fields in the struct." ],
"sqlState" : "42000"
},
+ "BATCH_METADATA_FILE_NOT_FOUND_ERROR" : {
+ "message" : [ "Unable to find batch %s" ]
+ },
+ "CANNOT_CLONE_OR_COPY_READ_ONLY_SQL_CONF_ERROR" : {
+ "message" : [ "Cannot clone/copy ReadOnlySQLConf." ],
+ "sqlState" : "42000"
+ },
+ "CANNOT_EXECUTE_STREAMING_RELATION_EXEC_ERROR" : {
+ "message" : [ "StreamingRelationExec cannot be executed" ],
+ "sqlState" : "42000"
+ },
+ "CANNOT_GET_EVENT_TIME_WATERMARK_ERROR" : {
+ "message" : [ "Cannot get event time watermark timestamp without setting watermark before [map|flatMap]GroupsWithState" ],
+ "sqlState" : "42000"
+ },
+ "CANNOT_GET_SQL_CONF_IN_SCHEDULER_EVENT_LOOP_THREAD_ERROR" : {
+ "message" : [ "Cannot get SQLConf inside scheduler event loop thread." ],
+ "sqlState" : "42000"
Review comment:
This seems to be a testing-only (internal) error; we can remove the SQLSTATE.
##########
File path: core/src/main/resources/error/error-classes.json
##########
@@ -1,8 +1,61 @@
{
+ "ADD_FILES_WITH_ABSOLUTE_PATH_UNSUPPORTED" : {
+ "message" : [ "%s does not support adding files with an absolute path" ],
+ "sqlState" : "0A000"
+ },
"AMBIGUOUS_FIELD_NAME" : {
"message" : [ "Field name %s is ambiguous and has %s matching fields in the struct." ],
"sqlState" : "42000"
},
+ "BATCH_METADATA_FILE_NOT_FOUND_ERROR" : {
+ "message" : [ "Unable to find batch %s" ]
+ },
+ "CANNOT_CLONE_OR_COPY_READ_ONLY_SQL_CONF_ERROR" : {
+ "message" : [ "Cannot clone/copy ReadOnlySQLConf." ],
+ "sqlState" : "42000"
+ },
+ "CANNOT_EXECUTE_STREAMING_RELATION_EXEC_ERROR" : {
+ "message" : [ "StreamingRelationExec cannot be executed" ],
+ "sqlState" : "42000"
+ },
+ "CANNOT_GET_EVENT_TIME_WATERMARK_ERROR" : {
+ "message" : [ "Cannot get event time watermark timestamp without setting watermark before [map|flatMap]GroupsWithState" ],
+ "sqlState" : "42000"
+ },
+ "CANNOT_GET_SQL_CONF_IN_SCHEDULER_EVENT_LOOP_THREAD_ERROR" : {
+ "message" : [ "Cannot get SQLConf inside scheduler event loop thread." ],
+ "sqlState" : "42000"
+ },
+ "CANNOT_INSTANTIATE_ABSTRACT_CATALOG_PLUGIN_CLASS_ERROR" : {
+ "message" : [ "Cannot instantiate abstract catalog plugin class for catalog '%s': %s" ],
+ "sqlState" : "42000"
+ },
+ "CANNOT_MUTATE_READ_ONLY_SQL_CONF_ERROR" : {
+ "message" : [ "Cannot mutate ReadOnlySQLConf." ],
+ "sqlState" : "23000"
+ },
+ "CANNOT_SET_TIMEOUT_TIMESTAMP_ERROR" : {
+ "message" : [ "Cannot set timeout timestamp without enabling event time timeout in [map|flatMapGroupsWithState" ],
+ "sqlState" : "42000"
+ },
+ "CATALOG_FAIL_TO_CALL_PUBLIC_NO_ARG_CONSTRUCTOR_ERROR" : {
Review comment:
These names don't make much sense grammatically. Can we name them something like `FAILED_TO_CALL_PUBLIC_NO_ARG_CONSTRUCTOR_FOR_CONSTRUCTOR`?
##########
File path: core/src/main/scala/org/apache/spark/SparkException.scala
##########
@@ -224,3 +224,29 @@ class SparkSQLFeatureNotSupportedException(errorClass: String, messageParameters
override def getErrorClass: String = errorClass
override def getSqlState: String = SparkThrowableHelper.getSqlState(errorClass)
}
+
+/**
+ * Unsupported Operation exception thrown from Spark with an error class.
+ */
+class SparkUnsupportedOperationException(errorClass: String, messageParameters: Array[String])
Review comment:
These should be package-private; eg. https://github.com/apache/spark/blob/f78d8394dcf19891141e353ea3b6a76020faf844/core/src/main/scala/org/apache/spark/SparkException.scala#L221.
##########
File path: core/src/main/resources/error/error-classes.json
##########
@@ -1,8 +1,61 @@
{
+ "ADD_FILES_WITH_ABSOLUTE_PATH_UNSUPPORTED" : {
+ "message" : [ "%s does not support adding files with an absolute path" ],
+ "sqlState" : "0A000"
+ },
"AMBIGUOUS_FIELD_NAME" : {
"message" : [ "Field name %s is ambiguous and has %s matching fields in the struct." ],
"sqlState" : "42000"
},
+ "BATCH_METADATA_FILE_NOT_FOUND_ERROR" : {
+ "message" : [ "Unable to find batch %s" ]
+ },
+ "CANNOT_CLONE_OR_COPY_READ_ONLY_SQL_CONF_ERROR" : {
+ "message" : [ "Cannot clone/copy ReadOnlySQLConf." ],
+ "sqlState" : "42000"
+ },
+ "CANNOT_EXECUTE_STREAMING_RELATION_EXEC_ERROR" : {
+ "message" : [ "StreamingRelationExec cannot be executed" ],
+ "sqlState" : "42000"
+ },
+ "CANNOT_GET_EVENT_TIME_WATERMARK_ERROR" : {
+ "message" : [ "Cannot get event time watermark timestamp without setting watermark before [map|flatMap]GroupsWithState" ],
+ "sqlState" : "42000"
+ },
+ "CANNOT_GET_SQL_CONF_IN_SCHEDULER_EVENT_LOOP_THREAD_ERROR" : {
+ "message" : [ "Cannot get SQLConf inside scheduler event loop thread." ],
+ "sqlState" : "42000"
+ },
+ "CANNOT_INSTANTIATE_ABSTRACT_CATALOG_PLUGIN_CLASS_ERROR" : {
+ "message" : [ "Cannot instantiate abstract catalog plugin class for catalog '%s': %s" ],
+ "sqlState" : "42000"
+ },
+ "CANNOT_MUTATE_READ_ONLY_SQL_CONF_ERROR" : {
+ "message" : [ "Cannot mutate ReadOnlySQLConf." ],
+ "sqlState" : "23000"
Review comment:
I think this is also a 42000 (like the clone/copy error).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org