Skip to content

Commit 0d63757

Browse files
cloud-fanyhuai
authored andcommitted
[SPARK-10371][SQL][FOLLOW-UP] fix code style
Author: Wenchen Fan <[email protected]> Closes #9627 from cloud-fan/follow. (cherry picked from commit 1510c52) Signed-off-by: Yin Huai <[email protected]>
1 parent 7de8abd commit 0d63757

File tree

3 files changed

+30
-33
lines changed

3 files changed

+30
-33
lines changed

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/EquivalentExpressions.scala

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,29 +29,27 @@ class EquivalentExpressions {
2929
* Wrapper around an Expression that provides semantic equality.
3030
*/
3131
case class Expr(e: Expression) {
32-
val hash = e.semanticHash()
3332
override def equals(o: Any): Boolean = o match {
3433
case other: Expr => e.semanticEquals(other.e)
3534
case _ => false
3635
}
37-
override def hashCode: Int = hash
36+
override val hashCode: Int = e.semanticHash()
3837
}
3938

4039
// For each expression, the set of equivalent expressions.
41-
private val equivalenceMap: mutable.HashMap[Expr, mutable.MutableList[Expression]] =
42-
new mutable.HashMap[Expr, mutable.MutableList[Expression]]
40+
private val equivalenceMap = mutable.HashMap.empty[Expr, mutable.MutableList[Expression]]
4341

4442
/**
4543
* Adds each expression to this data structure, grouping them with existing equivalent
4644
* expressions. Non-recursive.
47-
* Returns if there was already a matching expression.
45+
* Returns true if there was already a matching expression.
4846
*/
4947
def addExpr(expr: Expression): Boolean = {
5048
if (expr.deterministic) {
5149
val e: Expr = Expr(expr)
5250
val f = equivalenceMap.get(e)
5351
if (f.isDefined) {
54-
f.get.+= (expr)
52+
f.get += expr
5553
true
5654
} else {
5755
equivalenceMap.put(e, mutable.MutableList(expr))
@@ -63,19 +61,19 @@ class EquivalentExpressions {
6361
}
6462

6563
/**
66-
* Adds the expression to this datastructure recursively. Stops if a matching expression
64+
* Adds the expression to this data structure recursively. Stops if a matching expression
6765
* is found. That is, if `expr` has already been added, its children are not added.
6866
* If ignoreLeaf is true, leaf nodes are ignored.
6967
*/
7068
def addExprTree(root: Expression, ignoreLeaf: Boolean = true): Unit = {
7169
val skip = root.isInstanceOf[LeafExpression] && ignoreLeaf
72-
if (!skip && root.deterministic && !addExpr(root)) {
73-
root.children.foreach(addExprTree(_, ignoreLeaf))
70+
if (!skip && !addExpr(root)) {
71+
root.children.foreach(addExprTree(_, ignoreLeaf))
7472
}
7573
}
7674

7775
/**
78-
* Returns all fo the expression trees that are equivalent to `e`. Returns
76+
* Returns all of the expression trees that are equivalent to `e`. Returns
7977
* an empty collection if there are none.
8078
*/
8179
def getEquivalentExprs(e: Expression): Seq[Expression] = {
@@ -90,8 +88,8 @@ class EquivalentExpressions {
9088
}
9189

9290
/**
93-
* Returns the state of the datastructure as a string. If all is false, skips sets of equivalent
94-
* expressions with cardinality 1.
91+
* Returns the state of the data structure as a string. If `all` is false, skips sets of
92+
* equivalent expressions with cardinality 1.
9593
*/
9694
def debugString(all: Boolean = false): String = {
9795
val sb: mutable.StringBuilder = new StringBuilder()

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -92,17 +92,16 @@ abstract class Expression extends TreeNode[Expression] {
9292
* @return [[GeneratedExpressionCode]]
9393
*/
9494
def gen(ctx: CodeGenContext): GeneratedExpressionCode = {
95-
val subExprState = ctx.subExprEliminationExprs.get(this)
96-
if (subExprState.isDefined) {
95+
ctx.subExprEliminationExprs.get(this).map { subExprState =>
9796
// This expression is repeated meaning the code to evaluated has already been added
9897
// as a function, `subExprState.fnName`. Just call that.
9998
val code =
10099
s"""
101100
|/* $this */
102-
|${subExprState.get.fnName}(${ctx.INPUT_ROW});
103-
|""".stripMargin.trim
104-
GeneratedExpressionCode(code, subExprState.get.code.isNull, subExprState.get.code.value)
105-
} else {
101+
|${subExprState.fnName}(${ctx.INPUT_ROW});
102+
""".stripMargin.trim
103+
GeneratedExpressionCode(code, subExprState.code.isNull, subExprState.code.value)
104+
}.getOrElse {
106105
val isNull = ctx.freshName("isNull")
107106
val primitive = ctx.freshName("primitive")
108107
val ve = GeneratedExpressionCode("", isNull, primitive)
@@ -157,7 +156,7 @@ abstract class Expression extends TreeNode[Expression] {
157156
case (i1, i2) => i1 == i2
158157
}
159158
}
160-
// Non-determinstic expressions cannot be equal
159+
// Non-deterministic expressions cannot be semantic equal
161160
if (!deterministic || !other.deterministic) return false
162161
val elements1 = this.productIterator.toSeq
163162
val elements2 = other.asInstanceOf[Product].productIterator.toSeq
@@ -174,11 +173,11 @@ abstract class Expression extends TreeNode[Expression] {
174173
var hash: Int = 17
175174
e.foreach(i => {
176175
val h: Int = i match {
177-
case (e: Expression) => e.semanticHash()
178-
case (Some(e: Expression)) => e.semanticHash()
179-
case (t: Traversable[_]) => computeHash(t.toSeq)
176+
case e: Expression => e.semanticHash()
177+
case Some(e: Expression) => e.semanticHash()
178+
case t: Traversable[_] => computeHash(t.toSeq)
180179
case null => 0
181-
case (o) => o.hashCode()
180+
case other => other.hashCode()
182181
}
183182
hash = hash * 37 + h
184183
})

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -109,15 +109,15 @@ class CodeGenContext {
109109

110110
// State used for subexpression elimination.
111111
case class SubExprEliminationState(
112-
val isLoaded: String, code: GeneratedExpressionCode, val fnName: String)
112+
isLoaded: String,
113+
code: GeneratedExpressionCode,
114+
fnName: String)
113115

114116
// Foreach expression that is participating in subexpression elimination, the state to use.
115-
val subExprEliminationExprs: mutable.HashMap[Expression, SubExprEliminationState] =
116-
mutable.HashMap[Expression, SubExprEliminationState]()
117+
val subExprEliminationExprs = mutable.HashMap.empty[Expression, SubExprEliminationState]
117118

118119
// The collection of isLoaded variables that need to be reset on each row.
119-
val subExprIsLoadedVariables: mutable.ArrayBuffer[String] =
120-
mutable.ArrayBuffer.empty[String]
120+
val subExprIsLoadedVariables = mutable.ArrayBuffer.empty[String]
121121

122122
final val JAVA_BOOLEAN = "boolean"
123123
final val JAVA_BYTE = "byte"
@@ -361,7 +361,7 @@ class CodeGenContext {
361361
val expr = e.head
362362
val isLoaded = freshName("isLoaded")
363363
val isNull = freshName("isNull")
364-
val primitive = freshName("primitive")
364+
val value = freshName("value")
365365
val fnName = freshName("evalExpr")
366366

367367
// Generate the code for this expression tree and wrap it in a function.
@@ -373,13 +373,13 @@ class CodeGenContext {
373373
| ${code.code.trim}
374374
| $isLoaded = true;
375375
| $isNull = ${code.isNull};
376-
| $primitive = ${code.value};
376+
| $value = ${code.value};
377377
| }
378378
|}
379379
""".stripMargin
380380
code.code = fn
381381
code.isNull = isNull
382-
code.value = primitive
382+
code.value = value
383383

384384
addNewFunction(fnName, fn)
385385

@@ -406,8 +406,8 @@ class CodeGenContext {
406406
// optimizations.
407407
addMutableState("boolean", isLoaded, s"$isLoaded = false;")
408408
addMutableState("boolean", isNull, s"$isNull = false;")
409-
addMutableState(javaType(expr.dataType), primitive,
410-
s"$primitive = ${defaultValue(expr.dataType)};")
409+
addMutableState(javaType(expr.dataType), value,
410+
s"$value = ${defaultValue(expr.dataType)};")
411411
subExprIsLoadedVariables += isLoaded
412412

413413
val state = SubExprEliminationState(isLoaded, code, fnName)

0 commit comments

Comments
 (0)