You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "dengziming (via GitHub)" <gi...@apache.org> on 2023/09/21 07:39:30 UTC
[GitHub] [spark] dengziming opened a new pull request, #43029: [SPARK-45213][SQL] Assign name to the error _LEGACY_ERROR_TEMP_2151
dengziming opened a new pull request, #43029:
URL: https://github.com/apache/spark/pull/43029
### What changes were proposed in this pull request?
Assign the name `EXPRESSION_DECODING_FAILED` to the legacy error class `_LEGACY_ERROR_TEMP_2151`.
### Why are the changes needed?
To assign proper name as a part of activity in SPARK-37935.
### Does this PR introduce _any_ user-facing change?
Yes, the error message will include the error class name
### How was this patch tested?
An existing unit test to produce the error from user code.
### Was this patch authored or co-authored using generative AI tooling?
No.
--
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] dengziming commented on pull request #43029: [SPARK-45213][SQL] Assign name to the error _LEGACY_ERROR_TEMP_2151
Posted by "dengziming (via GitHub)" <gi...@apache.org>.
dengziming commented on PR #43029:
URL: https://github.com/apache/spark/pull/43029#issuecomment-1733330301
ping @MaxGekk to have a look.
--
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 #43029: [SPARK-45213][SQL] Assign name to the error _LEGACY_ERROR_TEMP_2151
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #43029:
URL: https://github.com/apache/spark/pull/43029#discussion_r1338083914
##########
common/utils/src/main/resources/error/error-classes.json:
##########
@@ -916,6 +916,12 @@
}
}
},
+ "EXPRESSION_DECODING_FAILED" : {
+ "message" : [
+ "Error while decoding: <e>",
Review Comment:
Let's not include the exception `<e>` to the error message because of:
1. The message is visible not only to Java/Scala users but also PySpark and SQL users
2. It is included as `cause` in `SparkRuntimeException`
--
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
Re: [PR] [SPARK-45213][SQL] Assign name to the error _LEGACY_ERROR_TEMP_2151 [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #43029:
URL: https://github.com/apache/spark/pull/43029#discussion_r1350302410
##########
common/utils/src/main/resources/error/error-classes.json:
##########
@@ -921,6 +921,11 @@
}
}
},
+ "EXPRESSION_DECODING_FAILED" : {
+ "message" : [
+ "Error while decoding: <expressions>."
Review Comment:
Maybe more precise:
```suggestion
"Failed to decode a row to a value of the expressions: <expressions>."
```
--
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] dengziming commented on a diff in pull request #43029: [SPARK-45213][SQL] Assign name to the error _LEGACY_ERROR_TEMP_2151
Posted by "dengziming (via GitHub)" <gi...@apache.org>.
dengziming commented on code in PR #43029:
URL: https://github.com/apache/spark/pull/43029#discussion_r1338172305
##########
common/utils/src/main/resources/error/error-classes.json:
##########
@@ -916,6 +916,12 @@
}
}
},
+ "EXPRESSION_DECODING_FAILED" : {
+ "message" : [
+ "Error while decoding: <e>",
Review Comment:
Good catch, I think the object of "Error while decoding" should be expression not the exception.
--
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
Re: [PR] [SPARK-45213][SQL] Assign name to the error _LEGACY_ERROR_TEMP_2151 [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk closed pull request #43029: [SPARK-45213][SQL] Assign name to the error _LEGACY_ERROR_TEMP_2151
URL: https://github.com/apache/spark/pull/43029
--
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
Re: [PR] [SPARK-45213][SQL] Assign name to the error _LEGACY_ERROR_TEMP_2151 [spark]
Posted by "dengziming (via GitHub)" <gi...@apache.org>.
dengziming commented on PR #43029:
URL: https://github.com/apache/spark/pull/43029#issuecomment-1755322337
Thanks @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
Re: [PR] [SPARK-45213][SQL] Assign name to the error _LEGACY_ERROR_TEMP_2151 [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #43029:
URL: https://github.com/apache/spark/pull/43029#issuecomment-1755315299
The docker test should not be related to the last commit.
+1, LGTM. Merging to master.
Thank you, @dengziming.
--
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
Re: [PR] [SPARK-45213][SQL] Assign name to the error _LEGACY_ERROR_TEMP_2151 [spark]
Posted by "dengziming (via GitHub)" <gi...@apache.org>.
dengziming commented on PR #43029:
URL: https://github.com/apache/spark/pull/43029#issuecomment-1752265044
Hi @MaxGekk , sorry for my late, I have fixed all the test failures, PTAL again.
--
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