Skip to content

[SPARK-41147][SQL] Assign a name to the legacy error class _LEGACY_ERROR_TEMP_1042 #38664

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
itholic wants to merge 18 commits into from
Closed

[SPARK-41147][SQL] Assign a name to the legacy error class _LEGACY_ERROR_TEMP_1042 #38664

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
fcbcd44
[SPARK-41147][SQL] Assign a name to the legacy error class _LEGACY_ER…
itholic Nov 15, 2022
032e1af
use DATATYPE_MISMATCH.WRONG_NUM_ARGS
itholic Nov 16, 2022
923fd16
Fix codes & Regen the goldenfiles
itholic Nov 16, 2022
b51829d
fix test to checkError
itholic Nov 16, 2022
b326c3e
remove comment
itholic Nov 17, 2022
ce20446
Use toPrettySQL
itholic Nov 17, 2022
57748f1
Merge branch 'master' of https://github.com/apache/spark into SPARK-4…
itholic Nov 17, 2022
e79d42e
regen golden files
itholic Nov 17, 2022
c57b22b
fix test
itholic Nov 17, 2022
ebcd21a
Merge branch 'master' of https://github.com/apache/spark into SPARK-4…
itholic Nov 18, 2022
f96b2f4
Merge branch 'master' of https://github.com/apache/spark into SPARK-4…
itholic Nov 18, 2022
43edd03
Resolve the comments
itholic Nov 22, 2022
543e34d
fix scala style
itholic Nov 22, 2022
5540bc4
fix scala style
itholic Nov 22, 2022
a474607
fix style
itholic Nov 22, 2022
8e4cd7d
resolve conflicts
itholic Nov 24, 2022
997165e
regen goldenfiles
itholic Nov 24, 2022
da80f72
fix test
itholic Nov 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions core/src/main/resources/error/error-classes.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1925,11 +1925,6 @@
"Undefined function <name>."
]
},
"_LEGACY_ERROR_TEMP_1042" : {
"message" : [
"Invalid number of arguments for function <name>. Expected: <expectedInfo>; Found: <actualNumber>."
]
},
"_LEGACY_ERROR_TEMP_1043" : {
"message" : [
"Invalid arguments for function <name>."
Expand Down
6 changes: 5 additions & 1 deletion sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import org.apache.spark.sql.catalyst.expressions.aggregate._
import org.apache.spark.sql.catalyst.expressions.xml._
import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, Range}
import org.apache.spark.sql.catalyst.trees.TreeNodeTag
import org.apache.spark.sql.catalyst.util.toPrettySQL
import org.apache.spark.sql.errors.QueryCompilationErrors
import org.apache.spark.sql.types._

Expand Down Expand Up @@ -146,7 +147,10 @@ object FunctionRegistryBase {
.filter(_.getParameterTypes.forall(_ == classOf[Expression]))
.map(_.getParameterCount).distinct.sorted
throw QueryCompilationErrors.invalidFunctionArgumentNumberError(
validParametersCount, name, params.length)
expressions.map(toPrettySQL(_)).mkString(","),

@MaxGekk MaxGekk Nov 18, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you pass Seq[Expression] into invalidFunctionArgumentNumberError(), and form a string inside of the method as we do in other places like:

  def moreThanOneGeneratorError(generators: Seq[Expression], clause: String): Throwable = {
    new AnalysisException(
      errorClass = "UNSUPPORTED_GENERATOR.MULTI_GENERATOR",
      messageParameters = Map(
        "clause" -> clause,
        "num" -> generators.size.toString,
        "generators" -> generators.map(toSQLExpr).mkString(", ")))
  }

For instance, if we change the implementation of toSQLExpr(), your exceptions won't pick up new impl.

@itholic itholic Nov 22, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good! Just addressed it.

validParametersCount,
name,
params.length)
}
try {
f.newInstance(expressions : _*).asInstanceOf[T]
Expand Down
5 changes: 3 additions & 2 deletions ...catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/Average.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import org.apache.spark.sql.catalyst.dsl.expressions._
import org.apache.spark.sql.catalyst.expressions._
import org.apache.spark.sql.catalyst.trees.{SQLQueryContext, UnaryLike}
import org.apache.spark.sql.catalyst.trees.TreePattern.{AVERAGE, TreePattern}
import org.apache.spark.sql.catalyst.util.TypeUtils
import org.apache.spark.sql.catalyst.util.{toPrettySQL, TypeUtils}
import org.apache.spark.sql.errors.QueryCompilationErrors
import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.sql.types._
Expand Down Expand Up @@ -162,7 +162,8 @@ object TryAverageExpressionBuilder extends ExpressionBuilder {
if (numArgs == 1) {
Average(expressions.head, EvalMode.TRY)
} else {
throw QueryCompilationErrors.invalidFunctionArgumentNumberError(Seq(1, 2), funcName, numArgs)
throw QueryCompilationErrors.invalidFunctionArgumentNumberError(
expressions.map(toPrettySQL(_)).mkString(","), Seq(1, 2), funcName, numArgs)
}
}
}
5 changes: 3 additions & 2 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/Sum.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import org.apache.spark.sql.catalyst.dsl.expressions._
import org.apache.spark.sql.catalyst.expressions.{EvalMode, _}
import org.apache.spark.sql.catalyst.trees.{SQLQueryContext, UnaryLike}
import org.apache.spark.sql.catalyst.trees.TreePattern.{SUM, TreePattern}
import org.apache.spark.sql.catalyst.util.TypeUtils
import org.apache.spark.sql.catalyst.util.{toPrettySQL, TypeUtils}
import org.apache.spark.sql.errors.QueryCompilationErrors
import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.sql.types._
Expand Down Expand Up @@ -219,7 +219,8 @@ object TrySumExpressionBuilder extends ExpressionBuilder {
if (numArgs == 1) {
Sum(expressions.head, EvalMode.TRY)
} else {
throw QueryCompilationErrors.invalidFunctionArgumentNumberError(Seq(1, 2), funcName, numArgs)
throw QueryCompilationErrors.invalidFunctionArgumentNumberError(
expressions.map(toPrettySQL(_)).mkString(","), Seq(1, 2), funcName, numArgs)
}
}
}
22 changes: 14 additions & 8 deletions ...talyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import org.apache.spark.sql.catalyst.analysis.{ExpressionBuilder, FunctionRegist
import org.apache.spark.sql.catalyst.expressions.codegen._
import org.apache.spark.sql.catalyst.expressions.codegen.Block._
import org.apache.spark.sql.catalyst.trees.TreePattern._
import org.apache.spark.sql.catalyst.util.{DateTimeUtils, LegacyDateFormats, TimestampFormatter}
import org.apache.spark.sql.catalyst.util.{toPrettySQL, DateTimeUtils, LegacyDateFormats, TimestampFormatter}
import org.apache.spark.sql.catalyst.util.DateTimeConstants._
import org.apache.spark.sql.catalyst.util.DateTimeUtils._
import org.apache.spark.sql.catalyst.util.LegacyDateFormats.SIMPLE_DATE_FORMAT
Expand Down Expand Up @@ -172,7 +172,7 @@ object CurDateExpressionBuilder extends ExpressionBuilder {
CurrentDate()
} else {
throw QueryCompilationErrors.invalidFunctionArgumentNumberError(
Seq.empty, funcName, expressions.length)
expressions.map(toPrettySQL(_)).mkString(","), Seq.empty, funcName, expressions.length)
}
}
}
Expand Down Expand Up @@ -1143,7 +1143,8 @@ object ParseToTimestampNTZExpressionBuilder extends ExpressionBuilder {
if (numArgs == 1 || numArgs == 2) {
ParseToTimestamp(expressions(0), expressions.drop(1).lastOption, TimestampNTZType)
} else {
throw QueryCompilationErrors.invalidFunctionArgumentNumberError(Seq(1, 2), funcName, numArgs)
throw QueryCompilationErrors.invalidFunctionArgumentNumberError(
expressions.map(toPrettySQL(_)).mkString(","), Seq(1, 2), funcName, numArgs)
}
}
}
Expand Down Expand Up @@ -1180,7 +1181,8 @@ object ParseToTimestampLTZExpressionBuilder extends ExpressionBuilder {
if (numArgs == 1 || numArgs == 2) {
ParseToTimestamp(expressions(0), expressions.drop(1).lastOption, TimestampType)
} else {
throw QueryCompilationErrors.invalidFunctionArgumentNumberError(Seq(1, 2), funcName, numArgs)
throw QueryCompilationErrors.invalidFunctionArgumentNumberError(
expressions.map(toPrettySQL(_)).mkString(","), Seq(1, 2), funcName, numArgs)
}
}
}
Expand Down Expand Up @@ -1224,7 +1226,8 @@ object TryToTimestampExpressionBuilder extends ExpressionBuilder {
SQLConf.get.timestampType,
failOnError = false)
} else {
throw QueryCompilationErrors.invalidFunctionArgumentNumberError(Seq(1, 2), funcName, numArgs)
throw QueryCompilationErrors.invalidFunctionArgumentNumberError(
expressions.map(toPrettySQL(_)).mkString(","), Seq(1, 2), funcName, numArgs)
}
}
}
Expand Down Expand Up @@ -2518,7 +2521,8 @@ object MakeTimestampNTZExpressionBuilder extends ExpressionBuilder {
expressions(5),
dataType = TimestampNTZType)
} else {
throw QueryCompilationErrors.invalidFunctionArgumentNumberError(Seq(6), funcName, numArgs)
throw QueryCompilationErrors.invalidFunctionArgumentNumberError(
expressions.map(toPrettySQL(_)).mkString(","), Seq(6), funcName, numArgs)
}
}
}
Expand Down Expand Up @@ -2566,7 +2570,8 @@ object MakeTimestampLTZExpressionBuilder extends ExpressionBuilder {
expressions.drop(6).lastOption,
dataType = TimestampType)
} else {
throw QueryCompilationErrors.invalidFunctionArgumentNumberError(Seq(6), funcName, numArgs)
throw QueryCompilationErrors.invalidFunctionArgumentNumberError(
expressions.map(toPrettySQL(_)).mkString(","), Seq(6), funcName, numArgs)
}
}
}
Expand Down Expand Up @@ -2837,7 +2842,8 @@ object DatePartExpressionBuilder extends ExpressionBuilder {
val source = expressions(1)
Extract(field, source, Extract.createExpr(funcName, field, source))
} else {
throw QueryCompilationErrors.invalidFunctionArgumentNumberError(Seq(2), funcName, numArgs)
throw QueryCompilationErrors.invalidFunctionArgumentNumberError(
expressions.map(toPrettySQL(_)).mkString(","), Seq(2), funcName, numArgs)
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import org.apache.spark.sql.catalyst.analysis.TypeCheckResult.{DataTypeMismatch,
import org.apache.spark.sql.catalyst.expressions.Cast._
import org.apache.spark.sql.catalyst.expressions.codegen._
import org.apache.spark.sql.catalyst.expressions.codegen.Block._
import org.apache.spark.sql.catalyst.util.{NumberConverter, TypeUtils}
import org.apache.spark.sql.catalyst.util.{toPrettySQL, NumberConverter, TypeUtils}
import org.apache.spark.sql.errors.{QueryCompilationErrors, QueryExecutionErrors}
import org.apache.spark.sql.types._
import org.apache.spark.unsafe.types.UTF8String
Expand Down Expand Up @@ -288,7 +288,8 @@ trait CeilFloorExpressionBuilderBase extends ExpressionBuilder {
}
buildWithTwoParams(expressions(0), scale)
} else {
throw QueryCompilationErrors.invalidFunctionArgumentNumberError(Seq(2), funcName, numArgs)
throw QueryCompilationErrors.invalidFunctionArgumentNumberError(
expressions.map(toPrettySQL(_)).mkString(","), Seq(2), funcName, numArgs)
}
}
}
Expand Down
11 changes: 7 additions & 4 deletions ...catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import org.apache.spark.sql.catalyst.expressions.codegen.Block._
import org.apache.spark.sql.catalyst.expressions.objects.StaticInvoke
import org.apache.spark.sql.catalyst.trees.{BinaryLike, SQLQueryContext}
import org.apache.spark.sql.catalyst.trees.TreePattern.{TreePattern, UPPER_OR_LOWER}
import org.apache.spark.sql.catalyst.util.{ArrayData, GenericArrayData, TypeUtils}
import org.apache.spark.sql.catalyst.util.{toPrettySQL, ArrayData, GenericArrayData, TypeUtils}
import org.apache.spark.sql.errors.{QueryCompilationErrors, QueryExecutionErrors}
import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.sql.types._
Expand Down Expand Up @@ -507,7 +507,8 @@ trait StringBinaryPredicateExpressionBuilderBase extends ExpressionBuilder {
createStringPredicate(expressions(0), expressions(1))
}
} else {
throw QueryCompilationErrors.invalidFunctionArgumentNumberError(Seq(2), funcName, numArgs)
throw QueryCompilationErrors.invalidFunctionArgumentNumberError(
expressions.map(toPrettySQL(_)).mkString(","), Seq(2), funcName, numArgs)
}
}

