[GitHub] [spark] MaxGekk commented on a diff in pull request #39501: [SPARK-41295][SPARK-41296][SQL] Rename the error classes

["MaxGekk (via GitHub)" <gi...@apache.org>]

MaxGekk commented on code in PR #39501:
URL: https://github.com/apache/spark/pull/39501#discussion_r1092846584


##########
core/src/main/resources/error/error-classes.json:
##########
@@ -738,12 +744,24 @@
     ],
     "sqlState" : "42K05"
   },
+  "INVALID_EXTRACT_BASE_FIELD_TYPE" : {
+    "message" : [
+      "Can't extract value from <base>: need a complex type [struct, array, map] but got <other>."

Review Comment:
   nit:
   ```suggestion
         "Can't extract a value from <base>. Need a complex type [struct, array, map] but got <other>."
   ```



##########
core/src/main/resources/error/error-classes.json:
##########
@@ -738,12 +744,24 @@
     ],
     "sqlState" : "42K05"
   },
+  "INVALID_EXTRACT_BASE_FIELD_TYPE" : {
+    "message" : [
+      "Can't extract value from <base>: need a complex type [struct, array, map] but got <other>."
+    ],
+    "sqlState" : "42000"
+  },
   "INVALID_EXTRACT_FIELD" : {
     "message" : [
       "Cannot extract <field> from <expr>."
     ],
     "sqlState" : "42601"
   },
+  "INVALID_EXTRACT_FIELD_TYPE" : {
+    "message" : [
+      "Field name should be String Literal, but it's <extraction>."

Review Comment:
   ```suggestion
         "Field name should be a non-null string literal, but it's <extraction>."
   ```



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala:
##########
@@ -2104,7 +2104,7 @@ private[sql] object QueryCompilationErrors extends QueryErrorsBase {
 
   def ambiguousReferenceToFieldsError(fields: String): Throwable = {
     new AnalysisException(
-      errorClass = "_LEGACY_ERROR_TEMP_1209",
+      errorClass = "AMBIGUOUS_REFERENCE_TO_FIELDS",
       messageParameters = Map("fields" -> fields))

Review Comment:
   Fields should be quoted by `toSQLId()`



##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ComplexTypeSuite.scala:
##########
@@ -460,24 +460,38 @@ class ComplexTypeSuite extends SparkFunSuite with ExpressionEvalHelper {
   }
 
   test("error message of ExtractValue") {
-    val structType = StructType(StructField("a", StringType, true) :: Nil)
-    val otherType = StringType
-
-    def checkErrorMessage(
-      childDataType: DataType,
-      fieldDataType: DataType,
-      errorMessage: String): Unit = {
-      val e = intercept[org.apache.spark.sql.AnalysisException] {
+    checkError(
+      exception = intercept[AnalysisException](
         ExtractValue(

Review Comment:
   Please, remove duplicate tests here. The added tests are enough.



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala:
##########
@@ -1117,13 +1117,13 @@ private[sql] object QueryCompilationErrors extends QueryErrorsBase {
     dataType match {
       case StructType(_) =>
         new AnalysisException(
-          errorClass = "_LEGACY_ERROR_TEMP_1105",
+          errorClass = "INVALID_EXTRACT_FIELD_TYPE",
           messageParameters = Map("extraction" -> extraction.toString))

Review Comment:
   ```suggestion
             messageParameters = Map("extraction" -> toSQLExpr(extraction)))
   ```



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala:
##########
@@ -1117,13 +1117,13 @@ private[sql] object QueryCompilationErrors extends QueryErrorsBase {
     dataType match {
       case StructType(_) =>
         new AnalysisException(
-          errorClass = "_LEGACY_ERROR_TEMP_1105",
+          errorClass = "INVALID_EXTRACT_FIELD_TYPE",
           messageParameters = Map("extraction" -> extraction.toString))
       case other =>
         new AnalysisException(
-          errorClass = "_LEGACY_ERROR_TEMP_1106",
+          errorClass = "INVALID_EXTRACT_BASE_FIELD_TYPE",
           messageParameters = Map(
-            "child" -> child.toString,
+            "base" -> child.toString,
             "other" -> other.catalogString))

Review Comment:
   ```suggestion
               "child" -> toSQLExpr(child),
               "other" -> toSQLType(other)))
   ```



-- 
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