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:58:03 UTC
[GitHub] [spark] karenfeng commented on a change in pull request #33839: [WIP][SPARK-36291][SQL] Refactor second set of 20 in QueryExecutionErrors to use error classes
karenfeng commented on a change in pull request #33839:
URL: https://github.com/apache/spark/pull/33839#discussion_r703856422
##########
File path: core/src/main/resources/error/error-classes.json
##########
@@ -6,6 +6,10 @@
"CONCURRENT_QUERY_ERROR" : {
"message" : [ "Another instance of this query was just started by a concurrent session." ]
},
+ "COPY_NULL_FIELD" : {
+ "message" : [ "Do not attempt to copy a null field" ],
+ "sqlState" : "39004"
Review comment:
This only applies to external routine invocations.
##########
File path: core/src/main/resources/error/error-classes.json
##########
@@ -32,20 +44,36 @@
"GROUPING_SIZE_LIMIT_EXCEEDED" : {
"message" : [ "Grouping sets size cannot be greater than %s" ]
},
+ "GROUP_INDEX_EXCEED_GROUP_COUNT" : {
+ "message" : [ "Regex group count is %s, but the specified group index is %s" ],
+ "sqlState" : "07009"
Review comment:
I'm not sure if 07009 is right for group index, it seems to apply to descriptor index. Maybe 42000?
##########
File path: core/src/main/resources/error/error-classes.json
##########
@@ -32,20 +44,36 @@
"GROUPING_SIZE_LIMIT_EXCEEDED" : {
"message" : [ "Grouping sets size cannot be greater than %s" ]
},
+ "GROUP_INDEX_EXCEED_GROUP_COUNT" : {
+ "message" : [ "Regex group count is %s, but the specified group index is %s" ],
+ "sqlState" : "07009"
+ },
+ "GROUP_INDEX_LESS_THAN_ZERO" : {
+ "message" : [ "The specified group index cannot be less than zero" ],
+ "sqlState" : "07009"
+ },
"IF_PARTITION_NOT_EXISTS_UNSUPPORTED" : {
"message" : [ "Cannot write, IF NOT EXISTS is not supported for table: %s" ]
},
"INCOMPARABLE_PIVOT_COLUMN" : {
"message" : [ "Invalid pivot column '%s'. Pivot columns must be comparable." ],
"sqlState" : "42000"
},
+ "INCOMPARABLE_TYPE" : {
+ "message" : [ "cannot generate %s code for un-comparable type: %s" ],
+ "sqlState" : "07006"
Review comment:
0A000 may be a better fit
##########
File path: core/src/main/resources/error/error-classes.json
##########
@@ -57,6 +85,10 @@
"INVALID_JSON_SCHEMA_MAPTYPE" : {
"message" : [ "Input schema %s can only contain StringType as a key type for a MapType." ]
},
+ "INVALID_URL" : {
+ "message" : [ "Find an invalid url string %s" ],
+ "sqlState" : "22000"
Review comment:
42000 may be a better fit
##########
File path: core/src/main/resources/error/error-classes.json
##########
@@ -77,6 +109,18 @@
"message" : [ "PARTITION clause cannot contain a non-partition column name: %s" ],
"sqlState" : "42000"
},
+ "NOT_MATCH_FUNCTION_NAME" : {
+ "message" : [ "%s is not matched at addNewFunction" ],
+ "sqlState" : "07000"
+ },
+ "NOT_SUPPORT_ORDERED_OPERATIONS" : {
+ "message" : [ "Type %s does not support ordered operations" ],
+ "sqlState" : "07006"
Review comment:
0A000 may be a better fit
##########
File path: core/src/main/resources/error/error-classes.json
##########
@@ -77,6 +109,18 @@
"message" : [ "PARTITION clause cannot contain a non-partition column name: %s" ],
"sqlState" : "42000"
},
+ "NOT_MATCH_FUNCTION_NAME" : {
+ "message" : [ "%s is not matched at addNewFunction" ],
+ "sqlState" : "07000"
+ },
+ "NOT_SUPPORT_ORDERED_OPERATIONS" : {
Review comment:
Can we name this `UNSUPPORTED_ORDERED_OPERATIONS`?
##########
File path: core/src/main/resources/error/error-classes.json
##########
@@ -14,13 +18,21 @@
"message" : [ "Found duplicate keys '%s'" ],
"sqlState" : "23000"
},
+ "EXCEED_ARRAY_SIZE_WHEN_ZIP_MAP" : {
+ "message" : [ "Unsuccessful try to zip maps with %s unique keys due to exceeding the array size limit %s." ],
+ "sqlState" : "22026"
Review comment:
We can leave this blank for now; I don't see an obvious SQLSTATE for exceeding the array size limit (22026 is only for string data).
##########
File path: core/src/main/resources/error/error-classes.json
##########
@@ -104,6 +172,10 @@
"message" : [ "The target JDBC server does not support transaction and can only support ALTER TABLE with a single action." ],
"sqlState" : "0A000"
},
+ "WINDOW_NOT_SUPPORT_MERGING" : {
Review comment:
This doesn't make sense grammatically; can we rephrase it along the lines of "MERGE_UNSUPPORTED_BY_WINDOW_FUNCTIONS"?
##########
File path: core/src/main/resources/error/error-classes.json
##########
@@ -32,20 +44,36 @@
"GROUPING_SIZE_LIMIT_EXCEEDED" : {
"message" : [ "Grouping sets size cannot be greater than %s" ]
},
+ "GROUP_INDEX_EXCEED_GROUP_COUNT" : {
+ "message" : [ "Regex group count is %s, but the specified group index is %s" ],
+ "sqlState" : "07009"
+ },
+ "GROUP_INDEX_LESS_THAN_ZERO" : {
+ "message" : [ "The specified group index cannot be less than zero" ],
+ "sqlState" : "07009"
+ },
"IF_PARTITION_NOT_EXISTS_UNSUPPORTED" : {
"message" : [ "Cannot write, IF NOT EXISTS is not supported for table: %s" ]
},
"INCOMPARABLE_PIVOT_COLUMN" : {
"message" : [ "Invalid pivot column '%s'. Pivot columns must be comparable." ],
"sqlState" : "42000"
},
+ "INCOMPARABLE_TYPE" : {
Review comment:
I think this should be more specific; eg. `CANNOT_GENERATE_CODE_FOR_UNCOMPARABLE_TYPE`.
##########
File path: core/src/main/scala/org/apache/spark/SparkException.scala
##########
@@ -224,3 +224,70 @@ 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
##########
@@ -89,13 +122,31 @@
"message" : [ "The second argument of '%s' function needs to be an integer." ],
"sqlState" : "22023"
},
+ "SHOULD_NOT_CALL_ALIAS_DO_GEN_CODE" : {
+ "message" : [ "Alias.doGenCode should not be called." ]
+ },
+ "SUM_OF_DECIMAL_OVERFLOW" : {
+ "message" : [ "Overflow in sum of decimals." ]
+ },
"UNABLE_TO_ACQUIRE_MEMORY" : {
"message" : [ "Unable to acquire %s bytes of memory, got %s" ]
},
+ "UNEXPECTED_DATATYPE" : {
Review comment:
I also think 42000 may be a better fit.
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryExecutionErrors.scala
##########
@@ -153,90 +153,88 @@ object QueryExecutionErrors {
}
def inputTypeUnsupportedError(dataType: DataType): Throwable = {
- new IllegalArgumentException(s"Unsupported input type ${dataType.catalogString}")
+ new SparkIllegalArgumentException("UNSUPPORTED_INPUT_TYPE", Array(dataType.catalogString))
}
def invalidFractionOfSecondError(): DateTimeException = {
- new SparkDateTimeException(errorClass = "INVALID_FRACTION_OF_SECOND", Array.empty)
+ new SparkDateTimeException("INVALID_FRACTION_OF_SECOND", Array.empty)
}
def overflowInSumOfDecimalError(): ArithmeticException = {
- new ArithmeticException("Overflow in sum of decimals.")
+ new SparkArithmeticException("SUM_OF_DECIMAL_OVERFLOW", Array.empty)
}
def overflowInIntegralDivideError(): ArithmeticException = {
- new ArithmeticException("Overflow in integral divide.")
+ new SparkArithmeticException("INTEGRAL_DIVIDE_OVERFLOW", Array.empty)
}
def mapSizeExceedArraySizeWhenZipMapError(size: Int): RuntimeException = {
- new RuntimeException(s"Unsuccessful try to zip maps with $size " +
- "unique keys due to exceeding the array size limit " +
- s"${ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH}.")
+ new SparkRuntimeException("EXCEED_ARRAY_SIZE_WHEN_ZIP_MAP",
+ Array(size.toString, ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH.toString))
}
def copyNullFieldNotAllowedError(): Throwable = {
- new IllegalStateException("Do not attempt to copy a null field")
+ new SparkIllegalStateException("COPY_NULL_FIELD", Array.empty)
}
def literalTypeUnsupportedError(v: Any): RuntimeException = {
- new SparkRuntimeException("UNSUPPORTED_LITERAL_TYPE",
- Array(v.getClass.toString, v.toString))
+ new SparkRuntimeException("UNSUPPORTED_LITERAL_TYPE", Array(v.getClass.toString, v.toString))
}
def noDefaultForDataTypeError(dataType: DataType): RuntimeException = {
- new RuntimeException(s"no default for type $dataType")
+ new SparkRuntimeException("NO_DEFAULT_FOR_TYPE", Array(dataType.toString))
}
def doGenCodeOfAliasShouldNotBeCalledError(): Throwable = {
- new IllegalStateException("Alias.doGenCode should not be called.")
+ new SparkIllegalStateException("SHOULD_NOT_CALL_ALIAS_DO_GEN_CODE", Array.empty)
}
def orderedOperationUnsupportedByDataTypeError(dataType: DataType): Throwable = {
- new IllegalArgumentException(s"Type $dataType does not support ordered operations")
+ new SparkIllegalArgumentException("NOT_SUPPORT_ORDERED_OPERATIONS", Array(dataType.toString))
}
def regexGroupIndexLessThanZeroError(): Throwable = {
- new IllegalArgumentException("The specified group index cannot be less than zero")
+ new SparkIllegalArgumentException("GROUP_INDEX_LESS_THAN_ZERO", Array.empty)
}
def regexGroupIndexExceedGroupCountError(
groupCount: Int, groupIndex: Int): Throwable = {
- new IllegalArgumentException(
- s"Regex group count is $groupCount, but the specified group index is $groupIndex")
+ new SparkIllegalArgumentException("GROUP_INDEX_EXCEED_GROUP_COUNT",
+ Array(groupCount.toString, groupIndex.toString))
}
def invalidUrlError(url: UTF8String, e: URISyntaxException): Throwable = {
- new IllegalArgumentException(s"Find an invaild url string ${url.toString}", e)
+ new SparkIllegalArgumentException("INVALID_URL", Array(url.toString), cause = e)
}
def dataTypeOperationUnsupportedError(): Throwable = {
- new UnsupportedOperationException("dataType")
+ new SparkUnsupportedOperationException("UNSUPPORTED_DATATYPE", Array.empty)
}
def mergeUnsupportedByWindowFunctionError(): Throwable = {
- new UnsupportedOperationException("Window Functions do not support merging.")
+ new SparkUnsupportedOperationException("WINDOW_NOT_SUPPORT_MERGING", Array.empty)
}
def dataTypeUnexpectedError(dataType: DataType): Throwable = {
- new UnsupportedOperationException(s"Unexpected data type ${dataType.catalogString}")
+ new SparkUnsupportedOperationException("UNEXPECTED_DATATYPE", Array(dataType.catalogString))
}
def typeUnsupportedError(dataType: DataType): Throwable = {
- new IllegalArgumentException(s"Unexpected type $dataType")
+ new SparkIllegalArgumentException("UNEXPECTED_TYPE", Array(dataType.toString))
}
def negativeValueUnexpectedError(frequencyExpression : Expression): Throwable = {
- new SparkException(s"Negative values found in ${frequencyExpression.sql}")
+ new SparkException(errorClass = "FOUND_NEGATIVE_VALUES",
+ messageParameters = Array(frequencyExpression.sql), cause = null)
}
def addNewFunctionMismatchedWithFunctionError(funcName: String): Throwable = {
- new IllegalArgumentException(s"$funcName is not matched at addNewFunction")
+ new SparkIllegalArgumentException("NOT_MATCH_FUNCTION_NAME", Array(funcName))
}
- def cannotGenerateCodeForUncomparableTypeError(
+ def cannotGenerateCodeForIncomparableTypeError(
Review comment:
"Uncomparable" is more accurate than "incomparable": "incomparable" (although more commonly used) generally refers to something that is much better than the others, while "uncomparable" refers to something that cannot be compared to others.
##########
File path: core/src/main/resources/error/error-classes.json
##########
@@ -89,13 +133,37 @@
"message" : [ "The second argument of '%s' function needs to be an integer." ],
"sqlState" : "22023"
},
+ "SHOULD_NOT_CALL_ALIAS_DO_GEN_CODE" : {
+ "message" : [ "Alias.doGenCode should not be called." ],
+ "sqlState" : "42000"
Review comment:
We can leave this blank; this is an internal error.
##########
File path: core/src/main/resources/error/error-classes.json
##########
@@ -89,13 +133,37 @@
"message" : [ "The second argument of '%s' function needs to be an integer." ],
"sqlState" : "22023"
},
+ "SHOULD_NOT_CALL_ALIAS_DO_GEN_CODE" : {
+ "message" : [ "Alias.doGenCode should not be called." ],
+ "sqlState" : "42000"
+ },
+ "SUM_OF_DECIMAL_OVERFLOW" : {
+ "message" : [ "Overflow in sum of decimals." ],
+ "sqlState" : "22015"
Review comment:
22015 is for interval fields; maybe 22003 is a better fit?
##########
File path: core/src/main/resources/error/error-classes.json
##########
@@ -89,13 +122,31 @@
"message" : [ "The second argument of '%s' function needs to be an integer." ],
"sqlState" : "22023"
},
+ "SHOULD_NOT_CALL_ALIAS_DO_GEN_CODE" : {
+ "message" : [ "Alias.doGenCode should not be called." ]
+ },
+ "SUM_OF_DECIMAL_OVERFLOW" : {
+ "message" : [ "Overflow in sum of decimals." ]
+ },
"UNABLE_TO_ACQUIRE_MEMORY" : {
"message" : [ "Unable to acquire %s bytes of memory, got %s" ]
},
+ "UNEXPECTED_DATATYPE" : {
Review comment:
I think it makes sense to just keep one or the other; maybe `UNEXPECTED_TYPE`?
##########
File path: core/src/main/resources/error/error-classes.json
##########
@@ -77,6 +109,18 @@
"message" : [ "PARTITION clause cannot contain a non-partition column name: %s" ],
"sqlState" : "42000"
},
+ "NOT_MATCH_FUNCTION_NAME" : {
+ "message" : [ "%s is not matched at addNewFunction" ],
+ "sqlState" : "07000"
Review comment:
42000 may be a better fit
--
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