spark git commit: [SPARK-24007][SQL] EqualNullSafe for FloatType and DoubleType might generate a wrong result by codegen.

2018-04-18 Thread lixiao
Repository: spark
Updated Branches:
  refs/heads/branch-2.2 e957c4e88 -> a902323fb


[SPARK-24007][SQL] EqualNullSafe for FloatType and DoubleType might generate a 
wrong result by codegen.

`EqualNullSafe` for `FloatType` and `DoubleType` might generate a wrong result 
by codegen.

```scala
scala> val df = Seq((Some(-1.0d), None), (None, Some(-1.0d))).toDF()
df: org.apache.spark.sql.DataFrame = [_1: double, _2: double]

scala> df.show()
+++
|  _1|  _2|
+++
|-1.0|null|
|null|-1.0|
+++

scala> df.filter("_1 <=> _2").show()
+++
|  _1|  _2|
+++
|-1.0|null|
|null|-1.0|
+++
```

The result should be empty but the result remains two rows.

Added a test.

Author: Takuya UESHIN 

Closes #21094 from ueshin/issues/SPARK-24007/equalnullsafe.

(cherry picked from commit f09a9e9418c1697d198de18f340b1288f5eb025c)
Signed-off-by: gatorsmile 


Project: http://git-wip-us.apache.org/repos/asf/spark/repo
Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/a902323f
Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/a902323f
Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/a902323f

Branch: refs/heads/branch-2.2
Commit: a902323fbf7be27a7ca747105eedd61b1d57b9d4
Parents: e957c4e
Author: Takuya UESHIN 
Authored: Wed Apr 18 08:22:05 2018 -0700
Committer: gatorsmile 
Committed: Wed Apr 18 08:23:46 2018 -0700

--
 .../sql/catalyst/expressions/codegen/CodeGenerator.scala  | 6 --
 .../spark/sql/catalyst/expressions/PredicateSuite.scala   | 7 +++
 2 files changed, 11 insertions(+), 2 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/spark/blob/a902323f/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala
--
diff --git 
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala
 
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala
index 3964471..9e5eaf6 100644
--- 
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala
+++ 
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala
@@ -482,8 +482,10 @@ class CodegenContext {
*/
   def genEqual(dataType: DataType, c1: String, c2: String): String = dataType 
match {
 case BinaryType => s"java.util.Arrays.equals($c1, $c2)"
-case FloatType => s"(java.lang.Float.isNaN($c1) && 
java.lang.Float.isNaN($c2)) || $c1 == $c2"
-case DoubleType => s"(java.lang.Double.isNaN($c1) && 
java.lang.Double.isNaN($c2)) || $c1 == $c2"
+case FloatType =>
+  s"((java.lang.Float.isNaN($c1) && java.lang.Float.isNaN($c2)) || $c1 == 
$c2)"
+case DoubleType =>
+  s"((java.lang.Double.isNaN($c1) && java.lang.Double.isNaN($c2)) || $c1 
== $c2)"
 case dt: DataType if isPrimitiveType(dt) => s"$c1 == $c2"
 case dt: DataType if dt.isInstanceOf[AtomicType] => s"$c1.equals($c2)"
 case array: ArrayType => genComp(array, c1, c2) + " == 0"

http://git-wip-us.apache.org/repos/asf/spark/blob/a902323f/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/PredicateSuite.scala
--
diff --git 
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/PredicateSuite.scala
 
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/PredicateSuite.scala
index bf3b184..15ae624 100644
--- 
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/PredicateSuite.scala
+++ 
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/PredicateSuite.scala
@@ -340,4 +340,11 @@ class PredicateSuite extends SparkFunSuite with 
ExpressionEvalHelper {
 val infinity = Literal(Double.PositiveInfinity)
 checkEvaluation(EqualTo(infinity, infinity), true)
   }
+
+  test("SPARK-24007: EqualNullSafe for FloatType and DoubleType might generate 
a wrong result") {
+checkEvaluation(EqualNullSafe(Literal(null, FloatType), Literal(-1.0f)), 
false)
+checkEvaluation(EqualNullSafe(Literal(-1.0f), Literal(null, FloatType)), 
false)
+checkEvaluation(EqualNullSafe(Literal(null, DoubleType), Literal(-1.0d)), 
false)
+checkEvaluation(EqualNullSafe(Literal(-1.0d), Literal(null, DoubleType)), 
false)
+  }
 }


