-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-25256][SQL][TEST] Plan mismatch errors in Hive tests in Scala 2.12 #22264
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
Conversation
|
Hah yeah I hope this is all there is to it! |
|
@srowen Still work in progress. I'm running the Scala 2.12 build locally. Before working on this one, I summited another PR #22260 to fix the failing compiling. There are three unit test failures in total. The first one is fixed. And this is the cause of the second one: A behavior change introduced by scala/scala#5551 and effectively this merged one scala/scala#5607 . I will continue my work later. |
|
To simplify: is |
|
Thanks a lot for your help, and the lead on what's happening here. You're not saying it's a scala issue, right? some behavior changed but it's arguably a fix? The ideal outcome in Spark is some implementation that works in both 2.11 and 2.12 with the same semantics. Let me know how far you get or if I can help test. This might be the last line of issues for 2.12 support. |
|
The fix works for both 2.11 and 2.12. And I reported a bug: scala/bug#11123 |
|
|
|
Test build #4304 has finished for PR 22264 at commit
|
|
Test build #4309 has finished for PR 22264 at commit
|
|
@sadhen this looks plausible. The fact that it only touches tests makes it low-risk to merge. My only larger concern is: is there a behavior change that will impact user code, that we are merely working around in tests here? I'm OK with getting to a passing 2.12 build that we can release as a 'beta' with some known issues, so this is probably fine to merge as it won't affect 2.11 core functionality. |
And for Scala 2.11, the result is true. |
|
scala/bug#11123 had been added into https://github.com/scala/bug/milestone/93 . I will spare some time working on it. |
|
Yeah, OK. I think this is acceptable as a potential "known issue" for Scala 2.12 support, which we can accept for a beta release of 2.12 support with Spark 2.4. I think I'd merge this and then see where we are. |
|
@srowen A PR for this "bug" is proposed: scala/scala#7156 Hopefully, Scala 2.12.7 will fix it. |
…2.12
## What changes were proposed in this pull request?
### For `SPARK-5775 read array from partitioned_parquet_with_key_and_complextypes`:
scala2.12
```
scala> (1 to 10).toString
res4: String = Range 1 to 10
```
scala2.11
```
scala> (1 to 10).toString
res2: String = Range(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)
```
And
```
def prepareAnswer(answer: Seq[Row], isSorted: Boolean): Seq[Row] = {
val converted: Seq[Row] = answer.map(prepareRow)
if (!isSorted) converted.sortBy(_.toString()) else converted
}
```
sortBy `_.toString` is not a good idea.
### Other failures are caused by
```
Array(Int.box(1)).toSeq == Array(Double.box(1.0)).toSeq
```
It is false in 2.12.2 + and is true in 2.11.x , 2.12.0, 2.12.1
## How was this patch tested?
This is a patch on a specific unit test.
Closes apache#22264 from sadhen/SPARK25256.
Authored-by: 忍冬 <rendong@wacai.com>
Signed-off-by: Sean Owen <sean.owen@databricks.com>
What changes were proposed in this pull request?
For
SPARK-5775 read array from partitioned_parquet_with_key_and_complextypes:scala2.12
scala2.11
And
sortBy
_.toStringis not a good idea.Other failures are caused by
It is false in 2.12.2 + and is true in 2.11.x , 2.12.0, 2.12.1
How was this patch tested?
This is a patch on a specific unit test.