[SPARK-19544][SQL] Improve error message when some column types are compatible and others are not in set operations #16882
Conversation
|
cc @hvanhovell could you please take a look? |
HyukjinKwon
changed the title
|
Test build #72683 has finished for PR 16882 at commit
|
HyukjinKwon
force-pushed
the
SPARK-19544
branch
2 times, most recently
from
c2fe413 to
073c6f9
Compare
|
Test build #72691 has started for PR 16882 at commit |
…s are not in set/union operations
HyukjinKwon
force-pushed
the
SPARK-19544
branch
from
073c6f9 to
20b8e77
Compare
|
Test build #72692 has started for PR 16882 at commit |
|
Test build #72693 has started for PR 16882 at commit |
|
retest this please |
|
@HyukjinKwon Can you also improve the error message. I don't think Other than that, this looks fine. |
|
Oh, sure. Let me give a shot. Thank you for your quick review! |
|
Test build #72701 has finished for PR 16882 at commit
|
| |${operator.nodeName} can only be performed on tables with the compatible | ||
| |column types. $dt1 <> $dt2 at the ${ordinalNumber(ci)} column of | ||
| |the ${ordinalNumber(ti + 1)} table | ||
| |column types. ${dt1.simpleString} <> ${dt2.simpleString} at the |
There was a problem hiding this comment.
(I used simpleString for consistency with other codes in this)
There was a problem hiding this comment.
StructType.simpleString gets truncated, so it might be hard/impossible to find the error. Can you use StructType.catalogString?
For better UX it might be an idea to render the path to the offending variable, instead of printing the entire struct.
There was a problem hiding this comment.
Sure, let me change.
|
Test build #72737 has finished for PR 16882 at commit
|
| * i.e. the main difference with [[findTightestCommonType]] is that here we allow some | ||
| * loss of precision when widening decimal and double, and promotion to string. | ||
| */ | ||
| private def findWiderTypeForTwo(t1: DataType, t2: DataType): Option[DataType] = (t1, t2) match { |
|
Test build #72756 has finished for PR 16882 at commit
|
|
Test build #72817 has finished for PR 16882 at commit
|
|
LGTM - merging to master. Thanks! |
|
Thank you @hvanhovell |
…ompatible and others are not in set operations
## What changes were proposed in this pull request?
This PR proposes to fix the error message when some data types are compatible and others are not in set/union operation.
Currently, the code below:
```scala
Seq((1,("a", 1))).toDF.union(Seq((1L,("a", "b"))).toDF)
```
throws an exception saying `LongType` and `IntegerType` are incompatible types. It should say something about `StructType`s with more readable format as below:
**Before**
```
Union can only be performed on tables with the compatible column types.
LongType <> IntegerType at the first column of the second table;;
```
**After**
```
Union can only be performed on tables with the compatible column types.
struct<_1:string,_2:string> <> struct<_1:string,_2:int> at the second column of the second table;;
```
*I manually inserted a newline in the messages above for readability only in this PR description.
## How was this patch tested?
Unit tests in `AnalysisErrorSuite`, manual tests and build wth Scala 2.10.
Author: hyukjinkwon <[email protected]>
Closes apache#16882 from HyukjinKwon/SPARK-19544.
4 participants
What changes were proposed in this pull request?
This PR proposes to fix the error message when some data types are compatible and others are not in set/union operation.
Currently, the code below:
throws an exception saying
LongTypeandIntegerTypeare incompatible types. It should say something aboutStructTypes with more readable format as below:Before
After
*I manually inserted a newline in the messages above for readability only in this PR description.
How was this patch tested?
Unit tests in
AnalysisErrorSuite, manual tests and build wth Scala 2.10.