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/06/12 13:47:25 UTC
[GitHub] [spark] panbingkun opened a new pull request, #36852: [SPARK-38700][SQL][3.3] Use error classes in the execution errors of save mode
panbingkun opened a new pull request, #36852:
URL: https://github.com/apache/spark/pull/36852
### What changes were proposed in this pull request?
Migrate the following errors in QueryExecutionErrors:
* unsupportedSaveModeError -> UNSUPPORTED_SAVE_MODE
### Why are the changes needed?
Porting execution errors of unsupported saveMode to new error framework.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Add new UT.
--
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 #36852: [SPARK-38700][SQL][3.3] Use error classes in the execution errors of save mode
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on PR #36852:
URL: https://github.com/apache/spark/pull/36852#issuecomment-1153230519
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 #36852: [SPARK-38700][SQL][3.3] Use error classes in the execution errors of save mode
Posted by GitBox <gi...@apache.org>.
MaxGekk closed pull request #36852: [SPARK-38700][SQL][3.3] Use error classes in the execution errors of save mode
URL: https://github.com/apache/spark/pull/36852
--
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] dongjoon-hyun commented on a diff in pull request #36852: [SPARK-38700][SQL][3.3] Use error classes in the execution errors of save mode
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #36852:
URL: https://github.com/apache/spark/pull/36852#discussion_r895260558
##########
core/src/main/scala/org/apache/spark/ErrorInfo.scala:
##########
@@ -28,14 +28,30 @@ import com.fasterxml.jackson.module.scala.DefaultScalaModule
import org.apache.spark.util.Utils
+/**
+ * Information associated with an error subclass.
+ *
+ * @param subClass SubClass associated with this class.
+ * @param message C-style message format compatible with printf.
+ * The error message is constructed by concatenating the lines with newlines.
+ */
+private[spark] case class ErrorSubInfo(message: Seq[String]) {
+ // For compatibility with multi-line error messages
+ @JsonIgnore
+ val messageFormat: String = message.mkString("\n")
+}
+
/**
* Information associated with an error class.
*
* @param sqlState SQLSTATE associated with this class.
+ * @param subClass A sequence of subclasses
* @param message C-style message format compatible with printf.
* The error message is constructed by concatenating the lines with newlines.
*/
-private[spark] case class ErrorInfo(message: Seq[String], sqlState: Option[String]) {
+private[spark] case class ErrorInfo(message: Seq[String],
+ subClass: Option[Map[String, ErrorSubInfo]],
Review Comment:
Apache Spark uses 4-space indentation in this case, @panbingkun .
- https://github.com/databricks/scala-style-guide#indent
--
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 #36852: [SPARK-38700][SQL][3.3] Use error classes in the execution errors of save mode
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on PR #36852:
URL: https://github.com/apache/spark/pull/36852#issuecomment-1154740365
+1, LGTM. Merging to 3.3.
Thank you, @panbingkun and @dongjoon-hyun 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] dongjoon-hyun commented on a diff in pull request #36852: [SPARK-38700][SQL][3.3] Use error classes in the execution errors of save mode
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #36852:
URL: https://github.com/apache/spark/pull/36852#discussion_r895260896
##########
core/src/main/scala/org/apache/spark/ErrorInfo.scala:
##########
@@ -28,14 +28,30 @@ import com.fasterxml.jackson.module.scala.DefaultScalaModule
import org.apache.spark.util.Utils
+/**
+ * Information associated with an error subclass.
+ *
+ * @param subClass SubClass associated with this class.
+ * @param message C-style message format compatible with printf.
+ * The error message is constructed by concatenating the lines with newlines.
+ */
+private[spark] case class ErrorSubInfo(message: Seq[String]) {
+ // For compatibility with multi-line error messages
+ @JsonIgnore
+ val messageFormat: String = message.mkString("\n")
+}
+
/**
* Information associated with an error class.
*
* @param sqlState SQLSTATE associated with this class.
+ * @param subClass A sequence of subclasses
Review Comment:
`Option[Map[...]]` is not a sequence, isn't it?
--
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] dongjoon-hyun commented on a diff in pull request #36852: [SPARK-38700][SQL][3.3] Use error classes in the execution errors of save mode
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #36852:
URL: https://github.com/apache/spark/pull/36852#discussion_r895261028
##########
core/src/main/scala/org/apache/spark/ErrorInfo.scala:
##########
@@ -28,14 +28,30 @@ import com.fasterxml.jackson.module.scala.DefaultScalaModule
import org.apache.spark.util.Utils
+/**
+ * Information associated with an error subclass.
+ *
+ * @param subClass SubClass associated with this class.
Review Comment:
This is wrong because `ErrorSubInfo` doesn't have `subClass` parameter.
--
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] panbingkun commented on pull request #36852: [SPARK-38700][SQL][3.3] Use error classes in the execution errors of save mode
Posted by GitBox <gi...@apache.org>.
panbingkun commented on PR #36852:
URL: https://github.com/apache/spark/pull/36852#issuecomment-1153686788
> I guess we need @MaxGekk 's final sign-off.
OK，Thank you for review @dongjoon-hyun
ping @MaxGekk
--
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 #36852: [SPARK-38700][SQL][3.3] Use error classes in the execution errors of save mode
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on PR #36852:
URL: https://github.com/apache/spark/pull/36852#issuecomment-1153746752
@panbingkun Could you update PR's description and add "This is a backport of https://github.com/apache/spark/pull/36350".
--
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 #36852: [SPARK-38700][SQL][3.3] Use error classes in the execution errors of save mode
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on PR #36852:
URL: https://github.com/apache/spark/pull/36852#issuecomment-1154053041
@panbingkun Could you fix the test failure, please:
```
QueryExecutionErrorsSuite.UNSUPPORTED_SAVE_MODE: unsupported null saveMode whether the path exists or not
org.scalatest.exceptions.TestFailedException: "... supported for: a no[n-]existent path." did not equal "... supported for: a no[t ]existent
```
--
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] dongjoon-hyun commented on pull request #36852: [SPARK-38700][SQL][3.3] Use error classes in the execution errors of save mode
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on PR #36852:
URL: https://github.com/apache/spark/pull/36852#issuecomment-1153540713
Thank you for updates.
--
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 #36852: [SPARK-38700][SQL][3.3] Use error classes in the execution errors of save mode
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on code in PR #36852:
URL: https://github.com/apache/spark/pull/36852#discussion_r895568330
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -184,6 +184,17 @@
"UNSUPPORTED_OPERATION" : {
"message" : [ "The operation is not supported: <operation>" ]
},
+ "UNSUPPORTED_SAVE_MODE" : {
+ "message" : [ "The save mode <saveMode> is not supported for: " ],
+ "subClass" : {
+ "EXISTENT_PATH" : {
+ "message" : [ "an existent path." ]
+ },
+ "NON_EXISTENT_PATH" : {
+ "message" : [ "a not existent path." ]
Review Comment:
see master:
```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] panbingkun commented on pull request #36852: [SPARK-38700][SQL][3.3] Use error classes in the execution errors of save mode
Posted by GitBox <gi...@apache.org>.
panbingkun commented on PR #36852:
URL: https://github.com/apache/spark/pull/36852#issuecomment-1153175452
FYI old pr: https://github.com/apache/spark/pull/36676, i have closed it!
master branch pr: https://github.com/apache/spark/pull/36350
ping @MaxGekk
--
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] panbingkun commented on pull request #36852: [SPARK-38700][SQL][3.3] Use error classes in the execution errors of save mode
Posted by GitBox <gi...@apache.org>.
panbingkun commented on PR #36852:
URL: https://github.com/apache/spark/pull/36852#issuecomment-1154604668
> @panbingkun Could you fix the test failure, please:
>
> ```
> QueryExecutionErrorsSuite.UNSUPPORTED_SAVE_MODE: unsupported null saveMode whether the path exists or not
> org.scalatest.exceptions.TestFailedException: "... supported for: a no[n-]existent path." did not equal "... supported for: a no[t ]existent
> ```
Done @MaxGekk
--
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] panbingkun commented on pull request #36852: [SPARK-38700][SQL][3.3] Use error classes in the execution errors of save mode
Posted by GitBox <gi...@apache.org>.
panbingkun commented on PR #36852:
URL: https://github.com/apache/spark/pull/36852#issuecomment-1153793630
> @panbingkun Could you update PR's description and add "This is a backport of #36350".
sure！
--
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] dongjoon-hyun commented on pull request #36852: [SPARK-38700][SQL][3.3] Use error classes in the execution errors of save mode
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on PR #36852:
URL: https://github.com/apache/spark/pull/36852#issuecomment-1153663961
I guess we need @MaxGekk 's final sign-off.
--
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