Repository: spark
Updated Branches:
refs/heads/master 87ca7396c -> 408a3ff2c
[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 <ishiz...@jp.ibm.com>
Closes #22012 from kiszk/SPARK-25036a.
Project: http://git-wip-us.apache.org/repos/asf/spark/repo
Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/408a3ff2
Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/408a3ff2
Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/408a3ff2
Branch: refs/heads/master
Commit: 408a3ff2c484fba5734c03dbc570b654dcbc1f23
Parents: 87ca739
Author: Kazuaki Ishizaki <ishiz...@jp.ibm.com>
Authored: Mon Aug 6 19:43:21 2018 -0400
Committer: Xiao Li <gatorsm...@gmail.com>
Committed: Mon Aug 6 19:43:21 2018 -0400
----------------------------------------------------------------------
.../expressions/codegen/GenerateUnsafeProjection.scala | 2 +-
.../spark/sql/catalyst/expressions/stringExpressions.scala | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/spark/blob/408a3ff2/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateUnsafeProjection.scala
----------------------------------------------------------------------
diff --git
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateUnsafeProjection.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateUnsafeProjection.scala
index 8f2a5a0..998a675 100644
---
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateUnsafeProjection.scala
+++
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateUnsafeProjection.scala
@@ -87,7 +87,7 @@ object GenerateUnsafeProjection extends
CodeGenerator[Seq[Expression], UnsafePro
// For top level row writer, it always writes to the beginning of the
global buffer holder,
// which means its fixed-size region always in the same position, so we
don't need to call
// `reset` to set up its fixed-size region every time.
- if (inputs.map(_.isNull).forall(_ == "false")) {
+ if (inputs.map(_.isNull).forall(_ == FalseLiteral)) {
// If all fields are not nullable, which means the null bits never
changes, then we don't
// need to clear it out every time.
""
http://git-wip-us.apache.org/repos/asf/spark/blob/408a3ff2/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
----------------------------------------------------------------------
diff --git
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
index 1838b9f..e1549d3 100755
---
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
+++
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
@@ -91,7 +91,7 @@ case class ConcatWs(children: Seq[Expression])
val args = ctx.freshName("args")
val inputs = strings.zipWithIndex.map { case (eval, index) =>
- if (eval.isNull != "true") {
+ if (eval.isNull != TrueLiteral) {
s"""
${eval.code}
if (!${eval.isNull}) {
@@ -123,14 +123,14 @@ case class ConcatWs(children: Seq[Expression])
child.dataType match {
case StringType =>
("", // we count all the StringType arguments num at once below.
- if (eval.isNull == "true") {
+ if (eval.isNull == TrueLiteral) {
""
} else {
s"$array[$idxVararg ++] = ${eval.isNull} ? (UTF8String) null :
${eval.value};"
})
case _: ArrayType =>
val size = ctx.freshName("n")
- if (eval.isNull == "true") {
+ if (eval.isNull == TrueLiteral) {
("", "")
} else {
(s"""
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org
