Skip to content

Commit

Permalink
[SPARK-18585][SQL] Use ev.isNull = "false" if possible for Janino t…
Browse files Browse the repository at this point in the history
…o have a chance to optimize.

## What changes were proposed in this pull request?

Janino can optimize `true ? a : b` into `a` or `false ? a : b` into `b`, or if/else with literal condition, so we should use literal as `ev.isNull` if possible.

## How was this patch tested?

Existing tests.

Author: Takuya UESHIN <[email protected]>

Closes apache#16008 from ueshin/issues/SPARK-18585.
  • Loading branch information
ueshin authored and rxin committed Nov 28, 2016
1 parent fc2c13b commit 8714162
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ case class CreateArray(children: Seq[Expression]) extends Expression {
ctx.addMutableState("Object[]", values, s"this.$values = null;")

ev.copy(code = s"""
final boolean ${ev.isNull} = false;
this.$values = new Object[${children.size}];""" +
ctx.splitExpressions(
ctx.INPUT_ROW,
Expand All @@ -78,7 +77,7 @@ case class CreateArray(children: Seq[Expression]) extends Expression {
s"""
final ArrayData ${ev.value} = new $arrayClass($values);
this.$values = null;
""")
""", isNull = "false")
}

override def prettyName: String = "array"
Expand Down Expand Up @@ -144,7 +143,6 @@ case class CreateMap(children: Seq[Expression]) extends Expression {
val keyData = s"new $arrayClass($keyArray)"
val valueData = s"new $arrayClass($valueArray)"
ev.copy(code = s"""
final boolean ${ev.isNull} = false;
$keyArray = new Object[${keys.size}];
$valueArray = new Object[${values.size}];""" +
ctx.splitExpressions(
Expand Down Expand Up @@ -177,7 +175,7 @@ case class CreateMap(children: Seq[Expression]) extends Expression {
final MapData ${ev.value} = new $mapClass($keyData, $valueData);
this.$keyArray = null;
this.$valueArray = null;
""")
""", isNull = "false")
}

override def prettyName: String = "map"
Expand Down Expand Up @@ -301,7 +299,6 @@ case class CreateNamedStruct(children: Seq[Expression]) extends CreateNamedStruc
ctx.addMutableState("Object[]", values, s"this.$values = null;")

ev.copy(code = s"""
boolean ${ev.isNull} = false;
$values = new Object[${valExprs.size}];""" +
ctx.splitExpressions(
ctx.INPUT_ROW,
Expand All @@ -317,7 +314,7 @@ case class CreateNamedStruct(children: Seq[Expression]) extends CreateNamedStruc
s"""
final InternalRow ${ev.value} = new $rowClass($values);
this.$values = null;
""")
""", isNull = "false")
}

override def prettyName: String = "named_struct"
Expand All @@ -333,7 +330,7 @@ case class CreateNamedStruct(children: Seq[Expression]) extends CreateNamedStruc
case class CreateNamedStructUnsafe(children: Seq[Expression]) extends CreateNamedStructLike {
override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
val eval = GenerateUnsafeProjection.createCode(ctx, valExprs)
ExprCode(code = eval.code, isNull = eval.isNull, value = eval.value)
ExprCode(code = eval.code, isNull = "false", value = eval.value)
}

override def prettyName: String = "named_struct_unsafe"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,8 @@ case class IsNaN(child: Expression) extends UnaryExpression
case DoubleType | FloatType =>
ev.copy(code = s"""
${eval.code}
boolean ${ev.isNull} = false;
${ctx.javaType(dataType)} ${ev.value} = ${ctx.defaultValue(dataType)};
${ev.value} = !${eval.isNull} && Double.isNaN(${eval.value});""")
${ev.value} = !${eval.isNull} && Double.isNaN(${eval.value});""", isNull = "false")
}
}
}
Expand Down Expand Up @@ -383,7 +382,6 @@ case class AtLeastNNonNulls(n: Int, children: Seq[Expression]) extends Predicate
ev.copy(code = s"""
int $nonnull = 0;
$code
boolean ${ev.isNull} = false;
boolean ${ev.value} = $nonnull >= $n;""")
boolean ${ev.value} = $nonnull >= $n;""", isNull = "false")
}
}

0 comments on commit 8714162

Please sign in to comment.