MaxGekk commented on code in PR #36064:
URL: https://github.com/apache/spark/pull/36064#discussion_r845498313
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -120,6 +120,9 @@
"message" : [ "Invalid SQL syntax: %s" ],
"sqlState" : "42000"
},
+ "INVALID_UDF" : {
+ "message" : [ "Invalid UDF: %s" ]
+ },
Review Comment:
Is there any test for the error class?
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala:
##########
@@ -2310,16 +2297,22 @@ object QueryCompilationErrors {
}
def udfClassDoesNotImplementAnyUDFInterfaceError(className: String):
Throwable = {
- new AnalysisException(s"UDF class $className doesn't implement any UDF
interface")
+ new AnalysisException(
+ errorClass = "UDF_CLASS_NOT_IMPLEMENT_ANY_UDF_INTERFACE",
+ messageParameters = Array(className))
}
def udfClassNotAllowedToImplementMultiUDFInterfacesError(className: String):
Throwable = {
new AnalysisException(
- s"It is invalid to implement multiple UDF interfaces, UDF class
$className")
+ errorClass = "UDF_CLASS_IMPLEMENT_MULTI_UDF_INTERFACE",
+ messageParameters = Array(className))
}
def udfClassWithTooManyTypeArgumentsError(n: Int): Throwable = {
- new AnalysisException(s"UDF class with $n type arguments is not
supported.")
+ new AnalysisException(
+ errorClass = "UNSUPPORTED_FEATURE",
+ messageParameters = Array(
+ s"UDF class with $n type arguments is not supported."))
Review Comment:
Please, add new error class for this. We should have this error message in
the json file.
##########
sql/core/src/test/scala/org/apache/spark/sql/errors/QueryCompilationErrorsSuite.scala:
##########
@@ -168,4 +172,115 @@ class QueryCompilationErrorsSuite extends QueryTest with
SharedSparkSession {
"The feature is not supported: " +
"Pandas UDF aggregate expressions don't support pivot.")
}
+
+ test("NO_HANDLER_FOR_UDAF: No handler for UDAF error") {
+ val functionName = "myCast"
+ withUserDefinedFunction(functionName -> true) {
+ sql(
+ s"""
+ |CREATE TEMPORARY FUNCTION $functionName
+ |AS 'org.apache.spark.sql.errors.MyCastToString'
+ |""".stripMargin)
+
+ val e = intercept[AnalysisException] (
+ sql("SELECT myCast(123) as value")
+ )
+
+ assert(e.errorClass === Some("NO_HANDLER_FOR_UDAF"))
+ assert(e.message ===
+ s"No handler for UDAF 'org.apache.spark.sql.errors.MyCastToString'. " +
+ s"Use sparkSession.udf.register(...) instead.")
Review Comment:
s is not needed. Also adjust indentation of the second line.
##########
sql/core/src/test/scala/org/apache/spark/sql/errors/QueryCompilationErrorsSuite.scala:
##########
@@ -168,4 +172,115 @@ class QueryCompilationErrorsSuite extends QueryTest with
SharedSparkSession {
"The feature is not supported: " +
"Pandas UDF aggregate expressions don't support pivot.")
}
+
+ test("NO_HANDLER_FOR_UDAF: No handler for UDAF error") {
+ val functionName = "myCast"
+ withUserDefinedFunction(functionName -> true) {
+ sql(
+ s"""
+ |CREATE TEMPORARY FUNCTION $functionName
+ |AS 'org.apache.spark.sql.errors.MyCastToString'
+ |""".stripMargin)
+
+ val e = intercept[AnalysisException] (
+ sql("SELECT myCast(123) as value")
Review Comment:
Can you use functionName here?
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala:
##########
@@ -2310,16 +2297,22 @@ object QueryCompilationErrors {
}
def udfClassDoesNotImplementAnyUDFInterfaceError(className: String):
Throwable = {
- new AnalysisException(s"UDF class $className doesn't implement any UDF
interface")
+ new AnalysisException(
+ errorClass = "UDF_CLASS_NOT_IMPLEMENT_ANY_UDF_INTERFACE",
+ messageParameters = Array(className))
}
def udfClassNotAllowedToImplementMultiUDFInterfacesError(className: String):
Throwable = {
Review Comment:
Please, make function name shorter.
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -166,6 +172,12 @@
"message" : [ "The second argument of '%s' function needs to be an
integer." ],
"sqlState" : "22023"
},
+ "UDF_CLASS_IMPLEMENT_MULTI_UDF_INTERFACE" : {
+ "message" : [ "It is invalid to implement multiple UDF interfaces, UDF
class %s" ]
+ },
+ "UDF_CLASS_NOT_IMPLEMENT_ANY_UDF_INTERFACE" : {
Review Comment:
Also, please, make it shorter like `NO_UDF_INTERFACE_ERROR`
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/python/ExtractPythonUDFs.scala:
##########
@@ -264,7 +263,9 @@ object ExtractPythonUDFs extends Rule[LogicalPlan] with
PredicateHelper {
val evalTypes = validUdfs.map(_.evalType).toSet
if (evalTypes.size != 1) {
- throw
QueryCompilationErrors.unexpectedEvalTypesForUDFsError(evalTypes)
+ throw new IllegalStateException(
+ s"Expected udfs have the same evalType but got different
evalTypes: " +
+ s"${evalTypes.mkString(",")}")
Review Comment:
```suggestion
"Expected udfs have the same evalType but got different
evalTypes: " +
evalTypes.mkString(","))
```
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -166,6 +172,12 @@
"message" : [ "The second argument of '%s' function needs to be an
integer." ],
"sqlState" : "22023"
},
+ "UDF_CLASS_IMPLEMENT_MULTI_UDF_INTERFACE" : {
+ "message" : [ "It is invalid to implement multiple UDF interfaces, UDF
class %s" ]
Review Comment:
Not allowed to implement ...?
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -166,6 +172,12 @@
"message" : [ "The second argument of '%s' function needs to be an
integer." ],
"sqlState" : "22023"
},
+ "UDF_CLASS_IMPLEMENT_MULTI_UDF_INTERFACE" : {
Review Comment:
Could you make it shorter like `MULTI_UDF_INTERFACE_ERROR`. We are going to
use error classes in our public doc to refer to errors. For readability, it
would be nice to have not so long names.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
