Skip to content

Commit 625afb4

Browse files
committed
[SPARK-39255][SQL] Improve error messages
### What changes were proposed in this pull request? In the PR, I propose to improve errors of the following error classes: 1. NON_PARTITION_COLUMN - `a non-partition column name` -> `the non-partition column` 2. UNSUPPORTED_SAVE_MODE - `a not existent path` -> `a non existent path`. 3. INVALID_FIELD_NAME. Quote ids to follow the rules #36621. 4. FAILED_SET_ORIGINAL_PERMISSION_BACK. It is renamed to FAILED_PERMISSION_RESET_ORIGINAL. 5. NON_LITERAL_PIVOT_VALUES - Wrap error's expression by double quotes. The PR adds new helper method `toSQLExpr()` for that. 6. CAST_INVALID_INPUT - Add the recommendation: `... Correct the syntax for the value before casting it, or change the type to one appropriate for the value.` ### Why are the changes needed? To improve user experience with Spark SQL by making error message more clear. ### Does this PR introduce _any_ user-facing change? Yes, it changes user-facing error messages. ### How was this patch tested? By running the affected test suites: ``` $ build/sbt "sql/testOnly org.apache.spark.sql.SQLQueryTestSuite" $ build/sbt "sql/testOnly *QueryCompilationErrorsDSv2Suite" $ build/sbt "sql/testOnly *QueryCompilationErrorsSuite" $ build/sbt "sql/testOnly *QueryExecutionAnsiErrorsSuite" $ build/sbt "sql/testOnly *QueryExecutionErrorsSuite" $ build/sbt "sql/testOnly *QueryParsingErrorsSuite*" ``` Closes #36635 from MaxGekk/error-class-improve-msg-3. Lead-authored-by: Max Gekk <max.gekk@gmail.com> Co-authored-by: Maxim Gekk <max.gekk@gmail.com> Signed-off-by: Max Gekk <max.gekk@gmail.com>
1 parent d9fd36e commit 625afb4

24 files changed

+119
-113
lines changed

core/src/main/resources/error/error-classes.json

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
"message" : [ "Cannot up cast <value> from <sourceType> to <targetType>.\n<details>" ]
2424
},
2525
"CAST_INVALID_INPUT" : {
26-
"message" : [ "The value <value> of the type <sourceType> cannot be cast to <targetType> because it is malformed. To return NULL instead, use `try_cast`. If necessary set <config> to \"false\" to bypass this error." ],
26+
"message" : [ "The value <value> of the type <sourceType> cannot be cast to <targetType> because it is malformed. Correct the value as per the syntax, or change its target type. To return NULL instead, use `try_cast`. If necessary set <config> to \"false\" to bypass this error." ],
2727
"sqlState" : "42000"
2828
},
2929
"CAST_OVERFLOW" : {
@@ -52,9 +52,6 @@
5252
"message" : [ "Failed to rename <sourcePath> to <targetPath> as destination already exists" ],
5353
"sqlState" : "22023"
5454
},
55-
"FAILED_SET_ORIGINAL_PERMISSION_BACK" : {
56-
"message" : [ "Failed to set original permission <permission> back to the created path: <path>. Exception: <message>" ]
57-
},
5855
"FORBIDDEN_OPERATION" : {
5956
"message" : [ "The operation <statement> is not allowed on <objectType>: <objectName>" ]
6057
},
@@ -164,11 +161,11 @@
164161
"message" : [ "more than one row returned by a subquery used as an expression: <plan>" ]
165162
},
166163
"NON_LITERAL_PIVOT_VALUES" : {
167-
"message" : [ "Literal expressions required for pivot values, found '<expression>'" ],
164+
"message" : [ "Literal expressions required for pivot values, found <expression>." ],
168165
"sqlState" : "42000"
169166
},
170167
"NON_PARTITION_COLUMN" : {
171-
"message" : [ "PARTITION clause cannot contain a non-partition column name: <columnName>" ],
168+
"message" : [ "PARTITION clause cannot contain the non-partition column: <columnName>." ],
172169
"sqlState" : "42000"
173170
},
174171
"NO_HANDLER_FOR_UDAF" : {
@@ -197,6 +194,9 @@
197194
"message" : [ "Failed to rename as <sourcePath> was not found" ],
198195
"sqlState" : "22023"
199196
},
197+
"RESET_PERMISSION_TO_ORIGINAL" : {
198+
"message" : [ "Failed to set original permission <permission> back to the created path: <path>. Exception: <message>" ]
199+
},
200200
"SECOND_FUNCTION_ARGUMENT_NOT_INTEGER" : {
201201
"message" : [ "The second argument of '<functionName>' function needs to be an integer." ],
202202
"sqlState" : "22023"
@@ -322,7 +322,7 @@
322322
"message" : [ "an existent path." ]
323323
},
324324
"NON_EXISTENT_PATH" : {
325-
"message" : [ "a not existent path." ]
325+
"message" : [ "a non-existent path." ]
326326
}
327327
}
328328
},

sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ object QueryCompilationErrors extends QueryErrorsBase {
8282
def nonLiteralPivotValError(pivotVal: Expression): Throwable = {
8383
new AnalysisException(
8484
errorClass = "NON_LITERAL_PIVOT_VALUES",
85-
messageParameters = Array(pivotVal.toString))
85+
messageParameters = Array(toSQLExpr(pivotVal)))
8686
}
8787

8888
def pivotValDataTypeMismatchError(pivotVal: Expression, pivotCol: Expression): Throwable = {
@@ -2364,7 +2364,7 @@ object QueryCompilationErrors extends QueryErrorsBase {
23642364
def invalidFieldName(fieldName: Seq[String], path: Seq[String], context: Origin): Throwable = {
23652365
new AnalysisException(
23662366
errorClass = "INVALID_FIELD_NAME",
2367-
messageParameters = Array(fieldName.quoted, path.quoted),
2367+
messageParameters = Array(toSQLId(fieldName), toSQLId(path)),
23682368
origin = context)
23692369
}
23702370

sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryErrorsBase.scala

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ import org.apache.spark.sql.types.{DataType, DoubleType, FloatType}
3939
* For example: "spark.sql.ansi.enabled".
4040
* 6. Any values of datasource options or SQL configs shall be double quoted.
4141
* For example: "true", "CORRECTED".
42+
* 7. SQL expressions shall be wrapped by double quotes.
43+
* For example: "earnings + 1".
4244
*/
4345
trait QueryErrorsBase {
4446
// Converts an error class parameter to its SQL representation

sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryExecutionErrors.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1659,7 +1659,7 @@ object QueryExecutionErrors extends QueryErrorsBase {
16591659
permission: FsPermission,
16601660
path: Path,
16611661
e: Throwable): Throwable = {
1662-
new SparkSecurityException(errorClass = "FAILED_SET_ORIGINAL_PERMISSION_BACK",
1662+
new SparkSecurityException(errorClass = "RESET_PERMISSION_TO_ORIGINAL",
16631663
Array(permission.toString, path.toString, e.getMessage))
16641664
}
16651665

sql/catalyst/src/test/scala/org/apache/spark/sql/types/StructTypeSuite.scala

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,8 @@ class StructTypeSuite extends SparkFunSuite with SQLHelper {
321321
var e = intercept[AnalysisException] {
322322
check(Seq("S1", "S12", "S123"), None)
323323
}
324-
assert(e.getMessage.contains("Field name S1.S12.S123 is invalid: s1.s12 is not a struct"))
324+
assert(e.getMessage.contains(
325+
"Field name `S1`.`S12`.`S123` is invalid: `s1`.`s12` is not a struct"))
325326

326327
// ambiguous name
327328
e = intercept[AnalysisException] {
@@ -335,17 +336,19 @@ class StructTypeSuite extends SparkFunSuite with SQLHelper {
335336
e = intercept[AnalysisException] {
336337
check(Seq("m1", "key"), None)
337338
}
338-
assert(e.getMessage.contains("Field name m1.key is invalid: m1 is not a struct"))
339+
assert(e.getMessage.contains("Field name `m1`.`key` is invalid: `m1` is not a struct"))
339340
checkCollection(Seq("m1", "key"), Some(Seq("m1") -> StructField("key", IntegerType, false)))
340341
checkCollection(Seq("M1", "value"), Some(Seq("m1") -> StructField("value", IntegerType)))
341342
e = intercept[AnalysisException] {
342343
checkCollection(Seq("M1", "key", "name"), None)
343344
}
344-
assert(e.getMessage.contains("Field name M1.key.name is invalid: m1.key is not a struct"))
345+
assert(e.getMessage.contains(
346+
"Field name `M1`.`key`.`name` is invalid: `m1`.`key` is not a struct"))
345347
e = intercept[AnalysisException] {
346348
checkCollection(Seq("M1", "value", "name"), None)
347349
}
348-
assert(e.getMessage.contains("Field name M1.value.name is invalid: m1.value is not a struct"))
350+
assert(e.getMessage.contains(
351+
"Field name `M1`.`value`.`name` is invalid: `m1`.`value` is not a struct"))
349352

350353
// map of struct
351354
checkCollection(Seq("M2", "key", "A"),
@@ -357,24 +360,25 @@ class StructTypeSuite extends SparkFunSuite with SQLHelper {
357360
e = intercept[AnalysisException] {
358361
checkCollection(Seq("m2", "key", "A", "name"), None)
359362
}
360-
assert(e.getMessage.contains("Field name m2.key.A.name is invalid: m2.key.a is not a struct"))
363+
assert(e.getMessage.contains(
364+
"Field name `m2`.`key`.`A`.`name` is invalid: `m2`.`key`.`a` is not a struct"))
361365
e = intercept[AnalysisException] {
362366
checkCollection(Seq("M2", "value", "b", "name"), None)
363367
}
364368
assert(e.getMessage.contains(
365-
"Field name M2.value.b.name is invalid: m2.value.b is not a struct"))
369+
"Field name `M2`.`value`.`b`.`name` is invalid: `m2`.`value`.`b` is not a struct"))
366370

367371
// simple array type
368372
e = intercept[AnalysisException] {
369373
check(Seq("A1", "element"), None)
370374
}
371-
assert(e.getMessage.contains("Field name A1.element is invalid: a1 is not a struct"))
375+
assert(e.getMessage.contains("Field name `A1`.`element` is invalid: `a1` is not a struct"))
372376
checkCollection(Seq("A1", "element"), Some(Seq("a1") -> StructField("element", IntegerType)))
373377
e = intercept[AnalysisException] {
374378
checkCollection(Seq("A1", "element", "name"), None)
375379
}
376380
assert(e.getMessage.contains(
377-
"Field name A1.element.name is invalid: a1.element is not a struct"))
381+
"Field name `A1`.`element`.`name` is invalid: `a1`.`element` is not a struct"))
378382

379383
// array of struct
380384
checkCollection(Seq("A2", "element", "C"),
@@ -384,7 +388,7 @@ class StructTypeSuite extends SparkFunSuite with SQLHelper {
384388
checkCollection(Seq("a2", "element", "C", "name"), None)
385389
}
386390
assert(e.getMessage.contains(
387-
"Field name a2.element.C.name is invalid: a2.element.c is not a struct"))
391+
"Field name `a2`.`element`.`C`.`name` is invalid: `a2`.`element`.`c` is not a struct"))
388392
}
389393

390394
test("SPARK-36807: Merge ANSI interval types to a tightest common type") {

0 commit comments

Comments
 (0)