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/11/23 13:55:39 UTC
[GitHub] [spark] itholic opened a new pull request, #38772: [SPARK-41237][SQL] Assign a name to the error class `_LEGACY_ERROR_TEMP_0030`
itholic opened a new pull request, #38772:
URL: https://github.com/apache/spark/pull/38772
### What changes were proposed in this pull request?
The PR proposes to assign a name `DATA_TYPE_NOT_SUPPORTED ` to the error class `_LEGACY_ERROR_TEMP_0030`.
### Why are the changes needed?
We should assign proper name to `_LEGACY_ERROR_TEMP_*` error classes.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
```
./build/sbt “sql/testOnly org.apache.spark.sql.SQLQueryTestSuite*”
```
--
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 #38772: [SPARK-41237][SQL] Assign a name to the error class `_LEGACY_ERROR_TEMP_0030`
Posted by GitBox <gi...@apache.org>.
itholic commented on PR #38772:
URL: https://github.com/apache/spark/pull/38772#issuecomment-1325972686
> Can't you re-use the existing error class `UNSUPPORTED_DATATYPE`?
Oh... I missed that one. Sounds good!
--
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 #38772: [SPARK-41237][SQL] Reuse the error class `UNSUPPORTED_DATATYPE` for `_LEGACY_ERROR_TEMP_0030`
Posted by GitBox <gi...@apache.org>.
MaxGekk closed pull request #38772: [SPARK-41237][SQL] Reuse the error class `UNSUPPORTED_DATATYPE` for `_LEGACY_ERROR_TEMP_0030`
URL: https://github.com/apache/spark/pull/38772
--
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 #38772: [SPARK-41237][SQL] Assign a name to the error class `_LEGACY_ERROR_TEMP_0030`
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on PR #38772:
URL: https://github.com/apache/spark/pull/38772#issuecomment-1333262758
+1, LGTM. Merging to master.
Thank you, @itholic.
--
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 #38772: [SPARK-41237][SQL] Assign a name to the error class `_LEGACY_ERROR_TEMP_0030`
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on PR #38772:
URL: https://github.com/apache/spark/pull/38772#issuecomment-1328770649
@itholic Could you fix test failures. Seems like they are related to your changes:
```
DataTypeParserSuite.Do not print empty parentheses for no params
org.scalatest.exceptions.TestFailedException: "
[UNSUPPORTED_DATATYPE] Unsupported data type unknown(line 1, pos 0)
== SQL ==
unknown
^^^
" did not contain "unknown is not supported"
```
--
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 #38772: [SPARK-41237][SQL] Assign a name to the error class `_LEGACY_ERROR_TEMP_0030`
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on PR #38772:
URL: https://github.com/apache/spark/pull/38772#issuecomment-1325981880
> Maybe do we want to consolidate them in one rule ?
I agree. Let's use consolidate. I would prefer `DATATYPE` since it is shorter ;-)
--
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 #38772: [SPARK-41237][SQL] Assign a name to the error class `_LEGACY_ERROR_TEMP_0030`
Posted by GitBox <gi...@apache.org>.
itholic commented on PR #38772:
URL: https://github.com/apache/spark/pull/38772#issuecomment-1325973768
Btw, we use both `DATA_TYPE` and `DATATYPE` for error class name.
Maybe do we want to consolidate them in one rule ?
--
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 #38772: [SPARK-41237][SQL] Assign a name to the error class `_LEGACY_ERROR_TEMP_0030`
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on code in PR #38772:
URL: https://github.com/apache/spark/pull/38772#discussion_r1035011450
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryParsingErrors.scala:
##########
@@ -292,8 +292,8 @@ private[sql] object QueryParsingErrors extends QueryErrorsBase {
def dataTypeUnsupportedError(dataType: String, ctx: PrimitiveDataTypeContext): Throwable = {
new ParseException(
- errorClass = "_LEGACY_ERROR_TEMP_0030",
- messageParameters = Map("dataType" -> dataType),
+ errorClass = "UNSUPPORTED_DATATYPE",
+ messageParameters = Map("typeName" -> dataType),
Review Comment:
Need to quote the type even if it is incorrect. Please, call `toSQLType`.
--
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