MaxGekk commented on code in PR #38588:
URL: https://github.com/apache/spark/pull/38588#discussion_r1018715875
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala:
##########
@@ -2082,10 +2084,12 @@ private[sql] object QueryCompilationErrors extends
QueryErrorsBase {
messageParameters = Map("fields" -> fields))
}
- def secondArgumentInFunctionIsNotBooleanLiteralError(funcName: String):
Throwable = {
+ def secondArgumentInFunctionIsNotBooleanLiteralError(
+ funcName: String, parameter: String): Throwable = {
new AnalysisException(
- errorClass = "_LEGACY_ERROR_TEMP_1210",
- messageParameters = Map("funcName" -> funcName))
+ errorClass = "INVALID_PARAMETER_VALUE",
+ messageParameters =
+ Map("parameter" -> parameter, "functionName" -> funcName, "expected"
-> "Boolean"))
Review Comment:
Please, quote parameters.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala:
##########
@@ -1085,10 +1085,11 @@ private[sql] object QueryCompilationErrors extends
QueryErrorsBase {
messageParameters = Map("clz" -> clz.toString))
}
- def secondArgumentNotDoubleLiteralError(): Throwable = {
+ def secondArgumentNotDoubleLiteralError(functionName: String, parameter:
String): Throwable = {
new AnalysisException(
- errorClass = "_LEGACY_ERROR_TEMP_1104",
- messageParameters = Map.empty)
+ errorClass = "INVALID_PARAMETER_VALUE",
+ messageParameters =
+ Map("parameter" -> parameter, "functionName" -> functionName,
"expected" -> "Double"))
Review Comment:
The error message parameters should be quoted:
functionName -> toSQLId(functionName)
Double -> toSQLType(DoubleType)
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala:
##########
@@ -2034,11 +2035,12 @@ private[sql] object QueryCompilationErrors extends
QueryErrorsBase {
}
def secondArgumentOfFunctionIsNotIntegerError(
- function: String, e: NumberFormatException): Throwable = {
+ function: String, parameter: String, e: NumberFormatException):
Throwable = {
// The second argument of {function} function needs to be an integer
new AnalysisException(
- errorClass = "SECOND_FUNCTION_ARGUMENT_NOT_INTEGER",
- messageParameters = Map("functionName" -> function),
+ errorClass = "INVALID_PARAMETER_VALUE",
+ messageParameters =
+ Map("parameter" -> parameter, "functionName" -> function, "expected"
-> "Integer"),
Review Comment:
The same comments as for secondArgumentNotDoubleLiteralError
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -2109,11 +2103,6 @@
"Unsupported component type <clz> in arrays."
]
},
- "_LEGACY_ERROR_TEMP_1104" : {
- "message" : [
- "The second argument should be a double literal."
Review Comment:
This has additional requirements:
1. Be a literal
2. and it can be a decimal literal
see
```scala
def validateDoubleLiteral(exp: Expression): Double = exp match {
case Literal(d: Double, DoubleType) => d
case Literal(dec: Decimal, _) => dec.toDouble
case _ =>
throw QueryCompilationErrors.secondArgumentNotDoubleLiteralError
}
```
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/aggregate/FirstLastTestSuite.scala:
##########
@@ -112,14 +112,14 @@ class FirstLastTestSuite extends SparkFunSuite {
val msg1 = intercept[AnalysisException] {
new First(input, Literal(1, IntegerType))
}.getMessage
- assert(msg1.contains("The second argument in first should be a boolean
literal"))
+ assert(msg1.contains("INVALID_PARAMETER_VALUE"))
Review Comment:
Use checkError() to make tests independent from error messages. In this way,
tech editors can change error message templates w/ modifying internal Spark
tests.
--
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]
