You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "MaxGekk (via GitHub)" <gi...@apache.org> on 2023/08/30 11:08:16 UTC
[GitHub] [spark] MaxGekk opened a new pull request, #42737: [SPARK-44987][SQL] Assign a name to the error class `_LEGACY_ERROR_TEMP_1100`
MaxGekk opened a new pull request, #42737:
URL: https://github.com/apache/spark/pull/42737
### What changes were proposed in this pull request?
In the PR, I propose to assign the name `NON_FOLDABLE_ARGUMENT` to the legacy error class `_LEGACY_ERROR_TEMP_1100`, and improve the error message format: make it less restrictive.
### Why are the changes needed?
1. To don't confuse users by slightly restrictive error message about literals.
2. To assign proper name as a part of activity in SPARK-37935
### Does this PR introduce _any_ user-facing change?
No. Only if user's code depends on error class name and message parameters.
### How was this patch tested?
By running the modified and affected tests:
```
$ build/sbt "test:testOnly *.StringFunctionsSuite"
$ PYSPARK_PYTHON=python3 build/sbt "sql/testOnly org.apache.spark.sql.SQLQueryTestSuite"
$ build/sbt "core/testOnly *SparkThrowableSuite"
```
### 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] Hisoka-X commented on a diff in pull request #42737: [SPARK-44987][SQL] Assign a name to the error class `_LEGACY_ERROR_TEMP_1100`
Posted by "Hisoka-X (via GitHub)" <gi...@apache.org>.
Hisoka-X commented on code in PR #42737:
URL: https://github.com/apache/spark/pull/42737#discussion_r1310155968
##########
common/utils/src/main/resources/error/error-classes.json:
##########
@@ -2210,6 +2210,12 @@
],
"sqlState" : "42607"
},
+ "NON_FOLDABLE_ARGUMENT" : {
+ "message" : [
+ "The function <funcName> requires the parameter <paramName> to be a foldable expression of the type <paramType>, but the actual argument is a non-foldable."
Review Comment:
After thinking about it, I really didn't find a better way to express it.😂
##########
common/utils/src/main/resources/error/error-classes.json:
##########
@@ -2210,6 +2210,12 @@
],
"sqlState" : "42607"
},
+ "NON_FOLDABLE_ARGUMENT" : {
+ "message" : [
+ "The function <funcName> requires the parameter <paramName> to be a foldable expression of the type <paramType>, but the actual argument is a non-foldable."
Review Comment:
After thinking about it, I really didn't find a better way to express 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] MaxGekk commented on pull request #42737: [SPARK-44987][SQL] Assign a name to the error class `_LEGACY_ERROR_TEMP_1100`
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #42737:
URL: https://github.com/apache/spark/pull/42737#issuecomment-1701546740
@dongjoon-hyun @HyukjinKwon Could you review this PR, 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] dongjoon-hyun commented on a diff in pull request #42737: [SPARK-44987][SQL] Assign a name to the error class `_LEGACY_ERROR_TEMP_1100`
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #42737:
URL: https://github.com/apache/spark/pull/42737#discussion_r1312041137
##########
common/utils/src/main/resources/error/error-classes.json:
##########
@@ -2210,6 +2210,12 @@
],
"sqlState" : "42607"
},
+ "NON_FOLDABLE_ARGUMENT" : {
+ "message" : [
+ "The function <funcName> requires the parameter <paramName> to be a foldable expression of the type <paramType>, but the actual argument is a non-foldable."
Review Comment:
Ya, in this case, new message looks reasonable to me. :)
--
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 #42737: [SPARK-44987][SQL] Assign a name to the error class `_LEGACY_ERROR_TEMP_1100`
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #42737:
URL: https://github.com/apache/spark/pull/42737#discussion_r1312042732
##########
docs/sql-error-conditions.md:
##########
@@ -1299,6 +1299,12 @@ Cannot call function `<functionName>` because named argument references are not
It is not allowed to use an aggregate function in the argument of another aggregate function. Please use the inner aggregate function in a sub-query.
+### NON_FOLDABLE_ARGUMENT
+
+[SQLSTATE: 22024](sql-error-conditions-sqlstates.html#class-22-data-exception)
Review Comment:
Data exception category is also reasonable to me if there is no other proper one.
--
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 #42737: [SPARK-44987][SQL] Assign a name to the error class `_LEGACY_ERROR_TEMP_1100`
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #42737:
URL: https://github.com/apache/spark/pull/42737#issuecomment-1701691007
Merging to master. Thank you, @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] Hisoka-X commented on a diff in pull request #42737: [SPARK-44987][SQL] Assign a name to the error class `_LEGACY_ERROR_TEMP_1100`
Posted by "Hisoka-X (via GitHub)" <gi...@apache.org>.
Hisoka-X commented on code in PR #42737:
URL: https://github.com/apache/spark/pull/42737#discussion_r1310125178
##########
common/utils/src/main/resources/error/error-classes.json:
##########
@@ -2210,6 +2210,12 @@
],
"sqlState" : "42607"
},
+ "NON_FOLDABLE_ARGUMENT" : {
+ "message" : [
+ "The function <funcName> requires the parameter <paramName> to be a foldable expression of the type <paramType>, but the actual argument is a non-foldable."
Review Comment:
I'm a bit concerned, does the user know what is foldable and what is non-foldable?
--
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 #42737: [SPARK-44987][SQL] Assign a name to the error class `_LEGACY_ERROR_TEMP_1100`
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #42737:
URL: https://github.com/apache/spark/pull/42737#discussion_r1310134304
##########
common/utils/src/main/resources/error/error-classes.json:
##########
@@ -2210,6 +2210,12 @@
],
"sqlState" : "42607"
},
+ "NON_FOLDABLE_ARGUMENT" : {
+ "message" : [
+ "The function <funcName> requires the parameter <paramName> to be a foldable expression of the type <paramType>, but the actual argument is a non-foldable."
Review Comment:
How would you improve the error message?
--
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 #42737: [SPARK-44987][SQL] Assign a name to the error class `_LEGACY_ERROR_TEMP_1100`
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk closed pull request #42737: [SPARK-44987][SQL] Assign a name to the error class `_LEGACY_ERROR_TEMP_1100`
URL: https://github.com/apache/spark/pull/42737
--
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 #42737: [SPARK-44987][SQL] Assign a name to the error class `_LEGACY_ERROR_TEMP_1100`
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #42737:
URL: https://github.com/apache/spark/pull/42737#issuecomment-1701549297
Sure, @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