[GitHub] [spark] MaxGekk commented on a diff in pull request #36416: [SPARK-37938][SQL][TESTS] Use error classes in the parsing errors of partitions

[GitBox <gi...@apache.org>]

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


##########
sql/core/src/test/scala/org/apache/spark/sql/errors/QueryParsingErrorsSuite.scala:
##########
@@ -582,4 +582,64 @@ class QueryParsingErrorsSuite extends QueryTest with QueryErrorsSuiteBase {
           |---------------------------^^^
           |""".stripMargin)
   }
+
+  test("INVALID_SQL_SYNTAX: show table partition key must set value") {
+    validateParsingError(
+      sqlText = "SHOW TABLE EXTENDED IN default LIKE 'employee' PARTITION (grade)",
+      errorClass = "INVALID_SQL_SYNTAX",
+      sqlState = "42000",
+      message =
+        """Invalid SQL syntax: Partition key 'grade' must set value (can't be empty).(line 1, pos 47)
+          |
+          |== SQL ==
+          |SHOW TABLE EXTENDED IN default LIKE 'employee' PARTITION (grade)
+          |-----------------------------------------------^^^
+          |""".stripMargin)
+  }
+
+  test("INVALID_SQL_SYNTAX: expected a column reference for transform bucket") {
+    validateParsingError(
+      sqlText =
+        "CREATE TABLE my_tab(a INT, b STRING) USING parquet PARTITIONED BY (bucket(32, a, 66))",
+      errorClass = "INVALID_SQL_SYNTAX",
+      sqlState = "42000",
+      message =
+        """Invalid SQL syntax: Expected a column reference for transform bucket: 66(line 1, pos 67)
+          |
+          |== SQL ==
+          |CREATE TABLE my_tab(a INT, b STRING) USING parquet PARTITIONED BY (bucket(32, a, 66))
+          |-------------------------------------------------------------------^^^
+          |""".stripMargin)
+  }
+
+  test("UNSUPPORTED_FEATURE: DESC TABLE COLUMN for a specific partition") {
+    validateParsingError(
+      sqlText = "DESCRIBE TABLE EXTENDED customer PARTITION (grade = 'A') customer.age",
+      errorClass = "UNSUPPORTED_FEATURE",
+      errorSubClass = Some("DESC_TABLE_COLUMN_PARTITION"),
+      sqlState = "0A000",
+      message =
+        """The feature is not supported: DESC TABLE COLUMN for a specific partition """ +
+        """is not supported.(line 1, pos 0)""" +

Review Comment:
   Could you remove "is not supported", please. It just duplicate parent't error message: "The feature is not supported"



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryParsingErrors.scala:
##########
@@ -77,7 +77,10 @@ object QueryParsingErrors extends QueryErrorsBase {
   }
 
   def emptyPartitionKeyError(key: String, ctx: PartitionSpecContext): Throwable = {
-    new ParseException(s"Found an empty partition key '$key'.", ctx)
+    new ParseException(
+      errorClass = "INVALID_SQL_SYNTAX",
+      messageParameters = Array(s"Partition key '$key' must set value (can't be empty)."),

Review Comment:
   Could you wrap the key by `toSQLId()`



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryParsingErrors.scala:
##########
@@ -243,7 +246,10 @@ object QueryParsingErrors extends QueryErrorsBase {
 
   def partitionTransformNotExpectedError(
       name: String, describe: String, ctx: ApplyTransformContext): Throwable = {
-    new ParseException(s"Expected a column reference for transform $name: $describe", ctx)
+    new ParseException(
+      errorClass = "INVALID_SQL_SYNTAX",
+      messageParameters = Array(s"Expected a column reference for transform $name: $describe"),

Review Comment:
   Use `toSQLId()` to wrap `name`, please.



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryParsingErrors.scala:
##########
@@ -298,12 +304,18 @@ object QueryParsingErrors extends QueryErrorsBase {
   }
 
   def descColumnForPartitionUnsupportedError(ctx: DescribeRelationContext): Throwable = {
-    new ParseException("DESC TABLE COLUMN for a specific partition is not supported", ctx)
+    new ParseException(
+      errorClass = "UNSUPPORTED_FEATURE",
+      messageParameters = Array("DESC_TABLE_COLUMN_PARTITION"),
+      ctx)
   }
 
   def incompletePartitionSpecificationError(
       key: String, ctx: DescribeRelationContext): Throwable = {
-    new ParseException(s"PARTITION specification is incomplete: `$key`", ctx)
+    new ParseException(
+      errorClass = "INVALID_SQL_SYNTAX",
+      messageParameters = Array(s"PARTITION specification is incomplete: `$key`"),

Review Comment:
   Please, use `toSQLId()` for `key`



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