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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org