From fa8b240ca27976addf0a8d3c46ea9eacb660cbe3 Mon Sep 17 00:00:00 2001 From: angerszhu Date: Sat, 21 Sep 2019 12:19:31 +0800 Subject: [PATCH 01/10] Simplify NOT(isnull(x)) and NOT(isnotnull(x)) --- .../spark/sql/catalyst/optimizer/expressions.scala | 3 +++ .../ReplaceNullWithFalseInPredicateSuite.scala | 14 +++++++++++++- .../scala/org/apache/spark/sql/SQLQuerySuite.scala | 13 +++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala index 39709529c00d3..bf44af8c4d073 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala @@ -556,6 +556,9 @@ object NullPropagation extends Rule[LogicalPlan] { // a null literal. case e: NullIntolerant if e.children.exists(isNullLiteral) => Literal.create(null, e.dataType) + + case n@Not(expr: IsNull) => IsNotNull(expr.child) + case n@Not(expr: IsNotNull) => IsNull(expr.child) } } } diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceNullWithFalseInPredicateSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceNullWithFalseInPredicateSuite.scala index b692c3fee53c7..34fa42e762162 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceNullWithFalseInPredicateSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceNullWithFalseInPredicateSuite.scala @@ -20,7 +20,7 @@ package org.apache.spark.sql.catalyst.optimizer import org.apache.spark.sql.catalyst.analysis.UnresolvedAttribute import org.apache.spark.sql.catalyst.dsl.expressions._ import org.apache.spark.sql.catalyst.dsl.plans._ -import org.apache.spark.sql.catalyst.expressions.{And, ArrayExists, ArrayFilter, ArrayTransform, CaseWhen, Expression, GreaterThan, If, LambdaFunction, Literal, MapFilter, NamedExpression, Or, UnresolvedNamedLambdaVariable} +import org.apache.spark.sql.catalyst.expressions.{And, ArrayExists, ArrayFilter, ArrayTransform, CaseWhen, Expression, GreaterThan, If, IsNotNull, IsNull, LambdaFunction, Literal, MapFilter, NamedExpression, Not, Or, UnresolvedNamedLambdaVariable} import org.apache.spark.sql.catalyst.expressions.Literal.{FalseLiteral, TrueLiteral} import org.apache.spark.sql.catalyst.plans.{Inner, PlanTest} import org.apache.spark.sql.catalyst.plans.logical.{LocalRelation, LogicalPlan} @@ -340,6 +340,18 @@ class ReplaceNullWithFalseInPredicateSuite extends PlanTest { testProjection(originalExpr = column, expectedExpr = column) } + test("NOT isnull(x) -> isnotnull(x)") { + val condition = Not(IsNull(UnresolvedAttribute("i"))) + val predicate = IsNotNull(UnresolvedAttribute("i")) + testFilter(condition, predicate) + } + + test("NOT isNotNull(x) -> isNull(x)") { + val condition = Not(IsNotNull(UnresolvedAttribute("i"))) + val predicate = IsNull(UnresolvedAttribute("i")) + testFilter(condition, predicate) + } + private def testFilter(originalCond: Expression, expectedCond: Expression): Unit = { test((rel, exp) => rel.where(exp), originalCond, expectedCond) } diff --git a/sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala index 80c1e24bfa568..c41c43c44951e 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala @@ -3192,6 +3192,19 @@ class SQLQuerySuite extends QueryTest with SharedSparkSession { checkAnswer(df3, Array(Row(new java.math.BigDecimal("0.100000000000000000000000100")))) } } + + test("SPARK-29152: Not/isNull/isNotNull rewrite") { + withTempView("tbl1") { + val df: DataFrame = + Seq[java.lang.Boolean](true, false, true, null, false, null, true).toDF("id") + df.createOrReplaceTempView("tbl1") + val query1 = sql("select id from tbl1 where not(isnull(id) or id == false)") + val query2 = sql("select id from tbl1 where not(isnotnull(id) and id == true) ") + + checkAnswer(query1, Row(true) :: Row(true) :: Row(true) :: Nil) + checkAnswer(query2, Row(false) :: Row(null) :: Row(false) :: Row(null) :: Nil) + } + } } case class Foo(bar: Option[String]) From a701dca35fd8141e59ab8d3bd8f76dca89bf0365 Mon Sep 17 00:00:00 2001 From: angerszhu Date: Sat, 21 Sep 2019 16:22:37 +0800 Subject: [PATCH 02/10] fix code style --- .../org/apache/spark/sql/catalyst/optimizer/expressions.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala index bf44af8c4d073..4596922b4c975 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala @@ -557,8 +557,8 @@ object NullPropagation extends Rule[LogicalPlan] { case e: NullIntolerant if e.children.exists(isNullLiteral) => Literal.create(null, e.dataType) - case n@Not(expr: IsNull) => IsNotNull(expr.child) - case n@Not(expr: IsNotNull) => IsNull(expr.child) + case n @ Not(expr: IsNull) => IsNotNull(expr.child) + case n @ Not(expr: IsNotNull) => IsNull(expr.child) } } } From 978a28481447357cd7f006eb5d69aa21e69cf8b4 Mon Sep 17 00:00:00 2001 From: angerszhu Date: Sat, 21 Sep 2019 17:51:26 +0800 Subject: [PATCH 03/10] move optimize place --- .../spark/sql/catalyst/optimizer/expressions.scala | 6 +++--- .../ReplaceNullWithFalseInPredicateSuite.scala | 12 ------------ .../optimizer/SimplifyConditionalSuite.scala | 9 +++++++++ .../scala/org/apache/spark/sql/SQLQuerySuite.scala | 2 +- 4 files changed, 13 insertions(+), 16 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala index 4596922b4c975..a6b3c2a25694a 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala @@ -461,6 +461,9 @@ object SimplifyConditionals extends Rule[LogicalPlan] with PredicateHelper { } else { e.copy(branches = branches.take(i).map(branch => (branch._1, elseValue))) } + + case _ @ Not(expr: IsNull) => IsNotNull(expr.child) + case _ @ Not(expr: IsNotNull) => IsNull(expr.child) } } } @@ -556,9 +559,6 @@ object NullPropagation extends Rule[LogicalPlan] { // a null literal. case e: NullIntolerant if e.children.exists(isNullLiteral) => Literal.create(null, e.dataType) - - case n @ Not(expr: IsNull) => IsNotNull(expr.child) - case n @ Not(expr: IsNotNull) => IsNull(expr.child) } } } diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceNullWithFalseInPredicateSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceNullWithFalseInPredicateSuite.scala index 34fa42e762162..a7366e494f102 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceNullWithFalseInPredicateSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceNullWithFalseInPredicateSuite.scala @@ -340,18 +340,6 @@ class ReplaceNullWithFalseInPredicateSuite extends PlanTest { testProjection(originalExpr = column, expectedExpr = column) } - test("NOT isnull(x) -> isnotnull(x)") { - val condition = Not(IsNull(UnresolvedAttribute("i"))) - val predicate = IsNotNull(UnresolvedAttribute("i")) - testFilter(condition, predicate) - } - - test("NOT isNotNull(x) -> isNull(x)") { - val condition = Not(IsNotNull(UnresolvedAttribute("i"))) - val predicate = IsNull(UnresolvedAttribute("i")) - testFilter(condition, predicate) - } - private def testFilter(originalCond: Expression, expectedCond: Expression): Unit = { test((rel, exp) => rel.where(exp), originalCond, expectedCond) } diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala index 8ad7c12020b82..bf47c36aae40d 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala @@ -166,4 +166,13 @@ class SimplifyConditionalSuite extends PlanTest with PredicateHelper { Literal(1)) ) } + + test("simplify NOT(IsNull(x)) and NOT(IsNotNull(x))") { + val condition1 = Not(IsNull(UnresolvedAttribute("i"))) + val predicate1 = IsNotNull(UnresolvedAttribute("i")) + val condition2 = Not(IsNotNull(UnresolvedAttribute("i"))) + val predicate2 = IsNull(UnresolvedAttribute("i")) + assertEquivalent(condition1, predicate1) + assertEquivalent(condition2, predicate2) + } } diff --git a/sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala index c41c43c44951e..62301b6947683 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala @@ -3193,7 +3193,7 @@ class SQLQuerySuite extends QueryTest with SharedSparkSession { } } - test("SPARK-29152: Not/isNull/isNotNull rewrite") { + test("SPARK-29152: Simplify NOT(IsNull(x)) and NOT(IsNotNull(x))") { withTempView("tbl1") { val df: DataFrame = Seq[java.lang.Boolean](true, false, true, null, false, null, true).toDF("id") From f1395586ca83058a20f4407b49909e3433dd655f Mon Sep 17 00:00:00 2001 From: angerszhu Date: Sat, 21 Sep 2019 18:27:44 +0800 Subject: [PATCH 04/10] change test method --- .../sql/catalyst/optimizer/expressions.scala | 4 ++-- ...ReplaceNullWithFalseInPredicateSuite.scala | 2 +- .../optimizer/SimplifyConditionalSuite.scala | 19 +++++++++++++------ 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala index a6b3c2a25694a..25ed318d35101 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala @@ -462,8 +462,8 @@ object SimplifyConditionals extends Rule[LogicalPlan] with PredicateHelper { e.copy(branches = branches.take(i).map(branch => (branch._1, elseValue))) } - case _ @ Not(expr: IsNull) => IsNotNull(expr.child) - case _ @ Not(expr: IsNotNull) => IsNull(expr.child) + case n @ Not(expr: IsNull) => IsNotNull(expr.child) + case n @ Not(expr: IsNotNull) => IsNull(expr.child) } } } diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceNullWithFalseInPredicateSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceNullWithFalseInPredicateSuite.scala index a7366e494f102..552fd41e0542a 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceNullWithFalseInPredicateSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceNullWithFalseInPredicateSuite.scala @@ -20,7 +20,7 @@ package org.apache.spark.sql.catalyst.optimizer import org.apache.spark.sql.catalyst.analysis.UnresolvedAttribute import org.apache.spark.sql.catalyst.dsl.expressions._ import org.apache.spark.sql.catalyst.dsl.plans._ -import org.apache.spark.sql.catalyst.expressions.{And, ArrayExists, ArrayFilter, ArrayTransform, CaseWhen, Expression, GreaterThan, If, IsNotNull, IsNull, LambdaFunction, Literal, MapFilter, NamedExpression, Not, Or, UnresolvedNamedLambdaVariable} +import org.apache.spark.sql.catalyst.expressions.{And, ArrayExists, ArrayFilter, ArrayTransform, CaseWhen, Expression, GreaterThan, If, LambdaFunction, Literal, MapFilter, NamedExpression, Not, Or, UnresolvedNamedLambdaVariable} import org.apache.spark.sql.catalyst.expressions.Literal.{FalseLiteral, TrueLiteral} import org.apache.spark.sql.catalyst.plans.{Inner, PlanTest} import org.apache.spark.sql.catalyst.plans.logical.{LocalRelation, LogicalPlan} diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala index bf47c36aae40d..5a60074041c56 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala @@ -18,6 +18,7 @@ package org.apache.spark.sql.catalyst.optimizer import org.apache.spark.sql.catalyst.analysis.UnresolvedAttribute +import org.apache.spark.sql.catalyst.dsl.expressions._ import org.apache.spark.sql.catalyst.dsl.plans._ import org.apache.spark.sql.catalyst.expressions._ import org.apache.spark.sql.catalyst.expressions.Literal.{FalseLiteral, TrueLiteral} @@ -40,10 +41,20 @@ class SimplifyConditionalSuite extends PlanTest with PredicateHelper { comparePlans(actual, correctAnswer) } + private def assertFilter(originalExpr: Expression, + expectedExpr: Expression): Unit = { + val originalPlan = testRelation.where(originalExpr).analyze + val optimizedPlan = Optimize.execute(originalPlan) + val expectedPlan = testRelation.where(expectedExpr).analyze + comparePlans(optimizedPlan, expectedPlan) + } + private val trueBranch = (TrueLiteral, Literal(5)) private val normalBranch = (NonFoldableLiteral(true), Literal(10)) private val unreachableBranch = (FalseLiteral, Literal(20)) private val nullBranch = (Literal.create(null, NullType), Literal(30)) + private val testRelation = + LocalRelation('i.int, 'b.boolean, 'a.array(IntegerType), 'm.map(IntegerType, IntegerType)) val isNotNullCond = IsNotNull(UnresolvedAttribute(Seq("a"))) val isNullCond = IsNull(UnresolvedAttribute("b")) @@ -168,11 +179,7 @@ class SimplifyConditionalSuite extends PlanTest with PredicateHelper { } test("simplify NOT(IsNull(x)) and NOT(IsNotNull(x))") { - val condition1 = Not(IsNull(UnresolvedAttribute("i"))) - val predicate1 = IsNotNull(UnresolvedAttribute("i")) - val condition2 = Not(IsNotNull(UnresolvedAttribute("i"))) - val predicate2 = IsNull(UnresolvedAttribute("i")) - assertEquivalent(condition1, predicate1) - assertEquivalent(condition2, predicate2) + assertFilter(Not(IsNotNull(UnresolvedAttribute("b"))), IsNull(UnresolvedAttribute("b"))) + assertFilter(Not(IsNull(UnresolvedAttribute("b"))), IsNotNull(UnresolvedAttribute("b"))) } } From 9bf2d7cc523f78274dcaa5a72fc6b24d4616b2d8 Mon Sep 17 00:00:00 2001 From: angerszhu Date: Sat, 21 Sep 2019 18:29:00 +0800 Subject: [PATCH 05/10] remove unused import --- .../optimizer/ReplaceNullWithFalseInPredicateSuite.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceNullWithFalseInPredicateSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceNullWithFalseInPredicateSuite.scala index 552fd41e0542a..b692c3fee53c7 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceNullWithFalseInPredicateSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceNullWithFalseInPredicateSuite.scala @@ -20,7 +20,7 @@ package org.apache.spark.sql.catalyst.optimizer import org.apache.spark.sql.catalyst.analysis.UnresolvedAttribute import org.apache.spark.sql.catalyst.dsl.expressions._ import org.apache.spark.sql.catalyst.dsl.plans._ -import org.apache.spark.sql.catalyst.expressions.{And, ArrayExists, ArrayFilter, ArrayTransform, CaseWhen, Expression, GreaterThan, If, LambdaFunction, Literal, MapFilter, NamedExpression, Not, Or, UnresolvedNamedLambdaVariable} +import org.apache.spark.sql.catalyst.expressions.{And, ArrayExists, ArrayFilter, ArrayTransform, CaseWhen, Expression, GreaterThan, If, LambdaFunction, Literal, MapFilter, NamedExpression, Or, UnresolvedNamedLambdaVariable} import org.apache.spark.sql.catalyst.expressions.Literal.{FalseLiteral, TrueLiteral} import org.apache.spark.sql.catalyst.plans.{Inner, PlanTest} import org.apache.spark.sql.catalyst.plans.logical.{LocalRelation, LogicalPlan} From a75c2ef33b3b644ed309e4efd2bf123a5c7350d0 Mon Sep 17 00:00:00 2001 From: angerszhu Date: Sun, 22 Sep 2019 08:13:54 +0800 Subject: [PATCH 06/10] follow comment --- .../apache/spark/sql/catalyst/optimizer/expressions.scala | 6 +++--- .../sql/catalyst/optimizer/SimplifyConditionalSuite.scala | 3 +-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala index 25ed318d35101..fea6d5ef2a4c0 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala @@ -373,6 +373,9 @@ object BooleanSimplification extends Rule[LogicalPlan] with PredicateHelper { case Not(a And b) => Or(Not(a), Not(b)) case Not(Not(e)) => e + + case Not(expr: IsNull) => IsNotNull(expr.child) + case Not(expr: IsNotNull) => IsNull(expr.child) } } } @@ -461,9 +464,6 @@ object SimplifyConditionals extends Rule[LogicalPlan] with PredicateHelper { } else { e.copy(branches = branches.take(i).map(branch => (branch._1, elseValue))) } - - case n @ Not(expr: IsNull) => IsNotNull(expr.child) - case n @ Not(expr: IsNotNull) => IsNull(expr.child) } } } diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala index 5a60074041c56..86b8ddbae779b 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala @@ -41,8 +41,7 @@ class SimplifyConditionalSuite extends PlanTest with PredicateHelper { comparePlans(actual, correctAnswer) } - private def assertFilter(originalExpr: Expression, - expectedExpr: Expression): Unit = { + private def assertFilter(originalExpr: Expression, expectedExpr: Expression): Unit = { val originalPlan = testRelation.where(originalExpr).analyze val optimizedPlan = Optimize.execute(originalPlan) val expectedPlan = testRelation.where(expectedExpr).analyze From 1fdea4869a6f2a91b899374707306ad37e4f4683 Mon Sep 17 00:00:00 2001 From: angerszhu Date: Sun, 22 Sep 2019 08:52:50 +0800 Subject: [PATCH 07/10] fix UT place --- .../optimizer/BooleanSimplificationSuite.scala | 5 +++++ .../optimizer/SimplifyConditionalSuite.scala | 14 -------------- .../scala/org/apache/spark/sql/SQLQuerySuite.scala | 13 ------------- 3 files changed, 5 insertions(+), 27 deletions(-) diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/BooleanSimplificationSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/BooleanSimplificationSuite.scala index a0de5f6930958..62e4ed14bb5f4 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/BooleanSimplificationSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/BooleanSimplificationSuite.scala @@ -239,6 +239,11 @@ class BooleanSimplificationSuite extends PlanTest with ExpressionEvalHelper with checkCondition(!'f || 'e, testRelationWithData.where(!'f || 'e).analyze) } + test("simplify NOT(IsNull(x)) and NOT(IsNotNull(x))") { + checkCondition(Not(IsNotNull(UnresolvedAttribute("b"))), IsNull(UnresolvedAttribute("b"))) + checkCondition(Not(IsNull(UnresolvedAttribute("b"))), IsNotNull(UnresolvedAttribute("b"))) + } + protected def assertEquivalent(e1: Expression, e2: Expression): Unit = { val correctAnswer = Project(Alias(e2, "out")() :: Nil, OneRowRelation()).analyze val actual = Optimize.execute(Project(Alias(e1, "out")() :: Nil, OneRowRelation()).analyze) diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala index 86b8ddbae779b..04a06d992bfb6 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala @@ -41,19 +41,10 @@ class SimplifyConditionalSuite extends PlanTest with PredicateHelper { comparePlans(actual, correctAnswer) } - private def assertFilter(originalExpr: Expression, expectedExpr: Expression): Unit = { - val originalPlan = testRelation.where(originalExpr).analyze - val optimizedPlan = Optimize.execute(originalPlan) - val expectedPlan = testRelation.where(expectedExpr).analyze - comparePlans(optimizedPlan, expectedPlan) - } - private val trueBranch = (TrueLiteral, Literal(5)) private val normalBranch = (NonFoldableLiteral(true), Literal(10)) private val unreachableBranch = (FalseLiteral, Literal(20)) private val nullBranch = (Literal.create(null, NullType), Literal(30)) - private val testRelation = - LocalRelation('i.int, 'b.boolean, 'a.array(IntegerType), 'm.map(IntegerType, IntegerType)) val isNotNullCond = IsNotNull(UnresolvedAttribute(Seq("a"))) val isNullCond = IsNull(UnresolvedAttribute("b")) @@ -176,9 +167,4 @@ class SimplifyConditionalSuite extends PlanTest with PredicateHelper { Literal(1)) ) } - - test("simplify NOT(IsNull(x)) and NOT(IsNotNull(x))") { - assertFilter(Not(IsNotNull(UnresolvedAttribute("b"))), IsNull(UnresolvedAttribute("b"))) - assertFilter(Not(IsNull(UnresolvedAttribute("b"))), IsNotNull(UnresolvedAttribute("b"))) - } } diff --git a/sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala index 62301b6947683..80c1e24bfa568 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala @@ -3192,19 +3192,6 @@ class SQLQuerySuite extends QueryTest with SharedSparkSession { checkAnswer(df3, Array(Row(new java.math.BigDecimal("0.100000000000000000000000100")))) } } - - test("SPARK-29152: Simplify NOT(IsNull(x)) and NOT(IsNotNull(x))") { - withTempView("tbl1") { - val df: DataFrame = - Seq[java.lang.Boolean](true, false, true, null, false, null, true).toDF("id") - df.createOrReplaceTempView("tbl1") - val query1 = sql("select id from tbl1 where not(isnull(id) or id == false)") - val query2 = sql("select id from tbl1 where not(isnotnull(id) and id == true) ") - - checkAnswer(query1, Row(true) :: Row(true) :: Row(true) :: Nil) - checkAnswer(query2, Row(false) :: Row(null) :: Row(false) :: Row(null) :: Nil) - } - } } case class Foo(bar: Option[String]) From cb9cef2515342989a373ef7f2591c1baa12566e4 Mon Sep 17 00:00:00 2001 From: angerszhu Date: Sun, 22 Sep 2019 08:53:31 +0800 Subject: [PATCH 08/10] remove unused import --- .../spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala index 04a06d992bfb6..8ad7c12020b82 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala @@ -18,7 +18,6 @@ package org.apache.spark.sql.catalyst.optimizer import org.apache.spark.sql.catalyst.analysis.UnresolvedAttribute -import org.apache.spark.sql.catalyst.dsl.expressions._ import org.apache.spark.sql.catalyst.dsl.plans._ import org.apache.spark.sql.catalyst.expressions._ import org.apache.spark.sql.catalyst.expressions.Literal.{FalseLiteral, TrueLiteral} From 9f5defa1dfb6f0526c6fc9157db2a9a326b8327b Mon Sep 17 00:00:00 2001 From: angerszhu Date: Sun, 22 Sep 2019 12:00:36 +0800 Subject: [PATCH 09/10] follow comment --- .../org/apache/spark/sql/catalyst/optimizer/expressions.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala index fea6d5ef2a4c0..3e5c5235b2514 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala @@ -374,8 +374,8 @@ object BooleanSimplification extends Rule[LogicalPlan] with PredicateHelper { case Not(Not(e)) => e - case Not(expr: IsNull) => IsNotNull(expr.child) - case Not(expr: IsNotNull) => IsNull(expr.child) + case Not(e: IsNull) => IsNotNull(e.child) + case Not(e: IsNotNull) => IsNull(e.child) } } } From f71698df1c5e2a88bad35b7183f3b18baf722399 Mon Sep 17 00:00:00 2001 From: angerszhu Date: Sun, 22 Sep 2019 12:16:04 +0800 Subject: [PATCH 10/10] for better code style --- .../org/apache/spark/sql/catalyst/optimizer/expressions.scala | 4 ++-- .../sql/catalyst/optimizer/BooleanSimplificationSuite.scala | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala index 3e5c5235b2514..0a6737ba42118 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala @@ -374,8 +374,8 @@ object BooleanSimplification extends Rule[LogicalPlan] with PredicateHelper { case Not(Not(e)) => e - case Not(e: IsNull) => IsNotNull(e.child) - case Not(e: IsNotNull) => IsNull(e.child) + case Not(IsNull(e)) => IsNotNull(e) + case Not(IsNotNull(e)) => IsNull(e) } } } diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/BooleanSimplificationSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/BooleanSimplificationSuite.scala index 62e4ed14bb5f4..a8b8417754b00 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/BooleanSimplificationSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/BooleanSimplificationSuite.scala @@ -240,8 +240,8 @@ class BooleanSimplificationSuite extends PlanTest with ExpressionEvalHelper with } test("simplify NOT(IsNull(x)) and NOT(IsNotNull(x))") { - checkCondition(Not(IsNotNull(UnresolvedAttribute("b"))), IsNull(UnresolvedAttribute("b"))) - checkCondition(Not(IsNull(UnresolvedAttribute("b"))), IsNotNull(UnresolvedAttribute("b"))) + checkCondition(Not(IsNotNull('b)), IsNull('b)) + checkCondition(Not(IsNull('b)), IsNotNull('b)) } protected def assertEquivalent(e1: Expression, e2: Expression): Unit = {