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-12719][HOTFIX] Fix compilation against Scala 2.10 #11798

Closed
wants to merge 9 commits into from

Conversation

yy2016
Copy link

@yy2016 yy2016 commented Mar 17, 2016

What changes were proposed in this pull request?

Compilation against Scala 2.10 fails with:
[error] [warn] /home/jenkins/workspace/spark-master-compile-sbt-scala-2.10/sql/hive/src/main/scala/org/apache/spark/sql/hive/SQLBuilder.scala:483: Cannot check match for unreachability. [error] (The analysis required more space than allowed. Please try with scalac -Dscalac.patmat.analysisBudget=512 or -Dscalac.patmat.analysisBudget=off.) [error] [warn] private def addSubqueryIfNeeded(plan: LogicalPlan): LogicalPlan = plan match {

This PR increases patmat.analysisBudget

How was this patch tested?

Compilation against Scala 2.10

@yy2016
Copy link
Author

yy2016 commented Mar 17, 2016

@JoshRosen
See if this is better.

@JoshRosen
Copy link
Contributor

Why not pass it to scalacOptions instead of setting a system property? https://github.com/yy2016/spark/blob/master/project/SparkBuild.scala#L194

@yhuai
Copy link
Contributor

yhuai commented Mar 18, 2016

ok to test

@JoshRosen
Copy link
Contributor

Note that the PR builder won't actually test this against 2.10, so we'll still need to check this out and test it ourselves before merge.

@yhuai
Copy link
Contributor

yhuai commented Mar 18, 2016

let me test this

@yhuai
Copy link
Contributor

yhuai commented Mar 18, 2016

I tested locally. Unfortunately, it did not fix the problem.

@SparkQA
Copy link

SparkQA commented Mar 18, 2016

Test build #53485 has finished for PR 11798 at commit a5403b1.

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

@yy2016
Copy link
Author

yy2016 commented Mar 18, 2016

See if this is good.

I can modify the description when I get confirmation.

@SparkQA
Copy link

SparkQA commented Mar 18, 2016

Test build #53493 has finished for PR 11798 at commit bd37dcf.

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

@liancheng
Copy link
Contributor

@JoshRosen It seems that tuning scalac.patmat.analysisBudget doesn't work for this case. According to Scala compiler code, -Dscalac.patmat.analysisBudget=off is equivalent to setting this option to Integer.MAX_VALUE, but this error is still there. I think we should merge this case-expansion fix first to bring master SBT build back.

@yy2016 I'm merging this to master, will update the commit message while merging. Thanks for the fix!

@asfgit asfgit closed this in 90a1d8d Mar 18, 2016
@liancheng
Copy link
Contributor

@yy2016 Updated commit message:

PR #11696 introduced a complex pattern match that broke Scala 2.10 match unreachability check and caused build failure. This PR fixes this issue by expanding this pattern match into several simpler ones.

Note that tuning or turning off -Dscalac.patmat.analysisBudget doesn't work for this case.

Compilation against Scala 2.10

@SparkQA
Copy link

SparkQA commented Mar 18, 2016

Test build #53492 has finished for PR 11798 at commit f13abe8.

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

roygao94 pushed a commit to roygao94/spark that referenced this pull request Mar 22, 2016
PR apache#11696 introduced a complex pattern match that broke Scala 2.10 match unreachability check and caused build failure.  This PR fixes this issue by expanding this pattern match into several simpler ones.

Note that tuning or turning off `-Dscalac.patmat.analysisBudget` doesn't work for this case.

Compilation against Scala 2.10

Author: tedyu <yuzhihong@gmail.com>

Closes apache#11798 from yy2016/master.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants