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/05/23 08:52:04 UTC
[GitHub] [spark] MaxGekk opened a new pull request, #36635: [WIP][SPARK-39255][SQL] Improve error messages
MaxGekk opened a new pull request, #36635:
URL: https://github.com/apache/spark/pull/36635
### What changes were proposed in this pull request?
In the PR, I propose to improve errors of the following error classes:
1. NON_PARTITION_COLUMN
2. UNSUPPORTED_SAVE_MODE
3. INVALID_FIELD_NAME
4. FAILED_SET_ORIGINAL_PERMISSION_BACK
5. NON_LITERAL_PIVOT_VALUES
6. INVALID_SYNTAX_FOR_CAST
### Why are the changes needed?
To improve user experience with Spark SQL by making error message more clear.
### Does this PR introduce _any_ user-facing change?
Yes, it changes user-facing error messages.
### How was this patch tested?
By running the affected test suites:
```
$ build/sbt "sql/testOnly org.apache.spark.sql.SQLQueryTestSuite"
$ build/sbt "sql/testOnly *QueryCompilationErrorsDSv2Suite"
$ build/sbt "sql/testOnly *QueryCompilationErrorsSuite"
$ build/sbt "sql/testOnly *QueryExecutionAnsiErrorsSuite"
$ build/sbt "sql/testOnly *QueryExecutionErrorsSuite"
$ 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] gengliangwang commented on a diff in pull request #36635: [SPARK-39255][SQL] Improve error messages
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on code in PR #36635:
URL: https://github.com/apache/spark/pull/36635#discussion_r879684995
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -23,7 +23,7 @@
"message" : [ "Cannot up cast <value> from <sourceType> to <targetType>.\n<details>" ]
},
"CAST_INVALID_INPUT" : {
- "message" : [ "The value <value> of the type <sourceType> cannot be cast to <targetType> because it is malformed. To return NULL instead, use `try_cast`. If necessary set <config> to \"false\" to bypass this error." ],
+ "message" : [ "The value <value> of the type <sourceType> cannot be cast to <targetType> because it is malformed. Correct the syntax for the value before casting it, or change the type to one appropriate for the value. To return NULL instead, use `try_cast`. If necessary set <config> to \"false\" to bypass this error." ],
Review Comment:
What do you mean by "Correct the syntax for the value"? Do you mean "Correct the values as per the syntax"?
--
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 #36635: [SPARK-39255][SQL] Improve error messages
Posted by GitBox <gi...@apache.org>.
MaxGekk closed pull request #36635: [SPARK-39255][SQL] Improve error messages
URL: https://github.com/apache/spark/pull/36635
--
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 #36635: [SPARK-39255][SQL] Improve error messages
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on PR #36635:
URL: https://github.com/apache/spark/pull/36635#issuecomment-1134985533
cc @srielau
--
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 #36635: [SPARK-39255][SQL] Improve error messages
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on code in PR #36635:
URL: https://github.com/apache/spark/pull/36635#discussion_r879761080
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -48,13 +48,13 @@
"FAILED_EXECUTE_UDF" : {
"message" : [ "Failed to execute user defined function (<functionName>: (<signature>) => <result>)" ]
},
+ "FAILED_PERMISSION_RESET_ORIGINAL" : {
Review Comment:
SET_PERMISSION_BACK is shorter than RESET_PERMISSION_TO_ORIGINAL ;-)
--
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 #36635: [SPARK-39255][SQL] Improve error messages
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on PR #36635:
URL: https://github.com/apache/spark/pull/36635#issuecomment-1135782496
Merging to master. Thank you, @gengliangwang and @cloud-fan @srielau for review.
--
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 #36635: [SPARK-39255][SQL] Improve error messages
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on code in PR #36635:
URL: https://github.com/apache/spark/pull/36635#discussion_r879750969
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -48,13 +48,13 @@
"FAILED_EXECUTE_UDF" : {
"message" : [ "Failed to execute user defined function (<functionName>: (<signature>) => <result>)" ]
},
+ "FAILED_PERMISSION_RESET_ORIGINAL" : {
Review Comment:
> How about FAILED_TO_SET_ORIGINAL_PERMISSION_BACK?
It would be nice to have shorter names for error classes since we are going to use them as tags in docs headers.
--
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] gengliangwang commented on a diff in pull request #36635: [SPARK-39255][SQL] Improve error messages
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on code in PR #36635:
URL: https://github.com/apache/spark/pull/36635#discussion_r879685343
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -23,7 +23,7 @@
"message" : [ "Cannot up cast <value> from <sourceType> to <targetType>.\n<details>" ]
},
"CAST_INVALID_INPUT" : {
- "message" : [ "The value <value> of the type <sourceType> cannot be cast to <targetType> because it is malformed. To return NULL instead, use `try_cast`. If necessary set <config> to \"false\" to bypass this error." ],
+ "message" : [ "The value <value> of the type <sourceType> cannot be cast to <targetType> because it is malformed. Correct the syntax for the value before casting it, or change the type to one appropriate for the value. To return NULL instead, use `try_cast`. If necessary set <config> to \"false\" to bypass this error." ],
Review Comment:
nit: "change the type to one appropriate for the value" => "change an appropriate type for the value"?
--
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] gengliangwang commented on a diff in pull request #36635: [SPARK-39255][SQL] Improve error messages
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on code in PR #36635:
URL: https://github.com/apache/spark/pull/36635#discussion_r879687277
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -305,7 +305,7 @@
"message" : [ "an existent path." ]
},
"NON_EXISTENT_PATH" : {
- "message" : [ "a not existent path." ]
+ "message" : [ "a non existent path." ]
Review Comment:
```suggestion
"message" : [ "a non-existent path." ]
```
--
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] srielau commented on a diff in pull request #36635: [SPARK-39255][SQL] Improve error messages
Posted by GitBox <gi...@apache.org>.
srielau commented on code in PR #36635:
URL: https://github.com/apache/spark/pull/36635#discussion_r879757479
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -48,13 +48,13 @@
"FAILED_EXECUTE_UDF" : {
"message" : [ "Failed to execute user defined function (<functionName>: (<signature>) => <result>)" ]
},
+ "FAILED_PERMISSION_RESET_ORIGINAL" : {
Review Comment:
Yeah, it's hard to come up with a pattern that really works.
Some thought:
- The error class is typically followed by the message. No need for a sentence.. That's what the message does.
- Short is beautiful
- Try to avoid stating that an error is an error in the name.. provides no new information. It's an error class! ;-)
How about
RESET_PERMISSION_TO_ORIGINAL
(again we KNOW it's failed)
--
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] gengliangwang commented on pull request #36635: [SPARK-39255][SQL] Improve error messages
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on PR #36635:
URL: https://github.com/apache/spark/pull/36635#issuecomment-1134920613
LGTM except some minor comments. Thanks for the work!
--
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 #36635: [SPARK-39255][SQL] Improve error messages
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on PR #36635:
URL: https://github.com/apache/spark/pull/36635#issuecomment-1134909286
The test failure:
```
[info] - Hide credentials in show create table *** FAILED *** (31 milliseconds)
[info] "[0,10000000d5,5420455441455243,62617420454c4241,414e20200a282031,4e4952545320454d,45485420200a2c47,a29544e49204449,726f20474e495355,6568636170612e67,732e6b726170732e,a6362646a2e6c71,20534e4f4954504f,7462642720200a28,203d2027656c6261,45502e5453455427,200a2c27454c504f,6f77737361702720,2a27203d20276472,2a2a2a2a2a2a2a2a,6574636164657228,2720200a2c272964,27203d20276c7275,2a2a2a2a2a2a2a2a,746361646572282a,20200a2c27296465,3d20277265737527,7355747365742720,a29277265]" did not contain "TEST.PEOPLE" (JDBCSuite.scala:1146)
```
is not related to the PR, I believe. @gengliangwang @HyukjinKwon @cloud-fan Could you have a look at this, please.
--
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 #36635: [SPARK-39255][SQL] Improve error messages
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on code in PR #36635:
URL: https://github.com/apache/spark/pull/36635#discussion_r879750969
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -48,13 +48,13 @@
"FAILED_EXECUTE_UDF" : {
"message" : [ "Failed to execute user defined function (<functionName>: (<signature>) => <result>)" ]
},
+ "FAILED_PERMISSION_RESET_ORIGINAL" : {
Review Comment:
> How about FAILED_TO_SET_ORIGINAL_PERMISSION_BACK?
I would be nice to have shorter names for error classes since we are going to use them as tags in docs headers.
--
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 #36635: [SPARK-39255][SQL] Improve error messages
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on code in PR #36635:
URL: https://github.com/apache/spark/pull/36635#discussion_r879754357
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -23,7 +23,7 @@
"message" : [ "Cannot up cast <value> from <sourceType> to <targetType>.\n<details>" ]
},
"CAST_INVALID_INPUT" : {
- "message" : [ "The value <value> of the type <sourceType> cannot be cast to <targetType> because it is malformed. To return NULL instead, use `try_cast`. If necessary set <config> to \"false\" to bypass this error." ],
+ "message" : [ "The value <value> of the type <sourceType> cannot be cast to <targetType> because it is malformed. Correct the syntax for the value before casting it, or change the type to one appropriate for the value. To return NULL instead, use `try_cast`. If necessary set <config> to \"false\" to bypass this error." ],
Review Comment:
> Do you mean "Correct the values as per the syntax"?
Yep
> nit: "change the type to one appropriate for the value" => "change an appropriate type for the value"?
Maybe just, "change the target type"?
How about:
`Correct the value as per the syntax, or change its target type.`?
--
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] gengliangwang commented on a diff in pull request #36635: [SPARK-39255][SQL] Improve error messages
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on code in PR #36635:
URL: https://github.com/apache/spark/pull/36635#discussion_r879688689
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -48,13 +48,13 @@
"FAILED_EXECUTE_UDF" : {
"message" : [ "Failed to execute user defined function (<functionName>: (<signature>) => <result>)" ]
},
+ "FAILED_PERMISSION_RESET_ORIGINAL" : {
Review Comment:
How about `FAILED_TO_SET_ORIGINAL_PERMISSION_BACK`?
I didn't follow the error message improvement project closely. Do we have a naming pattern?
--
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 #36635: [SPARK-39255][SQL] Improve error messages
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on code in PR #36635:
URL: https://github.com/apache/spark/pull/36635#discussion_r879748546
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -48,13 +48,13 @@
"FAILED_EXECUTE_UDF" : {
"message" : [ "Failed to execute user defined function (<functionName>: (<signature>) => <result>)" ]
},
+ "FAILED_PERMISSION_RESET_ORIGINAL" : {
Review Comment:
> Do we have a naming pattern?
no, we don't as far as I know.
--
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