Skip to content

Commit 408a3ff

Browse files
kiszkgatorsmile
authored andcommitted
[SPARK-25036][SQL] Should compare ExprValue.isNull with LiteralTrue/LiteralFalse
## What changes were proposed in this pull request? This PR fixes a comparison of `ExprValue.isNull` with `String`. `ExprValue.isNull` should be compared with `LiteralTrue` or `LiteralFalse`. This causes the following compilation error using scala-2.12 with sbt. In addition, this code may also generate incorrect code in Spark 2.3. ``` /home/ishizaki/Spark/PR/scala212/spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala:94: org.apache.spark.sql.catalyst.expressions.codegen.ExprValue and String are unrelated: they will most likely always compare unequal [error] [warn] if (eval.isNull != "true") { [error] [warn] [error] [warn] /home/ishizaki/Spark/PR/scala212/spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala:126: org.apache.spark.sql.catalyst.expressions.codegen.ExprValue and String are unrelated: they will most likely never compare equal [error] [warn] if (eval.isNull == "true") { [error] [warn] [error] [warn] /home/ishizaki/Spark/PR/scala212/spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala:133: org.apache.spark.sql.catalyst.expressions.codegen.ExprValue and String are unrelated: they will most likely never compare equal [error] [warn] if (eval.isNull == "true") { [error] [warn] [error] [warn] /home/ishizaki/Spark/PR/scala212/spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateUnsafeProjection.scala:90: org.apache.spark.sql.catalyst.expressions.codegen.ExprValue and String are unrelated: they will most likely never compare equal [error] [warn] if (inputs.map(_.isNull).forall(_ == "false")) { [error] [warn] ``` ## How was this patch tested? Existing UTs Author: Kazuaki Ishizaki <[email protected]> Closes apache#22012 from kiszk/SPARK-25036a.
1 parent 87ca739 commit 408a3ff

File tree

2 files changed

+4
-4
lines changed

2 files changed

+4
-4
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ object GenerateUnsafeProjection extends CodeGenerator[Seq[Expression], UnsafePro
8787
// For top level row writer, it always writes to the beginning of the global buffer holder,
8888
// which means its fixed-size region always in the same position, so we don't need to call
8989
// `reset` to set up its fixed-size region every time.
90-
if (inputs.map(_.isNull).forall(_ == "false")) {
90+
if (inputs.map(_.isNull).forall(_ == FalseLiteral)) {
9191
// If all fields are not nullable, which means the null bits never changes, then we don't
9292
// need to clear it out every time.
9393
""

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ case class ConcatWs(children: Seq[Expression])
9191
val args = ctx.freshName("args")
9292

9393
val inputs = strings.zipWithIndex.map { case (eval, index) =>
94-
if (eval.isNull != "true") {
94+
if (eval.isNull != TrueLiteral) {
9595
s"""
9696
${eval.code}
9797
if (!${eval.isNull}) {
@@ -123,14 +123,14 @@ case class ConcatWs(children: Seq[Expression])
123123
child.dataType match {
124124
case StringType =>
125125
("", // we count all the StringType arguments num at once below.
126-
if (eval.isNull == "true") {
126+
if (eval.isNull == TrueLiteral) {
127127
""
128128
} else {
129129
s"$array[$idxVararg ++] = ${eval.isNull} ? (UTF8String) null : ${eval.value};"
130130
})
131131
case _: ArrayType =>
132132
val size = ctx.freshName("n")
133-
if (eval.isNull == "true") {
133+
if (eval.isNull == TrueLiteral) {
134134
("", "")
135135
} else {
136136
(s"""

0 commit comments

Comments
 (0)