You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/04/30 03:53:09 UTC
[GitHub] [spark] panbingkun opened a new pull request, #36416: [SPARK-37938][SQL][TESTS] Use error classes in the parsing errors of partitions
panbingkun opened a new pull request, #36416:
URL: https://github.com/apache/spark/pull/36416
## What changes were proposed in this pull request?
Migrate the following errors in QueryParsingErrors onto use error classes:
- emptyPartitionKeyError => INVALID_SQL_SYNTAX
- partitionTransformNotExpectedError => INVALID_SQL_SYNTAX
- descColumnForPartitionUnsupportedError => UNSUPPORTED_FEATURE.DESC_TABLE_COLUMN_PARTITION
- incompletePartitionSpecificationError => INVALID_SQL_SYNTAX
### Why are the changes needed?
Porting parsing errors of partitions to new error framework, improve test coverage, and document expected error messages in tests.
### Does this PR introduce any user-facing change?
No
### How was this patch tested?
By running new test:
```
$ build/sbt "sql/testOnly *QueryParsingErrorsSuite*"
```
--
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
[GitHub] [spark] MaxGekk commented on pull request #36416: [SPARK-37938][SQL][TESTS] Use error classes in the parsing errors of partitions
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on PR #36416:
URL: https://github.com/apache/spark/pull/36416#issuecomment-1118766129
+1, LGTM. Merging to master.
Thank you, @panbingkun.
--
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
[GitHub] [spark] MaxGekk commented on a diff in pull request #36416: [SPARK-37938][SQL][TESTS] Use error classes in the parsing errors of partitions
Posted by 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
[GitHub] [spark] AmplabJenkins commented on pull request #36416: [SPARK-37938][SQL][TESTS] Use error classes in the parsing errors of partitions
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on PR #36416:
URL: https://github.com/apache/spark/pull/36416#issuecomment-1114171064
Can one of the admins verify this patch?
--
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
[GitHub] [spark] MaxGekk closed pull request #36416: [SPARK-37938][SQL][TESTS] Use error classes in the parsing errors of partitions
Posted by GitBox <gi...@apache.org>.
MaxGekk closed pull request #36416: [SPARK-37938][SQL][TESTS] Use error classes in the parsing errors of partitions
URL: https://github.com/apache/spark/pull/36416
--
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