Skip to content
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-40497][BUILD] Upgrade Scala to 2.13.11 #41626

Closed
wants to merge 7 commits into from

Conversation

LuciferYang
Copy link
Contributor

@LuciferYang LuciferYang commented Jun 16, 2023

What changes were proposed in this pull request?

This PR aims to upgrade Scala to 2.13.11

Additionally, this pr adds a new suppression rule for warning message: Implicit definition should have explicit type, this is a new compile check introduced by scala/scala#10083, we must fix it when we upgrading to use Scala 3,

Why are the changes needed?

This release improves collections, adds support for JDK 20 and 21, adds support for JDK 17 sealed:

There are 2 known issues in this version:

For the first one, there is no compilation warning messages related to match may not be exhaustive in Spark compile log, and for the second one, there is no case of method.isAnnotationPresent(Deprecated.class) in Spark code, there is just

val hasNonNull = readMethod.isAnnotationPresent(classOf[Nonnull])

in Spark Code, and I checked @javax.annotation.Nonnull no this issue.

So I think These two issues will not affect Spark itself, but this doesn't mean it won't affect the code written by end users themselves

The full release notes as follows:

Does this PR introduce any user-facing change?

Yes, this is a Scala version change.

How was this patch tested?

  • Existing Test
  • Checked Java 8/17 + Scala 2.13.11 using GA, all test passed

Java 8 + Scala 2.13.11: https://github.com/LuciferYang/spark/runs/14337279564
Java 17 + Scala 2.13.11: https://github.com/LuciferYang/spark/runs/14343012195

@github-actions github-actions bot added the BUILD label Jun 16, 2023
pom.xml Outdated Show resolved Hide resolved
@LuciferYang
Copy link
Contributor Author

Test first

@LuciferYang LuciferYang marked this pull request as draft June 16, 2023 13:08
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, did you have a chance to check the reported issues mentioned by Jungtaek, @LuciferYang ?

@LuciferYang
Copy link
Contributor Author

BTW, did you have a chance to check the reported issues mentioned by Jungtaek, @LuciferYang ?

OK, let me check this

@github-actions github-actions bot added the INFRA label Jun 17, 2023
@@ -190,6 +190,7 @@ jobs:
HIVE_PROFILE: ${{ matrix.hive }}
GITHUB_PREV_SHA: ${{ github.event.before }}
SPARK_LOCAL_IP: localhost
SCALA_PROFILE: scala2.13
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will revert after test

@LuciferYang
Copy link
Contributor Author

dc4def0 test Java8 + Scala 2.13, all test passed.

@@ -25,7 +25,7 @@ on:
java:
required: false
type: string
default: 8
default: 17
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try to test Java 17 + Scala 2.13.11, will revert later

@LuciferYang
Copy link
Contributor Author

https://github.com/LuciferYang/spark/actions/runs/5298355546/jobs/9595559054

image

Java 17 + Scala 2.13 all test passed

@github-actions github-actions bot removed the INFRA label Jun 18, 2023
@LuciferYang
Copy link
Contributor Author

revert change of build_and_test.yml

@LuciferYang
Copy link
Contributor Author

I will do more check tomorrow and update pr description, seems is a available new version for now.

@LuciferYang LuciferYang marked this pull request as ready for review June 19, 2023 09:10
@LuciferYang
Copy link
Contributor Author

@dongjoon-hyun @HyukjinKwon I think it's ok to upgrade to Scala 2.13.11 for Spark

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM. Thank you, @LuciferYang .
Merged to master.

@LuciferYang
Copy link
Contributor Author

Thanks @dongjoon-hyun

czxm pushed a commit to czxm/spark that referenced this pull request Jun 19, 2023
### What changes were proposed in this pull request?
This PR aims to upgrade Scala to 2.13.11
- https://www.scala-lang.org/news/2.13.11

Additionally, this pr adds a new suppression rule for warning message: `Implicit definition should have explicit type`, this is a new compile check introduced by scala/scala#10083, we must fix it when we upgrading to use Scala 3,

### Why are the changes needed?
This release improves collections, adds support for JDK 20 and 21, adds support for JDK 17 `sealed`:
- scala/scala#10363
- scala/scala#10184
- scala/scala#10397
- scala/scala#10348
- scala/scala#10105

There are 2 known issues in this version:

- scala/bug#12800
- scala/bug#12799

For the first one, there is no compilation warning messages related to `match may not be exhaustive` in Spark compile log, and for the second one, there is no case of `method.isAnnotationPresent(Deprecated.class)` in Spark code, there is just

https://github.com/apache/spark/blob/8c84d2c9349d7b607db949c2e114df781f23e438/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/JavaTypeInference.scala#L130

in Spark Code, and I checked `javax.annotation.Nonnull` no this issue.

So I think These two issues will not affect Spark itself, but this doesn't mean it won't affect the code written by end users themselves

The full release notes as follows:

- https://github.com/scala/scala/releases/tag/v2.13.11

### Does this PR introduce _any_ user-facing change?
Yes, this is a Scala version change.

### How was this patch tested?
- Existing Test
- Checked Java 8/17 + Scala 2.13.11 using GA, all test passed

Java 8 + Scala 2.13.11: https://github.com/LuciferYang/spark/runs/14337279564
Java 17 + Scala 2.13.11: https://github.com/LuciferYang/spark/runs/14343012195

Closes apache#41626 from LuciferYang/SPARK-40497.

Authored-by: yangjie01 <yangjie01@baidu.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
@dongjoon-hyun
Copy link
Member

Thank you always, @LuciferYang !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants