Skip to content

Commit 0841c4a

Browse files
viiryaviirya
committed
Address comment.
1 parent 37ae9b0 commit 0841c4a

File tree

19 files changed

+45
-40
lines changed

19 files changed

+45
-40
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ abstract class Expression extends TreeNode[Expression] {
105105
val isNull = ctx.freshName("isNull")
106106
val value = ctx.freshName("value")
107107
val eval = doGenCode(ctx, ExprCode("",
108-
VariableValue(isNull, ExprType(ctx.JAVA_BOOLEAN, true)),
108+
VariableValue(isNull, ExprType(ctx.JAVA_BOOLEAN)),
109109
VariableValue(value, ExprType(ctx, dataType))))
110110
reduceCodeSize(ctx, eval)
111111
if (eval.code.nonEmpty) {
@@ -123,7 +123,7 @@ abstract class Expression extends TreeNode[Expression] {
123123
val setIsNull = if (!eval.isNull.isInstanceOf[LiteralValue]) {
124124
val globalIsNull = ctx.addMutableState(ctx.JAVA_BOOLEAN, "globalIsNull")
125125
val localIsNull = eval.isNull
126-
eval.isNull = GlobalValue(globalIsNull, ExprType(ctx.JAVA_BOOLEAN, true))
126+
eval.isNull = GlobalValue(globalIsNull, ExprType(ctx.JAVA_BOOLEAN))
127127
s"$globalIsNull = $localIsNull;"
128128
} else {
129129
""

‎sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -603,7 +603,7 @@ case class Least(children: Seq[Expression]) extends Expression {
603603
override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
604604
val evalChildren = children.map(_.genCode(ctx))
605605
ev.isNull = GlobalValue(ctx.addMutableState(ctx.JAVA_BOOLEAN, ev.isNull),
606-
ExprType(ctx.JAVA_BOOLEAN, true))
606+
ExprType(ctx.JAVA_BOOLEAN))
607607
val evals = evalChildren.map(eval =>
608608
s"""
609609
|${eval.code}
@@ -683,7 +683,7 @@ case class Greatest(children: Seq[Expression]) extends Expression {
683683
override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
684684
val evalChildren = children.map(_.genCode(ctx))
685685
ev.isNull = GlobalValue(ctx.addMutableState(ctx.JAVA_BOOLEAN, ev.isNull),
686-
ExprType(ctx.JAVA_BOOLEAN, true))
686+
ExprType(ctx.JAVA_BOOLEAN))
687687
val evals = evalChildren.map(eval =>
688688
s"""
689689
|${eval.code}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1208,7 +1208,7 @@ class CodegenContext {
12081208
// at least two nodes) as the cost of doing it is expected to be low.
12091209

12101210
subexprFunctions += s"${addNewFunction(fnName, fn)}($INPUT_ROW);"
1211-
val state = SubExprEliminationState(GlobalValue(isNull, ExprType(JAVA_BOOLEAN, true)),
1211+
val state = SubExprEliminationState(GlobalValue(isNull, ExprType(JAVA_BOOLEAN)),
12121212
GlobalValue(value, ExprType(this, expr.dataType)))
12131213
e.foreach(subExprEliminationExprs.put(_, state))
12141214
}

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

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ abstract class ExprValue {
3030
// For example, a variable created outside a method can not be accessed inside the method.
3131
// For such cases, we may need to pass the evaluation as parameter.
3232
val canDirectAccess: Boolean
33+
34+
def isPrimitive(ctx: CodegenContext): Boolean = javaType.isPrimitive(ctx)
3335
}
3436

3537
object ExprValue {
@@ -51,9 +53,9 @@ object LiteralValue {
5153
// A variable evaluation of [[ExprCode]].
5254
case class VariableValue(
5355
val variableName: String,
54-
val javaType: ExprType,
55-
val canDirectAccess: Boolean = false) extends ExprValue {
56+
val javaType: ExprType) extends ExprValue {
5657
override def toString: String = variableName
58+
override val canDirectAccess: Boolean = false
5759
}
5860

5961
// A statement evaluation of [[ExprCode]].
@@ -70,13 +72,14 @@ case class GlobalValue(val value: String, val javaType: ExprType) extends ExprVa
7072
override val canDirectAccess: Boolean = true
7173
}
7274

73-
case object TrueLiteral extends LiteralValue("true", ExprType("boolean", true))
74-
case object FalseLiteral extends LiteralValue("false", ExprType("boolean", true))
75+
case object TrueLiteral extends LiteralValue("true", ExprType("boolean"))
76+
case object FalseLiteral extends LiteralValue("false", ExprType("boolean"))
7577

7678
// Represents the java type of an evaluation.
77-
case class ExprType(val typeName: String, val isPrimitive: Boolean)
79+
case class ExprType(val typeName: String) {
80+
def isPrimitive(ctx: CodegenContext): Boolean = ctx.isPrimitiveType(typeName)
81+
}
7882

7983
object ExprType {
80-
def apply(ctx: CodegenContext, dataType: DataType): ExprType = ExprType(ctx.javaType(dataType),
81-
ctx.isPrimitiveType(dataType))
84+
def apply(ctx: CodegenContext, dataType: DataType): ExprType = ExprType(ctx.javaType(dataType))
8285
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ object GenerateMutableProjection extends CodeGenerator[Seq[Expression], MutableP
6969
|${ev.code}
7070
|$isNull = ${ev.isNull};
7171
|$value = ${ev.value};
72-
""".stripMargin, GlobalValue(isNull, ExprType(ctx.JAVA_BOOLEAN, true)), value, i)
72+
""".stripMargin, GlobalValue(isNull, ExprType(ctx.JAVA_BOOLEAN)), value, i)
7373
} else {
7474
(s"""
7575
|${ev.code}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ object GenerateSafeProjection extends CodeGenerator[Seq[Expression], Projection]
7575
|final InternalRow $output = new $rowClass($values);
7676
""".stripMargin
7777

78-
ExprCode(code, FalseLiteral, VariableValue(output, ExprType("InternalRow", false)))
78+
ExprCode(code, FalseLiteral, VariableValue(output, ExprType("InternalRow")))
7979
}
8080

8181
private def createCodeForArray(
@@ -106,7 +106,7 @@ object GenerateSafeProjection extends CodeGenerator[Seq[Expression], Projection]
106106
final ArrayData $output = new $arrayClass($values);
107107
"""
108108

109-
ExprCode(code, FalseLiteral, VariableValue(output, ExprType("ArrayData", false)))
109+
ExprCode(code, FalseLiteral, VariableValue(output, ExprType("ArrayData")))
110110
}
111111

112112
private def createCodeForMap(
@@ -127,7 +127,7 @@ object GenerateSafeProjection extends CodeGenerator[Seq[Expression], Projection]
127127
final MapData $output = new $mapClass(${keyConverter.value}, ${valueConverter.value});
128128
"""
129129

130-
ExprCode(code, FalseLiteral, VariableValue(output, ExprType("MapData", false)))
130+
ExprCode(code, FalseLiteral, VariableValue(output, ExprType("MapData")))
131131
}
132132

133133
@tailrec

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ object GenerateUnsafeProjection extends CodeGenerator[Seq[Expression], UnsafePro
5252
// Puts `input` in a local variable to avoid to re-evaluate it if it's a statement.
5353
val tmpInput = ctx.freshName("tmpInput")
5454
val fieldEvals = fieldTypes.zipWithIndex.map { case (dt, i) =>
55-
ExprCode("", StatementValue(s"$tmpInput.isNullAt($i)", ExprType(ctx.JAVA_BOOLEAN, true)),
55+
ExprCode("", StatementValue(s"$tmpInput.isNullAt($i)", ExprType(ctx.JAVA_BOOLEAN)),
5656
StatementValue(ctx.getValue(tmpInput, dt, i.toString), ExprType(ctx, dt)))
5757
}
5858

@@ -348,7 +348,7 @@ object GenerateUnsafeProjection extends CodeGenerator[Seq[Expression], UnsafePro
348348
$writeExpressions
349349
$updateRowSize
350350
"""
351-
ExprCode(code, FalseLiteral, GlobalValue(result, ExprType("UnsafeRow", false)))
351+
ExprCode(code, FalseLiteral, GlobalValue(result, ExprType("UnsafeRow")))
352352
}
353353

354354
protected def canonicalize(in: Seq[Expression]): Seq[Expression] =

‎sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/nullExpressions.scala‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ case class Coalesce(children: Seq[Expression]) extends Expression {
7373

7474
override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
7575
ev.isNull = GlobalValue(ctx.addMutableState(ctx.JAVA_BOOLEAN, ev.isNull),
76-
ExprType(ctx.JAVA_BOOLEAN, true))
76+
ExprType(ctx.JAVA_BOOLEAN))
7777

7878
// all the evals are meant to be in a do { ... } while (false); loop
7979
val evals = children.map { e =>
@@ -323,9 +323,9 @@ case class IsNull(child: Expression) extends UnaryExpression with Predicate {
323323
override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
324324
val eval = child.genCode(ctx)
325325
val value = if (eval.isNull.isInstanceOf[LiteralValue]) {
326-
LiteralValue(eval.isNull, ExprType(ctx.JAVA_BOOLEAN, true))
326+
LiteralValue(eval.isNull, ExprType(ctx.JAVA_BOOLEAN))
327327
} else {
328-
VariableValue(eval.isNull, ExprType(ctx.JAVA_BOOLEAN, true))
328+
VariableValue(eval.isNull, ExprType(ctx.JAVA_BOOLEAN))
329329
}
330330
ExprCode(code = eval.code, isNull = FalseLiteral, value = value)
331331
}

‎sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ trait InvokeLike extends Expression with NonSQLExpression {
6363

6464
val resultIsNull = if (needNullCheck) {
6565
val resultIsNull = ctx.addMutableState(ctx.JAVA_BOOLEAN, "resultIsNull")
66-
GlobalValue(resultIsNull, ExprType(ctx.JAVA_BOOLEAN, true))
66+
GlobalValue(resultIsNull, ExprType(ctx.JAVA_BOOLEAN))
6767
} else {
6868
FalseLiteral
6969
}
@@ -445,7 +445,7 @@ case class LambdaVariable(
445445

446446
override def genCode(ctx: CodegenContext): ExprCode = {
447447
val isNullValue = if (nullable) {
448-
VariableValue(isNull, ExprType(ctx.JAVA_BOOLEAN, true))
448+
VariableValue(isNull, ExprType(ctx.JAVA_BOOLEAN))
449449
} else {
450450
FalseLiteral
451451
}

‎sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/ExprValueSuite.scala‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,17 @@ class ExprValueSuite extends SparkFunSuite {
2828
assert(trueLit.value == "true")
2929
assert(falseLit.value == "false")
3030

31+
val ctx = new CodegenContext()
32+
3133
trueLit match {
3234
case LiteralValue(value, javaType) =>
33-
assert(value == "true" && javaType.typeName == "boolean" && javaType.isPrimitive)
35+
assert(value == "true" && javaType.typeName == "boolean" && javaType.isPrimitive(ctx))
3436
case _ => fail()
3537
}
3638

3739
falseLit match {
3840
case LiteralValue(value, javaType) =>
39-
assert(value == "false" && javaType.typeName == "boolean" && javaType.isPrimitive)
41+
assert(value == "false" && javaType.typeName == "boolean" && javaType.isPrimitive(ctx))
4042
case _ => fail()
4143
}
4244
}

0 commit comments

Comments
 (0)