-
Notifications
You must be signed in to change notification settings - Fork 28.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPARK-31834][SQL]Improve error message for incompatible data types #28654
Conversation
ok to test. |
Test build #123216 has finished for PR 28654 at commit
|
retest this please |
Test build #123218 has finished for PR 28654 at commit
|
@@ -457,7 +457,7 @@ object DataType { | |||
|
|||
case (w: AtomicType, r: AtomicType) if storeAssignmentPolicy == STRICT => | |||
if (!Cast.canUpCast(w, r)) { | |||
addError(s"Cannot safely cast '$context': $w to $r") | |||
addError(s"Cannot safely cast '$context': ${w.simpleString} to ${r.simpleString}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: simpleString
-> catalogString
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for catalogString
:
spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
Lines 268 to 269 in 0fd98ab
TypeCheckResult.TypeCheckFailure( | |
s"cannot cast ${child.dataType.catalogString} to ${dataType.catalogString}") |
@@ -467,7 +467,7 @@ object DataType { | |||
|
|||
case (w: AtomicType, r: AtomicType) if storeAssignmentPolicy == ANSI => | |||
if (!Cast.canANSIStoreAssign(w, r)) { | |||
addError(s"Cannot safely cast '$context': $w to $r") | |||
addError(s"Cannot safely cast '$context': ${w.simpleString} to ${r.simpleString}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
Test build #123236 has finished for PR 28654 at commit
|
sql/catalyst/src/test/scala/org/apache/spark/sql/types/DataTypeWriteCompatibilitySuite.scala
Outdated
Show resolved
Hide resolved
sql/catalyst/src/test/scala/org/apache/spark/sql/types/DataTypeWriteCompatibilitySuite.scala
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM too
Test build #123415 has finished for PR 28654 at commit
|
retest this please |
Test build #123423 has finished for PR 28654 at commit
|
Merged to master and branch-3.0. |
### What changes were proposed in this pull request? We should use dataType.catalogString to unified the data type mismatch message. Before: ```sql spark-sql> create table SPARK_31834(a int) using parquet; spark-sql> insert into SPARK_31834 select '1'; Error in query: Cannot write incompatible data to table '`default`.`spark_31834`': - Cannot safely cast 'a': StringType to IntegerType; ``` After: ```sql spark-sql> create table SPARK_31834(a int) using parquet; spark-sql> insert into SPARK_31834 select '1'; Error in query: Cannot write incompatible data to table '`default`.`spark_31834`': - Cannot safely cast 'a': string to int; ``` ### How was this patch tested? UT. Closes #28654 from lipzhu/SPARK-31834. Authored-by: lipzhu <lipzhu@ebay.com> Signed-off-by: HyukjinKwon <gurwls223@apache.org> (cherry picked from commit d79a8a8) Signed-off-by: HyukjinKwon <gurwls223@apache.org>
What changes were proposed in this pull request?
We should use dataType.catalogString to unified the data type mismatch message.
Before:
After:
How was this patch tested?
UT.