[SPARK-10371][SQL][FOLLOW-UP] fix code style

Author: Wenchen Fan <wenchen@databricks.com>

Closes #9627 from cloud-fan/follow.

3 files changed, 30 insertions, 33 deletions

diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/EquivalentExpressions.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/EquivalentExpressions.scala
index e7380d21f9..f83df494ba 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/EquivalentExpressions.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/EquivalentExpressions.scala
@@ -29,29 +29,27 @@ class EquivalentExpressions {
    * Wrapper around an Expression that provides semantic equality.
    */
   case class Expr(e: Expression) {
-    val hash = e.semanticHash()
     override def equals(o: Any): Boolean = o match {
       case other: Expr => e.semanticEquals(other.e)
       case _ => false
     }
-    override def hashCode: Int = hash
+    override val hashCode: Int = e.semanticHash()
   }
 
   // For each expression, the set of equivalent expressions.
-  private val equivalenceMap: mutable.HashMap[Expr, mutable.MutableList[Expression]] =
-      new mutable.HashMap[Expr, mutable.MutableList[Expression]]
+  private val equivalenceMap = mutable.HashMap.empty[Expr, mutable.MutableList[Expression]]
 
   /**
    * Adds each expression to this data structure, grouping them with existing equivalent
    * expressions. Non-recursive.
-   * Returns if there was already a matching expression.
+   * Returns true if there was already a matching expression.
    */
   def addExpr(expr: Expression): Boolean = {
     if (expr.deterministic) {
       val e: Expr = Expr(expr)
       val f = equivalenceMap.get(e)
       if (f.isDefined) {
-        f.get.+= (expr)
+        f.get += expr
         true
       } else {
         equivalenceMap.put(e, mutable.MutableList(expr))
@@ -63,19 +61,19 @@ class EquivalentExpressions {
   }
 
   /**
-   * Adds the expression to this datastructure recursively. Stops if a matching expression
+   * Adds the expression to this data structure recursively. Stops if a matching expression
    * is found. That is, if `expr` has already been added, its children are not added.
    * If ignoreLeaf is true, leaf nodes are ignored.
    */
   def addExprTree(root: Expression, ignoreLeaf: Boolean = true): Unit = {
     val skip = root.isInstanceOf[LeafExpression] && ignoreLeaf
-    if (!skip && root.deterministic && !addExpr(root)) {
-     root.children.foreach(addExprTree(_, ignoreLeaf))
+    if (!skip && !addExpr(root)) {
+      root.children.foreach(addExprTree(_, ignoreLeaf))
     }
   }
 
   /**
-   * Returns all fo the expression trees that are equivalent to `e`. Returns
+   * Returns all of the expression trees that are equivalent to `e`. Returns
    * an empty collection if there are none.
    */
   def getEquivalentExprs(e: Expression): Seq[Expression] = {
@@ -90,8 +88,8 @@ class EquivalentExpressions {
   }
 
   /**
-   * Returns the state of the datastructure as a string. If all is false, skips sets of equivalent
-   * expressions with cardinality 1.
+   * Returns the state of the data structure as a string. If `all` is false, skips sets of
+   * equivalent expressions with cardinality 1.
    */
   def debugString(all: Boolean = false): String = {
     val sb: mutable.StringBuilder = new StringBuilder()
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala
index 7d5741eefc..540ed35006 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala
@@ -92,17 +92,16 @@ abstract class Expression extends TreeNode[Expression] {
    * @return [[GeneratedExpressionCode]]
    */
   def gen(ctx: CodeGenContext): GeneratedExpressionCode = {
-    val subExprState = ctx.subExprEliminationExprs.get(this)
-    if (subExprState.isDefined) {
+    ctx.subExprEliminationExprs.get(this).map { subExprState =>
       // This expression is repeated meaning the code to evaluated has already been added
       // as a function, `subExprState.fnName`. Just call that.
       val code =
         s"""
            |/* $this */
-           |${subExprState.get.fnName}(${ctx.INPUT_ROW});
-           |""".stripMargin.trim
-      GeneratedExpressionCode(code, subExprState.get.code.isNull, subExprState.get.code.value)
-    } else {
+           |${subExprState.fnName}(${ctx.INPUT_ROW});
+         """.stripMargin.trim
+      GeneratedExpressionCode(code, subExprState.code.isNull, subExprState.code.value)
+    }.getOrElse {
       val isNull = ctx.freshName("isNull")
       val primitive = ctx.freshName("primitive")
       val ve = GeneratedExpressionCode("", isNull, primitive)
@@ -157,7 +156,7 @@ abstract class Expression extends TreeNode[Expression] {
         case (i1, i2) => i1 == i2
       }
     }
-    // Non-determinstic expressions cannot be equal
+    // Non-deterministic expressions cannot be semantic equal
     if (!deterministic || !other.deterministic) return false
     val elements1 = this.productIterator.toSeq
     val elements2 = other.asInstanceOf[Product].productIterator.toSeq
@@ -174,11 +173,11 @@ abstract class Expression extends TreeNode[Expression] {
       var hash: Int = 17
       e.foreach(i => {
         val h: Int = i match {
-          case (e: Expression) => e.semanticHash()
-          case (Some(e: Expression)) => e.semanticHash()
-          case (t: Traversable[_]) => computeHash(t.toSeq)
+          case e: Expression => e.semanticHash()
+          case Some(e: Expression) => e.semanticHash()
+          case t: Traversable[_] => computeHash(t.toSeq)
           case null => 0
-          case (o) => o.hashCode()
+          case other => other.hashCode()
         }
         hash = hash * 37 + h
       })
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala
index 60a3d60184..5a4bba232b 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala
@@ -109,15 +109,15 @@ class CodeGenContext {
 
   // State used for subexpression elimination.
   case class SubExprEliminationState(
-    val isLoaded: String, code: GeneratedExpressionCode, val fnName: String)
+      isLoaded: String,
+      code: GeneratedExpressionCode,
+      fnName: String)
 
   // Foreach expression that is participating in subexpression elimination, the state to use.
-  val subExprEliminationExprs: mutable.HashMap[Expression, SubExprEliminationState] =
-    mutable.HashMap[Expression, SubExprEliminationState]()
+  val subExprEliminationExprs = mutable.HashMap.empty[Expression, SubExprEliminationState]
 
   // The collection of isLoaded variables that need to be reset on each row.
-  val subExprIsLoadedVariables: mutable.ArrayBuffer[String] =
-    mutable.ArrayBuffer.empty[String]
+  val subExprIsLoadedVariables = mutable.ArrayBuffer.empty[String]
 
   final val JAVA_BOOLEAN = "boolean"
   final val JAVA_BYTE = "byte"
@@ -361,7 +361,7 @@ class CodeGenContext {
       val expr = e.head
       val isLoaded = freshName("isLoaded")
       val isNull = freshName("isNull")
-      val primitive = freshName("primitive")
+      val value = freshName("value")
       val fnName = freshName("evalExpr")
 
       // Generate the code for this expression tree and wrap it in a function.
@@ -373,13 +373,13 @@ class CodeGenContext {
            |    ${code.code.trim}
            |    $isLoaded = true;
            |    $isNull = ${code.isNull};
-           |    $primitive = ${code.value};
+           |    $value = ${code.value};
            |  }
            |}
            """.stripMargin
       code.code = fn
       code.isNull = isNull
-      code.value = primitive
+      code.value = value
 
       addNewFunction(fnName, fn)
 
@@ -406,8 +406,8 @@ class CodeGenContext {
       // optimizations.
       addMutableState("boolean", isLoaded, s"$isLoaded = false;")
       addMutableState("boolean", isNull, s"$isNull = false;")
-      addMutableState(javaType(expr.dataType), primitive,
-        s"$primitive = ${defaultValue(expr.dataType)};")
+      addMutableState(javaType(expr.dataType), value,
+        s"$value = ${defaultValue(expr.dataType)};")
       subExprIsLoadedVariables += isLoaded
 
       val state = SubExprEliminationState(isLoaded, code, fnName)