Skip to content

Commit

Permalink
[SPARK-1199][REPL] Remove VALId and use the original import style for…
Browse files Browse the repository at this point in the history
… defined classes.

This is an alternate solution to apache#1176.

Author: Prashant Sharma <prashant.s@imaginea.com>

Closes apache#1179 from ScrapCodes/SPARK-1199/repl-fix-second-approach and squashes the following commits:

820b34b [Prashant Sharma] Here we generate two kinds of import wrappers based on whether it is a class or not.
  • Loading branch information
ScrapCodes authored and pwendell committed Jul 4, 2014
1 parent 5448804 commit d434150
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 11 deletions.
7 changes: 5 additions & 2 deletions repl/src/main/scala/org/apache/spark/repl/SparkIMain.scala
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,7 @@ import org.apache.spark.util.Utils
*
* Read! Eval! Print! Some of that not yet centralized here.
*/
class ReadEvalPrint(lineId: Int) {
class ReadEvalPrint(val lineId: Int) {
def this() = this(freshLineId())

private var lastRun: Run = _
Expand Down Expand Up @@ -1241,7 +1241,10 @@ import org.apache.spark.util.Utils
// old style
beSilentDuring(parse(code)) foreach { ts =>
ts foreach { t =>
withoutUnwrapping(logDebug(asCompactString(t)))
if (isShow || isShowRaw)
withoutUnwrapping(echo(asCompactString(t)))
else
withoutUnwrapping(logDebug(asCompactString(t)))
}
}
}
Expand Down
23 changes: 14 additions & 9 deletions repl/src/main/scala/org/apache/spark/repl/SparkImports.scala
Original file line number Diff line number Diff line change
Expand Up @@ -182,15 +182,26 @@ trait SparkImports {
// ambiguity errors will not be generated. Also, quote
// the name of the variable, so that we don't need to
// handle quoting keywords separately.
case x: ClassHandler =>
// I am trying to guess if the import is a defined class
// This is an ugly hack, I am not 100% sure of the consequences.
// Here we, let everything but "defined classes" use the import with val.
// The reason for this is, otherwise the remote executor tries to pull the
// classes involved and may fail.
for (imv <- x.definedNames) {
val objName = req.lineRep.readPath
code.append("import " + objName + ".INSTANCE" + req.accessPath + ".`" + imv + "`\n")
}

case x =>
for (imv <- x.definedNames) {
if (currentImps contains imv) addWrapper()
val objName = req.lineRep.readPath
val valName = "$VAL" + newValId();
val valName = "$VAL" + req.lineRep.lineId

if(!code.toString.endsWith(".`" + imv + "`;\n")) { // Which means already imported
code.append("val " + valName + " = " + objName + ".INSTANCE;\n")
code.append("import " + valName + req.accessPath + ".`" + imv + "`;\n")
code.append("val " + valName + " = " + objName + ".INSTANCE;\n")
code.append("import " + valName + req.accessPath + ".`" + imv + "`;\n")
}
// code.append("val " + valName + " = " + objName + ".INSTANCE;\n")
// code.append("import " + valName + req.accessPath + ".`" + imv + "`;\n")
Expand All @@ -211,10 +222,4 @@ trait SparkImports {
private def membersAtPickler(sym: Symbol): List[Symbol] =
beforePickler(sym.info.nonPrivateMembers.toList)

private var curValId = 0

private def newValId(): Int = {
curValId += 1
curValId
}
}
12 changes: 12 additions & 0 deletions repl/src/test/scala/org/apache/spark/repl/ReplSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,18 @@ class ReplSuite extends FunSuite {
assertContains("res4: Array[Int] = Array(0, 0, 0, 0, 0)", output)
}

test("SPARK-1199-simple-reproduce") {
val output = runInterpreter("local-cluster[1,1,512]",
"""
|case class Sum(exp: String, exp2: String)
|val a = Sum("A", "B")
|def b(a: Sum): String = a match { case Sum(_, _) => "Found Sum" }
|b(a)
""".stripMargin)
assertDoesNotContain("error:", output)
assertDoesNotContain("Exception", output)
}

if (System.getenv("MESOS_NATIVE_LIBRARY") != null) {
test("running on Mesos") {
val output = runInterpreter("localquiet",
Expand Down

0 comments on commit d434150

Please sign in to comment.