beliefer commented on code in PR #43438:
URL: https://github.com/apache/spark/pull/43438#discussion_r1365363954
##########
common/utils/src/main/scala/org/apache/spark/SparkThrowableHelper.scala:
##########
@@ -51,9 +51,11 @@ private[spark] object SparkThrowableHelper {
messageParameters: Map[String, String],
context: String): String = {
val displayMessage = errorReader.getErrorMessage(errorClass,
messageParameters)
+ val sqlState = errorReader.getSqlState(errorClass)
Review Comment:
```suggestion
val sqlState = getSqlState(errorClass)
```
##########
docs/sql-error-conditions.md:
##########
@@ -1170,6 +1170,12 @@ The input schema `<inputSchema>` is not a valid schema
string.
For more details see
[INVALID_SCHEMA](sql-error-conditions-invalid-schema-error-class.html)
+### INVALID_SCHEMA_OR_RELATION_NAME
+
+[SQLSTATE:
42602](sql-error-conditions-sqlstates.html#class-42-syntax-error-or-access-rule-violation)
+
+``<name>`` is not a valid name for tables/schemas. Valid names only contain
alphabet characters, numbers and _.
Review Comment:
ditto
##########
common/utils/src/main/scala/org/apache/spark/SparkThrowableHelper.scala:
##########
@@ -51,9 +51,11 @@ private[spark] object SparkThrowableHelper {
messageParameters: Map[String, String],
context: String): String = {
val displayMessage = errorReader.getErrorMessage(errorClass,
messageParameters)
+ val sqlState = errorReader.getSqlState(errorClass)
+ val displaySqlState = if (sqlState == null) "" else s" SQLSTATE: $sqlState"
Review Comment:
```suggestion
val displaySqlState = if (sqlState == null) "undefined" else s"
SQLSTATE: $sqlState"
```
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala:
##########
@@ -2767,8 +2767,8 @@ class AstBuilder extends DataTypeAstBuilder with
SQLConfHelper with Logging {
} catch {
case e: SparkArithmeticException =>
throw new ParseException(
- errorClass = "_LEGACY_ERROR_TEMP_0061",
- messageParameters = Map("msg" -> e.getMessage),
+ errorClass = e.getErrorClass,
+ messageParameters = e.getMessageParameters.asScala.toMap,
Review Comment:
Why change this?
##########
common/utils/src/main/scala/org/apache/spark/SparkThrowableHelper.scala:
##########
@@ -51,9 +51,11 @@ private[spark] object SparkThrowableHelper {
messageParameters: Map[String, String],
context: String): String = {
val displayMessage = errorReader.getErrorMessage(errorClass,
messageParameters)
+ val sqlState = errorReader.getSqlState(errorClass)
+ val displaySqlState = if (sqlState == null) "" else s" SQLSTATE: $sqlState"
val displayQueryContext = (if (context.isEmpty) "" else "\n") + context
val prefix = if (errorClass.startsWith("_LEGACY_ERROR_")) "" else
s"[$errorClass] "
- s"$prefix$displayMessage$displayQueryContext"
+ s"$prefix$displayMessage$displaySqlState$displayQueryContext"
Review Comment:
I think should put `displaySqlState` in front of `displayMessage`
##########
sql/core/src/test/resources/sql-tests/analyzer-results/ansi/literals.sql.out:
##########
@@ -427,9 +427,11 @@ select 1.20E-38BD
-- !query analysis
org.apache.spark.sql.catalyst.parser.ParseException
{
- "errorClass" : "_LEGACY_ERROR_TEMP_0061",
+ "errorClass" : "DECIMAL_PRECISION_EXCEEDS_MAX_PRECISION",
+ "sqlState" : "22003",
"messageParameters" : {
- "msg" : "[DECIMAL_PRECISION_EXCEEDS_MAX_PRECISION] Decimal precision 40
exceeds max precision 38."
Review Comment:
Personally, I feel the origin msg is better.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
