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-29296][BUILD][CORE] Remove use of .par to make 2.13 support easier; add scala-2.13 profile to enable pulling in par collections library separately, for the future #25980

Closed
wants to merge 2 commits into from

Conversation

srowen
Copy link
Member

@srowen srowen commented Sep 30, 2019

What changes were proposed in this pull request?

Scala 2.13 removes the parallel collections classes to a separate library, so first, this establishes a scala-2.13 profile to bring it back, for future use.

However the library enables use of .par implicit conversions via a new class that is not in 2.12, which makes cross-building hard. This implements a suggested workaround from scala/scala-parallel-collections#22 to avoid .par entirely.

Why are the changes needed?

To compile for 2.13 and later to work with 2.13.

Does this PR introduce any user-facing change?

Should not, no.

How was this patch tested?

Existing tests.

…e to enable pulling in par collections library separately, for the future
@srowen srowen self-assigned this Sep 30, 2019
@SparkQA
Copy link

SparkQA commented Sep 30, 2019

Test build #111617 has finished for PR 25980 at commit 04c7639.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@@ -113,7 +115,7 @@ class CastSuite extends SparkFunSuite with ExpressionEvalHelper {
}

test("cast string to timestamp") {
ALL_TIMEZONES.par.foreach { tz =>
new ParVector(ALL_TIMEZONES.toVector).foreach { tz =>
Copy link
Member

Choose a reason for hiding this comment

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

ALL_TIMEZONES is Seq here. Maybe, ParSeq instead of ParVector?

Copy link
Member Author

Choose a reason for hiding this comment

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

ParSeq is a trait unfortunately. I think this is the right impl for this case.

@@ -46,7 +46,7 @@ class SQLExecutionSuite extends SparkFunSuite {
import spark.implicits._
try {
// Should not throw IllegalArgumentException
(1 to 100).par.foreach { _ =>
new ParVector((1 to 100).toVector).foreach { _ =>
Copy link
Member

Choose a reason for hiding this comment

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

Would ParRange be better?

@srowen srowen changed the title [WIP][SPARK-29296][BUILD][CORE] Remove use of .par to make 2.13 support easier; add scala-2.13 profile to enable pulling in par collections library separately, for the future [SPARK-29296][BUILD][CORE] Remove use of .par to make 2.13 support easier; add scala-2.13 profile to enable pulling in par collections library separately, for the future Oct 1, 2019
@SparkQA
Copy link

SparkQA commented Oct 1, 2019

Test build #111645 has finished for PR 25980 at commit 18b6298.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@srowen
Copy link
Member Author

srowen commented Oct 3, 2019

Merged to master

@srowen srowen closed this in 7aca0dd Oct 3, 2019
@srowen srowen deleted the SPARK-29296 branch October 9, 2019 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants