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 2023/01/17 11:14:13 UTC
[GitHub] [spark] itholic opened a new pull request, #39625: [WIP][SPARK-42066][SQL] The DATATYPE_MISMATCH error class contains inappropriate and duplicating subclasses
itholic opened a new pull request, #39625:
URL: https://github.com/apache/spark/pull/39625
### What changes were proposed in this pull request?
This PR proposes to remove `DATATYPE_MISMATCH.WRONG_NUM_ARGS` and `DATATYPE_MISMATCH.WRONG_NUM_ARGS_WITH_SUGGESTION`from sub-class of `DATATYPE_MISMATCH`, and integrate into `WRONG_NUM_ARGS.WITH_SUGGESTION` and `WRONG_NUM_ARGS.WITHOUT_SUGGESTION`.
### Why are the changes needed?
Because `WRONG_NUM_ARGS` is logically not a part of `DATATYPE_MISMATCH` error, and duplicated.
We should use appropriate name for all error classes.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
The existing UT is fixed, so CI should pass.
--
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] itholic commented on pull request #39625: [SPARK-42066][SQL] The DATATYPE_MISMATCH error class contains inappropriate and duplicating subclasses
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on PR #39625:
URL: https://github.com/apache/spark/pull/39625#issuecomment-1407893985
Gentle reminder for @cloud-fan
--
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] itholic commented on a diff in pull request #39625: [SPARK-42066][SQL] The DATATYPE_MISMATCH error class contains inappropriate and duplicating subclasses
Posted by GitBox <gi...@apache.org>.
itholic commented on code in PR #39625:
URL: https://github.com/apache/spark/pull/39625#discussion_r1080824796
##########
sql/core/src/test/resources/sql-tests/results/ansi/string-functions.sql.out:
##########
@@ -6,21 +6,13 @@ struct<>
-- !query output
org.apache.spark.sql.AnalysisException
{
- "errorClass" : "DATATYPE_MISMATCH.WRONG_NUM_ARGS",
- "sqlState" : "42K09",
+ "errorClass" : "WRONG_NUM_ARGS.WITHOUT_SUGGESTION",
+ "sqlState" : "42605",
"messageParameters" : {
"actualNum" : "0",
"expectedNum" : "> 0",
- "functionName" : "`concat_ws`",
- "sqlExpr" : "\"concat_ws()\""
- },
- "queryContext" : [ {
Review Comment:
To be honest, it is hard to figure out where these inconsistency behaviors come from.
I used exactly the same errors, `wrongNumArgsError`, for all the cases but sometimes it lose query context and sometimes not.
e.g.
`select decode()` keeps query context in its error message, but `select concat_ws()` not.
But I did throw same error for these two cases....
Btw, do we want to necessarily expose the `queryContext` to user space?
IMHO, it seems not that really helpful information for end-users, rather sounds a bit confusing 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] LuciferYang commented on a diff in pull request #39625: [SPARK-42066][SQL] The DATATYPE_MISMATCH error class contains inappropriate and duplicating subclasses
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on code in PR #39625:
URL: https://github.com/apache/spark/pull/39625#discussion_r1073058711
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala:
##########
@@ -657,29 +657,38 @@ private[sql] object QueryCompilationErrors extends QueryErrorsBase {
origin = context)
}
- def invalidFunctionArgumentsError(
- name: String, expectedNum: String, actualNum: Int): Throwable = {
- new AnalysisException(
- errorClass = "WRONG_NUM_ARGS.WITH_SUGGESTION",
- messageParameters = Map(
- "functionName" -> toSQLId(name),
- "expectedNum" -> expectedNum,
- "actualNum" -> actualNum.toString))
- }
-
- def invalidFunctionArgumentNumberError(
- validParametersCount: Seq[Int], name: String, actualNumber: Int): Throwable = {
- if (validParametersCount.isEmpty) {
+ def wrongNumArgsError(
+ name: String,
+ validParametersCount: Seq[Any],
+ actualNumber: Int,
+ legacyNum: String = "",
+ legacyConfKey: String = "",
+ legacyConfValue: String = ""): Throwable = {
+ val expectedNumberOfParameters = if (validParametersCount.isEmpty) {
+ "0"
+ } else if (validParametersCount.length == 1) {
+ validParametersCount.head.toString
+ } else {
+ validParametersCount.mkString("[", ", ", "]")
+ }
+ if (legacyNum.isEmpty) {
new AnalysisException(
errorClass = "WRONG_NUM_ARGS.WITHOUT_SUGGESTION",
- messageParameters = Map("functionName" -> toSQLId(name)))
+ messageParameters = Map(
+ "functionName" -> toSQLId(name),
+ "expectedNum" -> expectedNumberOfParameters,
+ "actualNum" -> actualNumber.toString))
} else {
- val expectedNumberOfParameters = if (validParametersCount.length == 1) {
- validParametersCount.head.toString
- } else {
- validParametersCount.mkString("[", ", ", "]")
- }
- invalidFunctionArgumentsError(name, expectedNumberOfParameters, actualNumber)
+ new AnalysisException(
+ errorClass = "WRONG_NUM_ARGS.WITH_SUGGESTION",
+ messageParameters = Map(
+ "functionName" -> toSQLId(name),
+ "expectedNum" -> expectedNumberOfParameters,
+ "actualNum" -> actualNumber.toString,
+ "legacyNum" -> legacyNum.toString,
Review Comment:
`legacyNum`, `legacyConfKey`, `legacyConfValue` called redundant `toString`
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -1719,7 +1708,7 @@
},
"WRONG_NUM_ARGS" : {
"message" : [
- "Invalid number of arguments for the function <functionName>."
+ "The <functionName> requires <expectedNum> parameters but the actual number is <actualNum>."
Review Comment:
`DATATYPE_MISMATCH` will print
```
"Cannot resolve <sqlExpr> due to data type mismatch:"
```
Is it unnecessary here?
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -1719,7 +1708,7 @@
},
"WRONG_NUM_ARGS" : {
"message" : [
- "Invalid number of arguments for the function <functionName>."
+ "The <functionName> requires <expectedNum> parameters but the actual number is <actualNum>."
],
"subClass" : {
"WITHOUT_SUGGESTION" : {
Review Comment:
I think the error class name `WRONG_ NUM_ ARGS.WITHOUT_ SUGGESTION` is not very accurate after this pr because `expectedNum` and `actualNum` have been pointed out in the 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] srielau commented on a diff in pull request #39625: [SPARK-42066][SQL] The DATATYPE_MISMATCH error class contains inappropriate and duplicating subclasses
Posted by GitBox <gi...@apache.org>.
srielau commented on code in PR #39625:
URL: https://github.com/apache/spark/pull/39625#discussion_r1072499199
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -1729,7 +1718,8 @@
},
"WITH_SUGGESTION" : {
"message" : [
- "Consider to change the number of arguments because the function requires <expectedNum> parameters but the actual number is <actualNum>."
+ "The <functionName> requires <expectedNum> parameters but the actual number is <actualNum>.",
+ "If you have to call this function with <legacyNum> parameters, set the legacy configuration <legacyConfKey> to <legacyConfValue>."
Review Comment:
You are repeating the general error class 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] itholic commented on a diff in pull request #39625: [SPARK-42066][SQL] The DATATYPE_MISMATCH error class contains inappropriate and duplicating subclasses
Posted by GitBox <gi...@apache.org>.
itholic commented on code in PR #39625:
URL: https://github.com/apache/spark/pull/39625#discussion_r1073140160
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -1719,7 +1708,7 @@
},
"WRONG_NUM_ARGS" : {
"message" : [
- "Invalid number of arguments for the function <functionName>."
+ "The <functionName> requires <expectedNum> parameters but the actual number is <actualNum>."
Review Comment:
I think `WRONG_NUM_ARGS` is not a sub-class of `DATATYPE_MISMATCH`, so maybe we don't need to follow the existing message from `DATATYPE_MISMATCH`.
Maybe
```
"The <functionName> requires <expectedNum> parameters but the actual number is <actualNum>."
```
alone also can helpful for user to fix the problem ??
WDYT, @cloud-fan @srielau @MaxGekk ?
Do we want to show the `<sqlExpr>` in the error message as well??
--
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] cloud-fan commented on a diff in pull request #39625: [SPARK-42066][SQL] The DATATYPE_MISMATCH error class contains inappropriate and duplicating subclasses
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #39625:
URL: https://github.com/apache/spark/pull/39625#discussion_r1090465030
##########
sql/core/src/test/resources/sql-tests/results/ansi/string-functions.sql.out:
##########
@@ -6,21 +6,13 @@ struct<>
-- !query output
org.apache.spark.sql.AnalysisException
{
- "errorClass" : "DATATYPE_MISMATCH.WRONG_NUM_ARGS",
- "sqlState" : "42K09",
+ "errorClass" : "WRONG_NUM_ARGS.WITHOUT_SUGGESTION",
+ "sqlState" : "42605",
"messageParameters" : {
"actualNum" : "0",
"expectedNum" : "> 0",
- "functionName" : "`concat_ws`",
- "sqlExpr" : "\"concat_ws()\""
- },
- "queryContext" : [ {
Review Comment:
@MaxGekk can you help to take a look? BTW it does not block this PR.
--
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] cloud-fan closed pull request #39625: [SPARK-42066][SQL] The DATATYPE_MISMATCH error class contains inappropriate and duplicating subclasses
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan closed pull request #39625: [SPARK-42066][SQL] The DATATYPE_MISMATCH error class contains inappropriate and duplicating subclasses
URL: https://github.com/apache/spark/pull/39625
--
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] LuciferYang commented on a diff in pull request #39625: [SPARK-42066][SQL] The DATATYPE_MISMATCH error class contains inappropriate and duplicating subclasses
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on code in PR #39625:
URL: https://github.com/apache/spark/pull/39625#discussion_r1073161990
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -1719,7 +1708,7 @@
},
"WRONG_NUM_ARGS" : {
"message" : [
- "Invalid number of arguments for the function <functionName>."
+ "The <functionName> requires <expectedNum> parameters but the actual number is <actualNum>."
Review Comment:
fine 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] itholic commented on a diff in pull request #39625: [SPARK-42066][SQL] The DATATYPE_MISMATCH error class contains inappropriate and duplicating subclasses
Posted by GitBox <gi...@apache.org>.
itholic commented on code in PR #39625:
URL: https://github.com/apache/spark/pull/39625#discussion_r1073135508
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -1719,7 +1708,7 @@
},
"WRONG_NUM_ARGS" : {
"message" : [
- "Invalid number of arguments for the function <functionName>."
+ "The <functionName> requires <expectedNum> parameters but the actual number is <actualNum>."
],
"subClass" : {
"WITHOUT_SUGGESTION" : {
Review Comment:
Honestly, I think `expectedNum` and `actualNum` are sort of basic information rather than a suggestion.
That's why I put them into main error class instead of sub error classes.
--
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] itholic commented on a diff in pull request #39625: [SPARK-42066][SQL] The DATATYPE_MISMATCH error class contains inappropriate and duplicating subclasses
Posted by GitBox <gi...@apache.org>.
itholic commented on code in PR #39625:
URL: https://github.com/apache/spark/pull/39625#discussion_r1080824796
##########
sql/core/src/test/resources/sql-tests/results/ansi/string-functions.sql.out:
##########
@@ -6,21 +6,13 @@ struct<>
-- !query output
org.apache.spark.sql.AnalysisException
{
- "errorClass" : "DATATYPE_MISMATCH.WRONG_NUM_ARGS",
- "sqlState" : "42K09",
+ "errorClass" : "WRONG_NUM_ARGS.WITHOUT_SUGGESTION",
+ "sqlState" : "42605",
"messageParameters" : {
"actualNum" : "0",
"expectedNum" : "> 0",
- "functionName" : "`concat_ws`",
- "sqlExpr" : "\"concat_ws()\""
- },
- "queryContext" : [ {
Review Comment:
To be honest, it is hard to figure out where these inconsistency behaviors come from.
I used exactly the same errors, `wrongNumArgsError`, for all the cases but sometimes it lose query context and sometimes not.
e.g.
`select decode()` keeps query context in its error message, but `select concat_ws()` not.
But I did throw same error for these two cases....
Btw, do we want to necessarily expose the `queryContext` to user space?
IMHO, it seems not that really helpful information for end-users, rather sounds a bit confusing to me.
Maybe I could dig into the root cause for inconsistency, but I think it's a bit time consuming compared to the benefits we get ?
--
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] itholic commented on a diff in pull request #39625: [SPARK-42066][SQL] The DATATYPE_MISMATCH error class contains inappropriate and duplicating subclasses
Posted by GitBox <gi...@apache.org>.
itholic commented on code in PR #39625:
URL: https://github.com/apache/spark/pull/39625#discussion_r1080771038
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala:
##########
@@ -657,29 +657,38 @@ private[sql] object QueryCompilationErrors extends QueryErrorsBase {
origin = context)
}
- def invalidFunctionArgumentsError(
- name: String, expectedNum: String, actualNum: Int): Throwable = {
- new AnalysisException(
- errorClass = "WRONG_NUM_ARGS.WITH_SUGGESTION",
- messageParameters = Map(
- "functionName" -> toSQLId(name),
- "expectedNum" -> expectedNum,
- "actualNum" -> actualNum.toString))
- }
-
- def invalidFunctionArgumentNumberError(
- validParametersCount: Seq[Int], name: String, actualNumber: Int): Throwable = {
- if (validParametersCount.isEmpty) {
+ def wrongNumArgsError(
+ name: String,
+ validParametersCount: Seq[Any],
+ actualNumber: Int,
+ legacyNum: String = "",
+ legacyConfKey: String = "",
+ legacyConfValue: String = ""): Throwable = {
+ val expectedNumberOfParameters = if (validParametersCount.isEmpty) {
+ "0"
+ } else if (validParametersCount.length == 1) {
+ validParametersCount.head.toString
+ } else {
+ validParametersCount.mkString("[", ", ", "]")
+ }
+ if (legacyNum.isEmpty) {
new AnalysisException(
errorClass = "WRONG_NUM_ARGS.WITHOUT_SUGGESTION",
- messageParameters = Map("functionName" -> toSQLId(name)))
+ messageParameters = Map(
+ "functionName" -> toSQLId(name),
+ "expectedNum" -> expectedNumberOfParameters,
+ "actualNum" -> actualNumber.toString))
} else {
- val expectedNumberOfParameters = if (validParametersCount.length == 1) {
- validParametersCount.head.toString
- } else {
- validParametersCount.mkString("[", ", ", "]")
- }
- invalidFunctionArgumentsError(name, expectedNumberOfParameters, actualNumber)
+ new AnalysisException(
+ errorClass = "WRONG_NUM_ARGS.WITH_SUGGESTION",
+ messageParameters = Map(
+ "functionName" -> toSQLId(name),
+ "expectedNum" -> expectedNumberOfParameters,
+ "actualNum" -> actualNumber.toString,
+ "legacyNum" -> legacyNum.toString,
Review Comment:
Thanks! Let me address them
--
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] cloud-fan commented on pull request #39625: [SPARK-42066][SQL] The DATATYPE_MISMATCH error class contains inappropriate and duplicating subclasses
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #39625:
URL: https://github.com/apache/spark/pull/39625#issuecomment-1408404617
thanks, merging to master/3.4!
--
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] itholic commented on a diff in pull request #39625: [SPARK-42066][SQL] The DATATYPE_MISMATCH error class contains inappropriate and duplicating subclasses
Posted by GitBox <gi...@apache.org>.
itholic commented on code in PR #39625:
URL: https://github.com/apache/spark/pull/39625#discussion_r1073016243
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -1729,7 +1718,8 @@
},
"WITH_SUGGESTION" : {
"message" : [
- "Consider to change the number of arguments because the function requires <expectedNum> parameters but the actual number is <actualNum>."
+ "The <functionName> requires <expectedNum> parameters but the actual number is <actualNum>.",
+ "If you have to call this function with <legacyNum> parameters, set the legacy configuration <legacyConfKey> to <legacyConfValue>."
Review Comment:
Thanks for the correction! Just fixed 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] itholic commented on a diff in pull request #39625: [SPARK-42066][SQL] The DATATYPE_MISMATCH error class contains inappropriate and duplicating subclasses
Posted by GitBox <gi...@apache.org>.
itholic commented on code in PR #39625:
URL: https://github.com/apache/spark/pull/39625#discussion_r1080824796
##########
sql/core/src/test/resources/sql-tests/results/ansi/string-functions.sql.out:
##########
@@ -6,21 +6,13 @@ struct<>
-- !query output
org.apache.spark.sql.AnalysisException
{
- "errorClass" : "DATATYPE_MISMATCH.WRONG_NUM_ARGS",
- "sqlState" : "42K09",
+ "errorClass" : "WRONG_NUM_ARGS.WITHOUT_SUGGESTION",
+ "sqlState" : "42605",
"messageParameters" : {
"actualNum" : "0",
"expectedNum" : "> 0",
- "functionName" : "`concat_ws`",
- "sqlExpr" : "\"concat_ws()\""
- },
- "queryContext" : [ {
Review Comment:
To be honest, it is hard to figure out where these inconsistency behaviors come from.
I used exactly the same errors, `wrongNumArgsError`, for all the cases but sometimes it lose query context and sometimes not.
e.g.
`select decode()` keeps query context in its error message, but `select concat_ws()` not.
But I did throw same error for these two cases....
Btw, do we want to necessarily expose the `queryContext` to user space?
IMHO, it seems not that really helpful information for end-users, rather sounds a bit confusing to me.
Maybe I could dig into the root cause for inconsistency, but I think it's a bit time consuming compared to the benefits we get.
--
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] LuciferYang commented on pull request #39625: [SPARK-42066][SQL] The DATATYPE_MISMATCH error class contains inappropriate and duplicating subclasses
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on PR #39625:
URL: https://github.com/apache/spark/pull/39625#issuecomment-1386586327
The failed case seems related to this pr:
```
2023-01-18T02:40:46.1537051Z [info] - SPARK-32243: Spark UDAF Invalid arguments number error should throw earlier *** FAILED *** (54 milliseconds)
2023-01-18T02:40:46.1543666Z [info] "WRONG_NUM_ARGS.WITH[OUT]_SUGGESTION" did not equal "WRONG_NUM_ARGS.WITH[]_SUGGESTION" (SparkFunSuite.scala:301)
2023-01-18T02:40:46.1548530Z [info] Analysis:
2023-01-18T02:40:46.1553256Z [info] "WRONG_NUM_ARGS.WITH[OUT]_SUGGESTION" -> "WRONG_NUM_ARGS.WITH[]_SUGGESTION"
2023-01-18T02:40:46.1558426Z [info] org.scalatest.exceptions.TestFailedException:
```
--
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] cloud-fan commented on a diff in pull request #39625: [SPARK-42066][SQL] The DATATYPE_MISMATCH error class contains inappropriate and duplicating subclasses
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on code in PR #39625:
URL: https://github.com/apache/spark/pull/39625#discussion_r1073099029
##########
sql/core/src/test/resources/sql-tests/results/ansi/string-functions.sql.out:
##########
@@ -6,21 +6,13 @@ struct<>
-- !query output
org.apache.spark.sql.AnalysisException
{
- "errorClass" : "DATATYPE_MISMATCH.WRONG_NUM_ARGS",
- "sqlState" : "42K09",
+ "errorClass" : "WRONG_NUM_ARGS.WITHOUT_SUGGESTION",
+ "sqlState" : "42605",
"messageParameters" : {
"actualNum" : "0",
"expectedNum" : "> 0",
- "functionName" : "`concat_ws`",
- "sqlExpr" : "\"concat_ws()\""
- },
- "queryContext" : [ {
Review Comment:
Do we lose the query context?
--
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] itholic commented on a diff in pull request #39625: [SPARK-42066][SQL] The DATATYPE_MISMATCH error class contains inappropriate and duplicating subclasses
Posted by GitBox <gi...@apache.org>.
itholic commented on code in PR #39625:
URL: https://github.com/apache/spark/pull/39625#discussion_r1073140160
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -1719,7 +1708,7 @@
},
"WRONG_NUM_ARGS" : {
"message" : [
- "Invalid number of arguments for the function <functionName>."
+ "The <functionName> requires <expectedNum> parameters but the actual number is <actualNum>."
Review Comment:
```
"Cannot resolve <sqlExpr> due to data type mismatch:"
```
I think `WRONG_NUM_ARGS` is not a sub-class of `DATATYPE_MISMATCH`, so we don't need to follow the existing message from `DATATYPE_MISMATCH`.
Maybe
```
"The <functionName> requires <expectedNum> parameters but the actual number is <actualNum>."
```
alone also can helpful for user to fix the problem ??
WDYT, @cloud-fan @srielau @MaxGekk ?
Do we want to show the `<sqlExpr>` in the error message as well??
--
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] cloud-fan commented on a diff in pull request #39625: [SPARK-42066][SQL] The DATATYPE_MISMATCH error class contains inappropriate and duplicating subclasses
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on code in PR #39625:
URL: https://github.com/apache/spark/pull/39625#discussion_r1073156619
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -1719,7 +1708,7 @@
},
"WRONG_NUM_ARGS" : {
"message" : [
- "Invalid number of arguments for the function <functionName>."
+ "The <functionName> requires <expectedNum> parameters but the actual number is <actualNum>."
Review Comment:
with function name provided, I think it's fine to not have `sqlExpr`
--
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