Expand Down Expand Up @@ -1505,7 +1506,8 @@ trait PadExpressionBuilderBase extends ExpressionBuilder {
createStringPad(expressions(0), expressions(1), expressions(2))
}
} else {
throw QueryCompilationErrors.invalidFunctionArgumentNumberError(Seq(2, 3), funcName, numArgs)
throw QueryCompilationErrors.invalidFunctionArgumentNumberError(
expressions.map(toPrettySQL(_)).mkString(","), Seq(2, 3), funcName, numArgs)
}
}

Expand Down Expand Up @@ -2424,7 +2426,8 @@ object Decode {
def createExpr(params: Seq[Expression]): Expression = {
params.length match {
case 0 | 1 =>
throw QueryCompilationErrors.invalidFunctionArgumentsError("decode", "2", params.length)
throw QueryCompilationErrors.invalidFunctionArgumentsError(
params.map(toPrettySQL(_)).mkString(","), "decode", "2", params.length)
case 2 => StringDecode(params.head, params.last)
case _ =>
val input = params.head
Expand Down
18 changes: 11 additions & 7 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -637,17 +637,21 @@ private[sql] object QueryCompilationErrors extends QueryErrorsBase {
}

def invalidFunctionArgumentsError(
name: String, expectedInfo: String, actualNumber: Int): Throwable = {
sqlExpr: String, name: String, expectedInfo: String, actualNumber: Int): Throwable = {
new AnalysisException(
errorClass = "_LEGACY_ERROR_TEMP_1042",
errorClass = "DATATYPE_MISMATCH.WRONG_NUM_ARGS",
messageParameters = Map(
"name" -> name,
"expectedInfo" -> expectedInfo,
"actualNumber" -> actualNumber.toString))
"sqlExpr" -> sqlExpr,
"functionName" -> name,
Comment thread
itholic marked this conversation as resolved.
Outdated
"expectedNum" -> expectedInfo,
"actualNum" -> actualNumber.toString))
}

def invalidFunctionArgumentNumberError(
validParametersCount: Seq[Int], name: String, actualNumber: Int): Throwable = {
sqlExpr: String,
validParametersCount: Seq[Int],
name: String,
actualNumber: Int): Throwable = {
if (validParametersCount.length == 0) {
new AnalysisException(
errorClass = "_LEGACY_ERROR_TEMP_1043",
Expand All @@ -659,7 +663,7 @@ private[sql] object QueryCompilationErrors extends QueryErrorsBase {
validParametersCount.init.mkString("one of ", ", ", " and ") +
validParametersCount.last
}
invalidFunctionArgumentsError(name, expectedNumberOfParameters, actualNumber)
invalidFunctionArgumentsError(sqlExpr, name, expectedNumberOfParameters, actualNumber)
}
}

Expand Down
Loading