[SPARK-4397] Move object RDD to the front of RDD.scala. #3580

rxin · 2014-12-03T10:04:45Z

I ran into multiple cases that SBT/Scala compiler was confused by the implicits in continuous compilation mode. Adding explicit return types fixes the problem.

I ran into multiple cases that SBT/Scala compiler was confused by the implicits in continuous compilation mode. Moving them to the beginning of the file seemed to have fixed the problem.

SparkQA · 2014-12-03T10:10:24Z

Test build #24088 has started for PR 3580 at commit a836a37.

This patch merges cleanly.

SparkQA · 2014-12-03T10:12:48Z

Test build #24088 has finished for PR 3580 at commit a836a37.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2014-12-03T10:12:49Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24088/
Test FAILed.

zsxwing · 2014-12-03T10:22:14Z

core/src/main/scala/org/apache/spark/rdd/RDD.scala

+  }
+
+  implicit def rddToOrderedRDDFunctions[K : Ordering : ClassTag, V: ClassTag](rdd: RDD[(K, V)])
+    : OrderedRDDFunctions[K, V] = {


should be OrderedRDDFunctions[K, V, (K, V)]

zsxwing · 2014-12-03T12:55:07Z

Moving them to the beginning of the file seemed to have fixed the problem.

Not sure if this is the correct fix. Here is the error information in my machine:

Note: implicit method numericRDDToDoubleRDDFunctions is not applicable here because it comes after the application point and it lacks an explicit result type

After I added the return types to all implicit methods, I could not reproduce this error in continuous compilation mode.

zsxwing · 2014-12-03T12:56:12Z

Could you also add the return type for writableWritableConverter? It has the similar issue.

  implicit def writableWritableConverter[T <: Writable](): WritableConverter[T] =
    new WritableConverter[T](_.runtimeClass.asInstanceOf[Class[T]], _.asInstanceOf[T])

Conflicts: core/src/main/scala/org/apache/spark/SparkContext.scala

rxin · 2014-12-03T21:49:31Z

Alright pushed a new version.

SparkQA · 2014-12-03T21:54:53Z

Test build #24106 has started for PR 3580 at commit b8562c9.

This patch merges cleanly.

AmplabJenkins · 2014-12-03T22:02:22Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24104/
Test FAILed.

rxin · 2014-12-03T22:05:40Z

Jenkins, retest this please.

SparkQA · 2014-12-03T22:10:45Z

Test build #24111 has started for PR 3580 at commit b8562c9.

This patch merges cleanly.

SparkQA · 2014-12-03T23:15:29Z

Test build #24106 has finished for PR 3580 at commit b8562c9.

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

AmplabJenkins · 2014-12-03T23:15:33Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24106/
Test PASSed.

SparkQA · 2014-12-03T23:29:50Z

Test build #24111 has finished for PR 3580 at commit b8562c9.

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

AmplabJenkins · 2014-12-03T23:29:53Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24111/
Test PASSed.

zsxwing · 2014-12-04T02:59:27Z

Could you test only adding the return types without moving the codes? If adding the return types works, I suggest not moving these codes.

andrewor14 · 2014-12-04T03:23:47Z

core/src/main/scala/org/apache/spark/rdd/RDD.scala

+  // compatibility and forward to the following functions directly.
+
+  implicit def rddToPairRDDFunctions[K, V](rdd: RDD[(K, V)])
+    (implicit kt: ClassTag[K], vt: ClassTag[V], ord: Ordering[K] = null): PairRDDFunctions[K, V] = {


you didn't preserve the indentation down there. was this on purpose?

yea 100 char ...

andrewor14 · 2014-12-04T03:24:48Z

LGTM, and +1 especially on adding return types to implicits.

rxin · 2014-12-04T08:52:33Z

I moved it to the end. Looks like the problem is with implicit functions without explicit return types.

cc @pwendell @mateiz @andrewor14 @JoshRosen in the future we should make sure all implicit functions have explicit return types. (And all public functions should have explicit return types defined as well)

SparkQA · 2014-12-04T08:52:52Z

Test build #24132 has started for PR 3580 at commit ee32fcd.

This patch merges cleanly.

SparkQA · 2014-12-04T10:09:32Z

Test build #24132 has finished for PR 3580 at commit ee32fcd.

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

AmplabJenkins · 2014-12-04T10:09:36Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24132/
Test PASSed.

zsxwing · 2014-12-04T10:14:02Z

LGTM

mateiz · 2014-12-04T18:56:50Z

Good catch on the return types. Would be great if we can make ScalaStyle complain about those.

rxin · 2014-12-05T00:32:03Z

I believe @adriaanm 's new linter can lint that (doesn't work in scalastyle right now).

rxin · 2014-12-05T00:32:06Z

Merging in master.

adriaanm · 2014-12-05T00:42:25Z

Yes, it's over at https://github.com/scala/scala-abide, by the way. (The fabled ImplicitMustNotHaveInferredType rule does not exist yet, though it would be easy to add.)

rxin · 2014-12-05T00:55:34Z

Thanks - any ETA on the 1st release?

adriaanm · 2014-12-05T01:45:58Z

Not yet. You can already use it, though. We want to have a good number of rules before we start advertising it. In the phase where we're looking for partners to build out the platform / flesh out the rule set right now.

…Spark SQL As we learned in #3580, not explicitly typing implicit functions can lead to compiler bugs and potentially unexpected runtime behavior. Author: Reynold Xin <rxin@databricks.com> Closes #3859 from rxin/sql-implicits and squashes the following commits: 30c2c24 [Reynold Xin] [SPARK-5038] Add explicit return type for implicit functions in Spark SQL.

As we learned in #3580, not explicitly typing implicit functions can lead to compiler bugs and potentially unexpected runtime behavior. This is a follow up PR for rest of Spark (outside Spark SQL). The original PR for Spark SQL can be found at #3859 Author: Reynold Xin <rxin@databricks.com> Closes #3860 from rxin/implicit and squashes the following commits: 73702f9 [Reynold Xin] [SPARK-5038] Add explicit return type for implicit functions.

Move object RDD to the front of RDD.scala.

a836a37

I ran into multiple cases that SBT/Scala compiler was confused by the implicits in continuous compilation mode. Moving them to the beginning of the file seemed to have fixed the problem.

rxin changed the title ~~Move object RDD to the front of RDD.scala.~~ [SPARK-4397] Move object RDD to the front of RDD.scala. Dec 3, 2014

zsxwing reviewed Dec 3, 2014
View reviewed changes

rxin added 2 commits December 3, 2014 13:45

Code review.

d4e9f85

Merge branch 'master' of github.com:apache/spark into rdd-implicit

b8562c9

Conflicts: core/src/main/scala/org/apache/spark/SparkContext.scala

andrewor14 reviewed Dec 4, 2014
View reviewed changes

Move object RDD to the end of the file.

ee32fcd

asfgit closed this in ed92b47 Dec 5, 2014

This was referenced Dec 31, 2014

[SPARK-5038][SQL] Add explicit return type for implicit functions in Spark SQL #3859

Closed

[SPARK-5038] Add explicit return type for implicit functions. #3860

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-4397] Move object RDD to the front of RDD.scala. #3580

