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:24:30 UTC

[GitHub] [spark] karenfeng commented on a change in pull request #33863: [SPARK-36292][SQL] Refactor third set of 20 in QueryExecutionErrors to use error classes

karenfeng commented on a change in pull request #33863:
URL: https://github.com/apache/spark/pull/33863#discussion_r703842813



##########
File path: core/src/main/resources/error/error-classes.json
##########
@@ -21,6 +37,14 @@
   "FAILED_SET_ORIGINAL_PERMISSION_BACK" : {
     "message" : [ "Failed to set original permission %s back to the created path: %s. Exception: %s" ]
   },
+  "FAIL_TO_CONVERT_VALUE_TO_JSON" : {

Review comment:
       For consistency: FAIL -> FAILED?

##########
File path: core/src/main/resources/error/error-classes.json
##########
@@ -77,29 +110,73 @@
     "message" : [ "PARTITION clause cannot contain a non-partition column name: %s" ],
     "sqlState" : "42000"
   },
+  "NOT_EXPECTED_UNRESOLVED_ENCODER" : {
+    "message" : [ "Unresolved encoder expected, but %s was found." ],
+    "sqlState" : "42000"
+  },
+  "NOT_OVERRIDE_EXPECTED_METHODS" : {
+    "message" : [ "%s must override either %s or %s" ],
+    "sqlState" : "42000"
+  },
+  "NULL_AS_MAP_KEY_NOT_ALLOWED" : {
+    "message" : [ "Cannot use null as map key!" ],
+    "sqlState" : "42000"
+  },
   "PIVOT_VALUE_DATA_TYPE_MISMATCH" : {
     "message" : [ "Invalid pivot value '%s': value data type %s does not match pivot column data type %s" ],
     "sqlState" : "42000"
   },
+  "PRIMARY_CONSTRUCTOR_NOT_FOUND" : {
+    "message" : [ "Couldn't find a primary constructor on %s" ],
+    "sqlState" : "42000"
+  },
   "RENAME_SRC_PATH_NOT_FOUND" : {
     "message" : [ "Failed to rename as %s was not found" ],
     "sqlState" : "22023"
   },
+  "RESOLVE_CANNOT_HANDLE_NESTED_SCHEMA" : {
+    "message" : [ "Can not handle nested schema yet...  plan %s" ],

Review comment:
       We should [avoid](https://spark.apache.org/error-message-guidelines.html) making promises about future support. Can you remove `yet`?

##########
File path: core/src/main/resources/error/error-classes.json
##########
@@ -77,29 +110,73 @@
     "message" : [ "PARTITION clause cannot contain a non-partition column name: %s" ],
     "sqlState" : "42000"
   },
+  "NOT_EXPECTED_UNRESOLVED_ENCODER" : {
+    "message" : [ "Unresolved encoder expected, but %s was found." ],
+    "sqlState" : "42000"
+  },
+  "NOT_OVERRIDE_EXPECTED_METHODS" : {
+    "message" : [ "%s must override either %s or %s" ],
+    "sqlState" : "42000"
+  },
+  "NULL_AS_MAP_KEY_NOT_ALLOWED" : {
+    "message" : [ "Cannot use null as map key!" ],
+    "sqlState" : "42000"
+  },
   "PIVOT_VALUE_DATA_TYPE_MISMATCH" : {
     "message" : [ "Invalid pivot value '%s': value data type %s does not match pivot column data type %s" ],
     "sqlState" : "42000"
   },
+  "PRIMARY_CONSTRUCTOR_NOT_FOUND" : {
+    "message" : [ "Couldn't find a primary constructor on %s" ],
+    "sqlState" : "42000"
+  },
   "RENAME_SRC_PATH_NOT_FOUND" : {
     "message" : [ "Failed to rename as %s was not found" ],
     "sqlState" : "22023"
   },
+  "RESOLVE_CANNOT_HANDLE_NESTED_SCHEMA" : {
+    "message" : [ "Can not handle nested schema yet...  plan %s" ],
+    "sqlState" : "42000"
+  },
   "SECOND_FUNCTION_ARGUMENT_NOT_INTEGER" : {
     "message" : [ "The second argument of '%s' function needs to be an integer." ],
     "sqlState" : "22023"
   },
   "UNABLE_TO_ACQUIRE_MEMORY" : {
     "message" : [ "Unable to acquire %s bytes of memory, got %s" ]
   },
+  "UNEXPECTED_OPERATOR_IN_CORRELATED_SUBQUERY" : {
+    "message" : [ "Unexpected operator %s in correlated subquery%s" ],

Review comment:
       Nit: there should be a space between `subquery` and `%s`

##########
File path: core/src/main/resources/error/error-classes.json
##########
@@ -77,29 +110,73 @@
     "message" : [ "PARTITION clause cannot contain a non-partition column name: %s" ],
     "sqlState" : "42000"
   },
+  "NOT_EXPECTED_UNRESOLVED_ENCODER" : {
+    "message" : [ "Unresolved encoder expected, but %s was found." ],
+    "sqlState" : "42000"
+  },
+  "NOT_OVERRIDE_EXPECTED_METHODS" : {
+    "message" : [ "%s must override either %s or %s" ],
+    "sqlState" : "42000"
+  },
+  "NULL_AS_MAP_KEY_NOT_ALLOWED" : {
+    "message" : [ "Cannot use null as map key!" ],
+    "sqlState" : "42000"
+  },
   "PIVOT_VALUE_DATA_TYPE_MISMATCH" : {
     "message" : [ "Invalid pivot value '%s': value data type %s does not match pivot column data type %s" ],
     "sqlState" : "42000"
   },
+  "PRIMARY_CONSTRUCTOR_NOT_FOUND" : {
+    "message" : [ "Couldn't find a primary constructor on %s" ],
+    "sqlState" : "42000"
+  },
   "RENAME_SRC_PATH_NOT_FOUND" : {
     "message" : [ "Failed to rename as %s was not found" ],
     "sqlState" : "22023"
   },
+  "RESOLVE_CANNOT_HANDLE_NESTED_SCHEMA" : {
+    "message" : [ "Can not handle nested schema yet...  plan %s" ],
+    "sqlState" : "42000"
+  },
   "SECOND_FUNCTION_ARGUMENT_NOT_INTEGER" : {
     "message" : [ "The second argument of '%s' function needs to be an integer." ],
     "sqlState" : "22023"
   },
   "UNABLE_TO_ACQUIRE_MEMORY" : {
     "message" : [ "Unable to acquire %s bytes of memory, got %s" ]
   },
+  "UNEXPECTED_OPERATOR_IN_CORRELATED_SUBQUERY" : {
+    "message" : [ "Unexpected operator %s in correlated subquery%s" ],
+    "sqlState" : "42000"
+  },
+  "UNREACHABLE" : {
+    "message" : [ "This line should be unreachable%s" ],

Review comment:
       Nit: there should be a space between `unreachable` and `%s`

##########
File path: core/src/main/resources/error/error-classes.json
##########
@@ -77,29 +110,73 @@
     "message" : [ "PARTITION clause cannot contain a non-partition column name: %s" ],
     "sqlState" : "42000"
   },
+  "NOT_EXPECTED_UNRESOLVED_ENCODER" : {
+    "message" : [ "Unresolved encoder expected, but %s was found." ],
+    "sqlState" : "42000"
+  },
+  "NOT_OVERRIDE_EXPECTED_METHODS" : {
+    "message" : [ "%s must override either %s or %s" ],
+    "sqlState" : "42000"
+  },
+  "NULL_AS_MAP_KEY_NOT_ALLOWED" : {
+    "message" : [ "Cannot use null as map key!" ],
+    "sqlState" : "42000"
+  },
   "PIVOT_VALUE_DATA_TYPE_MISMATCH" : {
     "message" : [ "Invalid pivot value '%s': value data type %s does not match pivot column data type %s" ],
     "sqlState" : "42000"
   },
+  "PRIMARY_CONSTRUCTOR_NOT_FOUND" : {
+    "message" : [ "Couldn't find a primary constructor on %s" ],
+    "sqlState" : "42000"
+  },
   "RENAME_SRC_PATH_NOT_FOUND" : {
     "message" : [ "Failed to rename as %s was not found" ],
     "sqlState" : "22023"
   },
+  "RESOLVE_CANNOT_HANDLE_NESTED_SCHEMA" : {
+    "message" : [ "Can not handle nested schema yet...  plan %s" ],
+    "sqlState" : "42000"
+  },
   "SECOND_FUNCTION_ARGUMENT_NOT_INTEGER" : {
     "message" : [ "The second argument of '%s' function needs to be an integer." ],
     "sqlState" : "22023"
   },
   "UNABLE_TO_ACQUIRE_MEMORY" : {
     "message" : [ "Unable to acquire %s bytes of memory, got %s" ]
   },
+  "UNEXPECTED_OPERATOR_IN_CORRELATED_SUBQUERY" : {
+    "message" : [ "Unexpected operator %s in correlated subquery%s" ],
+    "sqlState" : "42000"
+  },
+  "UNREACHABLE" : {
+    "message" : [ "This line should be unreachable%s" ],
+    "sqlState" : "42000"
+  },
   "UNRECOGNIZED_SQL_TYPE" : {
     "message" : [ "Unrecognized SQL type %s" ],
     "sqlState" : "42000"
   },
+  "UNSUPPORTED_ENCODER" : {
+    "message" : [ "Only expression encoders are supported for now." ],

Review comment:
       We should [avoid](https://spark.apache.org/error-message-guidelines.html) making promises about future support. Can you remove `for now`?

##########
File path: core/src/main/resources/error/error-classes.json
##########
@@ -77,29 +110,73 @@
     "message" : [ "PARTITION clause cannot contain a non-partition column name: %s" ],
     "sqlState" : "42000"
   },
+  "NOT_EXPECTED_UNRESOLVED_ENCODER" : {
+    "message" : [ "Unresolved encoder expected, but %s was found." ],
+    "sqlState" : "42000"
+  },
+  "NOT_OVERRIDE_EXPECTED_METHODS" : {
+    "message" : [ "%s must override either %s or %s" ],
+    "sqlState" : "42000"
+  },
+  "NULL_AS_MAP_KEY_NOT_ALLOWED" : {
+    "message" : [ "Cannot use null as map key!" ],
+    "sqlState" : "42000"
+  },
   "PIVOT_VALUE_DATA_TYPE_MISMATCH" : {
     "message" : [ "Invalid pivot value '%s': value data type %s does not match pivot column data type %s" ],
     "sqlState" : "42000"
   },
+  "PRIMARY_CONSTRUCTOR_NOT_FOUND" : {
+    "message" : [ "Couldn't find a primary constructor on %s" ],
+    "sqlState" : "42000"
+  },
   "RENAME_SRC_PATH_NOT_FOUND" : {
     "message" : [ "Failed to rename as %s was not found" ],
     "sqlState" : "22023"
   },
+  "RESOLVE_CANNOT_HANDLE_NESTED_SCHEMA" : {
+    "message" : [ "Can not handle nested schema yet...  plan %s" ],
+    "sqlState" : "42000"
+  },
   "SECOND_FUNCTION_ARGUMENT_NOT_INTEGER" : {
     "message" : [ "The second argument of '%s' function needs to be an integer." ],
     "sqlState" : "22023"
   },
   "UNABLE_TO_ACQUIRE_MEMORY" : {
     "message" : [ "Unable to acquire %s bytes of memory, got %s" ]
   },
+  "UNEXPECTED_OPERATOR_IN_CORRELATED_SUBQUERY" : {
+    "message" : [ "Unexpected operator %s in correlated subquery%s" ],
+    "sqlState" : "42000"
+  },
+  "UNREACHABLE" : {
+    "message" : [ "This line should be unreachable%s" ],
+    "sqlState" : "42000"
+  },
   "UNRECOGNIZED_SQL_TYPE" : {
     "message" : [ "Unrecognized SQL type %s" ],
     "sqlState" : "42000"
   },
+  "UNSUPPORTED_ENCODER" : {
+    "message" : [ "Only expression encoders are supported for now." ],
+    "sqlState" : "42000"

Review comment:
       0A000 may be a better fit for all "unsupported" errors.

##########
File path: core/src/main/resources/error/error-classes.json
##########
@@ -3,9 +3,25 @@
     "message" : [ "Field name %s is ambiguous and has %s matching fields in the struct." ],
     "sqlState" : "42000"
   },
+  "CANNOT_GENERATE_CODE_FOR_UNSUPPORTED_TYPE" : {
+    "message" : [ "Cannot generate code for unsupported type: %s" ],
+    "sqlState" : "42000"
+  },
+  "CANNOT_INTERPOLATE_CLASS_INTO_CODE_BLOCK" : {
+    "message" : [ "Can not interpolate %s into code block." ],
+    "sqlState" : "42000"
+  },
+  "CLASS_UNSUPPORTED_BY_MAP_OBJECTS" : {
+    "message" : [ "Class `%s` is not supported by `MapObjects` as resulting collection." ],
+    "sqlState" : "42000"

Review comment:
       0A000 may be a better fit here.

##########
File path: core/src/main/scala/org/apache/spark/SparkException.scala
##########
@@ -224,3 +224,69 @@ 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:
       We should make these classes package-private, eg. https://github.com/apache/spark/blob/f78d8394dcf19891141e353ea3b6a76020faf844/core/src/main/scala/org/apache/spark/SparkException.scala#L234

##########
File path: core/src/main/resources/error/error-classes.json
##########
@@ -77,29 +110,73 @@
     "message" : [ "PARTITION clause cannot contain a non-partition column name: %s" ],
     "sqlState" : "42000"
   },
+  "NOT_EXPECTED_UNRESOLVED_ENCODER" : {
+    "message" : [ "Unresolved encoder expected, but %s was found." ],
+    "sqlState" : "42000"
+  },
+  "NOT_OVERRIDE_EXPECTED_METHODS" : {
+    "message" : [ "%s must override either %s or %s" ],
+    "sqlState" : "42000"
+  },
+  "NULL_AS_MAP_KEY_NOT_ALLOWED" : {
+    "message" : [ "Cannot use null as map key!" ],
+    "sqlState" : "42000"
+  },
   "PIVOT_VALUE_DATA_TYPE_MISMATCH" : {
     "message" : [ "Invalid pivot value '%s': value data type %s does not match pivot column data type %s" ],
     "sqlState" : "42000"
   },
+  "PRIMARY_CONSTRUCTOR_NOT_FOUND" : {
+    "message" : [ "Couldn't find a primary constructor on %s" ],
+    "sqlState" : "42000"
+  },
   "RENAME_SRC_PATH_NOT_FOUND" : {
     "message" : [ "Failed to rename as %s was not found" ],
     "sqlState" : "22023"
   },
+  "RESOLVE_CANNOT_HANDLE_NESTED_SCHEMA" : {
+    "message" : [ "Can not handle nested schema yet...  plan %s" ],
+    "sqlState" : "42000"
+  },
   "SECOND_FUNCTION_ARGUMENT_NOT_INTEGER" : {
     "message" : [ "The second argument of '%s' function needs to be an integer." ],
     "sqlState" : "22023"
   },
   "UNABLE_TO_ACQUIRE_MEMORY" : {
     "message" : [ "Unable to acquire %s bytes of memory, got %s" ]
   },
+  "UNEXPECTED_OPERATOR_IN_CORRELATED_SUBQUERY" : {
+    "message" : [ "Unexpected operator %s in correlated subquery%s" ],
+    "sqlState" : "42000"
+  },
+  "UNREACHABLE" : {
+    "message" : [ "This line should be unreachable%s" ],
+    "sqlState" : "42000"
+  },
   "UNRECOGNIZED_SQL_TYPE" : {
     "message" : [ "Unrecognized SQL type %s" ],
     "sqlState" : "42000"
   },
+  "UNSUPPORTED_ENCODER" : {
+    "message" : [ "Only expression encoders are supported for now." ],
+    "sqlState" : "42000"
+  },
   "UNSUPPORTED_LITERAL_TYPE" : {
     "message" : [ "Unsupported literal type %s %s" ],
     "sqlState" : "0A000"
   },
+  "UNSUPPORTED_NATURAL_JOIN_TYPE" : {
+    "message" : [ "Unsupported natural join type %s" ],
+    "sqlState" : "42000"

Review comment:
       0A000 may be a better fit for all "unsupported" errors.




-- 
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