-
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org



spark git commit: [SPARK-24007][SQL] EqualNullSafe for FloatType and DoubleType might generate a wrong result by codegen.

2018-04-18 Thread lixiao
Repository: spark
Updated Branches:
  refs/heads/master f81fa478f -> f09a9e941


[SPARK-24007][SQL] EqualNullSafe for FloatType and DoubleType might generate a 
wrong result by codegen.

## What changes were proposed in this pull request?

`EqualNullSafe` for `FloatType` and `DoubleType` might generate a wrong result 
by codegen.

```scala
scala> val df = Seq((Some(-1.0d), None), (None, Some(-1.0d))).toDF()
df: org.apache.spark.sql.DataFrame = [_1: double, _2: double]

scala> df.show()
+++
|  _1|  _2|
+++
|-1.0|null|
|null|-1.0|
+++

scala> df.filter("_1 <=> _2").show()
+++
|  _1|  _2|
+++
|-1.0|null|
|null|-1.0|
+++
```

The result should be empty but the result remains two rows.

## How was this patch tested?

Added a test.

Author: Takuya UESHIN 

Closes #21094 from ueshin/issues/SPARK-24007/equalnullsafe.


Project: http://git-wip-us.apache.org/repos/asf/spark/repo
Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/f09a9e94
Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/f09a9e94
Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/f09a9e94

Branch: refs/heads/master
Commit: f09a9e9418c1697d198de18f340b1288f5eb025c
Parents: f81fa47
Author: Takuya UESHIN 
Authored: Wed Apr 18 08:22:05 2018 -0700
Committer: gatorsmile 
Committed: Wed Apr 18 08:22:05 2018 -0700

--
 .../sql/catalyst/expressions/codegen/CodeGenerator.scala  | 6 --
 .../spark/sql/catalyst/expressions/PredicateSuite.scala   | 7 +++
 2 files changed, 11 insertions(+), 2 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/spark/blob/f09a9e94/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala
--
diff --git 
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala
 
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala
index f6b6775..cf0a91f 100644
--- 
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala
+++ 
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala
@@ -582,8 +582,10 @@ class CodegenContext {
*/
   def genEqual(dataType: DataType, c1: String, c2: String): String = dataType 
match {
 case BinaryType => s"java.util.Arrays.equals($c1, $c2)"
-case FloatType => s"(java.lang.Float.isNaN($c1) && 
java.lang.Float.isNaN($c2)) || $c1 == $c2"
-case DoubleType => s"(java.lang.Double.isNaN($c1) && 
java.lang.Double.isNaN($c2)) || $c1 == $c2"
+case FloatType =>
+  s"((java.lang.Float.isNaN($c1) && java.lang.Float.isNaN($c2)) || $c1 == 
$c2)"
+case DoubleType =>
+  s"((java.lang.Double.isNaN($c1) && java.lang.Double.isNaN($c2)) || $c1 
== $c2)"
 case dt: DataType if isPrimitiveType(dt) => s"$c1 == $c2"
 case dt: DataType if dt.isInstanceOf[AtomicType] => s"$c1.equals($c2)"
 case array: ArrayType => genComp(array, c1, c2) + " == 0"

http://git-wip-us.apache.org/repos/asf/spark/blob/f09a9e94/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/PredicateSuite.scala
--
diff --git 
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/PredicateSuite.scala
 
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/PredicateSuite.scala
index 8a8f8e1..1bfd180 100644
--- 
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/PredicateSuite.scala
+++ 
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/PredicateSuite.scala
@@ -442,4 +442,11 @@ class PredicateSuite extends SparkFunSuite with 
ExpressionEvalHelper {
 InSet(Literal(1), Set(1, 2, 3, 4)).genCode(ctx)
 assert(ctx.inlinedMutableStates.isEmpty)
   }
+
+  test("SPARK-24007: EqualNullSafe for FloatType and DoubleType might generate 
a wrong result") {
+checkEvaluation(EqualNullSafe(Literal(null, FloatType), Literal(-1.0f)), 
false)
+checkEvaluation(EqualNullSafe(Literal(-1.0f), Literal(null, FloatType)), 
false)
+checkEvaluation(EqualNullSafe(Literal(null, DoubleType), Literal(-1.0d)), 
false)
+checkEvaluation(EqualNullSafe(Literal(-1.0d), Literal(null, DoubleType)), 
false)
+  }
 }


-
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org