[SPARK-4397] Move object RDD to the front of RDD.scala. #3580

rxin commented Dec 3, 2014

SparkQA commented Dec 3, 2014

SparkQA commented Dec 3, 2014

AmplabJenkins commented Dec 3, 2014

zsxwing Dec 3, 2014

zsxwing commented Dec 3, 2014

zsxwing commented Dec 3, 2014

rxin commented Dec 3, 2014

SparkQA commented Dec 3, 2014

AmplabJenkins commented Dec 3, 2014

rxin commented Dec 3, 2014

SparkQA commented Dec 3, 2014

SparkQA commented Dec 3, 2014

AmplabJenkins commented Dec 3, 2014

SparkQA commented Dec 3, 2014

AmplabJenkins commented Dec 3, 2014

zsxwing commented Dec 4, 2014

andrewor14 Dec 4, 2014

rxin Dec 4, 2014

andrewor14 commented Dec 4, 2014

rxin commented Dec 4, 2014

SparkQA commented Dec 4, 2014

SparkQA commented Dec 4, 2014

AmplabJenkins commented Dec 4, 2014

zsxwing commented Dec 4, 2014

mateiz commented Dec 4, 2014

rxin commented Dec 5, 2014

rxin commented Dec 5, 2014

adriaanm commented Dec 5, 2014

rxin commented Dec 5, 2014

adriaanm commented Dec 5, 2014

[SPARK-4397] Move object RDD to the front of RDD.scala. #3580

[SPARK-4397] Move object RDD to the front of RDD.scala. #3580

Conversation

rxin commented Dec 3, 2014

SparkQA commented Dec 3, 2014

SparkQA commented Dec 3, 2014

AmplabJenkins commented Dec 3, 2014

zsxwing Dec 3, 2014

Choose a reason for hiding this comment

zsxwing commented Dec 3, 2014

zsxwing commented Dec 3, 2014

rxin commented Dec 3, 2014

SparkQA commented Dec 3, 2014

AmplabJenkins commented Dec 3, 2014

rxin commented Dec 3, 2014

SparkQA commented Dec 3, 2014

SparkQA commented Dec 3, 2014

AmplabJenkins commented Dec 3, 2014

SparkQA commented Dec 3, 2014

AmplabJenkins commented Dec 3, 2014

zsxwing commented Dec 4, 2014

andrewor14 Dec 4, 2014

Choose a reason for hiding this comment

rxin Dec 4, 2014

Choose a reason for hiding this comment

andrewor14 commented Dec 4, 2014

rxin commented Dec 4, 2014

SparkQA commented Dec 4, 2014

SparkQA commented Dec 4, 2014

AmplabJenkins commented Dec 4, 2014

zsxwing commented Dec 4, 2014

mateiz commented Dec 4, 2014

rxin commented Dec 5, 2014

rxin commented Dec 5, 2014

adriaanm commented Dec 5, 2014

rxin commented Dec 5, 2014

adriaanm commented Dec 5, 2014