From b16b2582cb29ea8eb0530b4643fb9f7065fe3556 Mon Sep 17 00:00:00 2001 From: beliefer Date: Wed, 26 Feb 2020 10:04:01 +0800 Subject: [PATCH 1/3] Improve boolean test --- .../sql/catalyst/expressions/predicates.scala | 60 ------------------- .../sql/catalyst/parser/AstBuilder.scala | 8 +-- 2 files changed, 4 insertions(+), 64 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala index bcd442ad3cc35..bd190c3e5abc7 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala @@ -927,66 +927,6 @@ case class GreaterThanOrEqual(left: Expression, right: Expression) protected override def nullSafeEval(input1: Any, input2: Any): Any = ordering.gteq(input1, input2) } -trait BooleanTest extends UnaryExpression with Predicate with ExpectsInputTypes { - - def boolValueForComparison: Boolean - def boolValueWhenNull: Boolean - - override def nullable: Boolean = false - override def inputTypes: Seq[DataType] = Seq(BooleanType) - - override def eval(input: InternalRow): Any = { - val value = child.eval(input) - Option(value) match { - case None => boolValueWhenNull - case other => if (boolValueWhenNull) { - value == !boolValueForComparison - } else { - value == boolValueForComparison - } - } - } - - override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = { - val eval = child.genCode(ctx) - ev.copy(code = code""" - ${eval.code} - ${CodeGenerator.javaType(dataType)} ${ev.value} = ${CodeGenerator.defaultValue(dataType)}; - if (${eval.isNull}) { - ${ev.value} = $boolValueWhenNull; - } else if ($boolValueWhenNull) { - ${ev.value} = ${eval.value} == !$boolValueForComparison; - } else { - ${ev.value} = ${eval.value} == $boolValueForComparison; - } - """, isNull = FalseLiteral) - } -} - -case class IsTrue(child: Expression) extends BooleanTest { - override def boolValueForComparison: Boolean = true - override def boolValueWhenNull: Boolean = false - override def sql: String = s"(${child.sql} IS TRUE)" -} - -case class IsNotTrue(child: Expression) extends BooleanTest { - override def boolValueForComparison: Boolean = true - override def boolValueWhenNull: Boolean = true - override def sql: String = s"(${child.sql} IS NOT TRUE)" -} - -case class IsFalse(child: Expression) extends BooleanTest { - override def boolValueForComparison: Boolean = false - override def boolValueWhenNull: Boolean = false - override def sql: String = s"(${child.sql} IS FALSE)" -} - -case class IsNotFalse(child: Expression) extends BooleanTest { - override def boolValueForComparison: Boolean = false - override def boolValueWhenNull: Boolean = true - override def sql: String = s"(${child.sql} IS NOT FALSE)" -} - /** * IS UNKNOWN and IS NOT UNKNOWN are the same as IS NULL and IS NOT NULL, respectively, * except that the input expression must be of a boolean type. diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala index 883128a8a2b06..30b278b7c52ca 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala @@ -1414,12 +1414,12 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging case SqlBaseParser.NULL => IsNull(e) case SqlBaseParser.TRUE => ctx.NOT match { - case null => IsTrue(e) - case _ => IsNotTrue(e) + case null => EqualNullSafe(e, Literal(true)) + case _ => Not(EqualNullSafe(e, Literal(true))) } case SqlBaseParser.FALSE => ctx.NOT match { - case null => IsFalse(e) - case _ => IsNotFalse(e) + case null => EqualNullSafe(e, Literal(false)) + case _ => Not(EqualNullSafe(e, Literal(false))) } case SqlBaseParser.UNKNOWN => ctx.NOT match { case null => IsUnknown(e) From f14632275c5a2b21032a62138c9ab5d07e4efc2b Mon Sep 17 00:00:00 2001 From: beliefer Date: Wed, 26 Feb 2020 10:26:12 +0800 Subject: [PATCH 2/3] Remove UT --- .../catalyst/expressions/PredicateSuite.scala | 28 ------------------- 1 file changed, 28 deletions(-) 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 67a41e7cc2767..bb894fb94c78d 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 @@ -523,34 +523,6 @@ class PredicateSuite extends SparkFunSuite with ExpressionEvalHelper { } val row0 = create_row(null) - val row1 = create_row(false) - val row2 = create_row(true) - - test("istrue and isnottrue") { - checkEvaluation(IsTrue(Literal.create(null, BooleanType)), false, row0) - checkEvaluation(IsNotTrue(Literal.create(null, BooleanType)), true, row0) - checkEvaluation(IsTrue(Literal.create(false, BooleanType)), false, row1) - checkEvaluation(IsNotTrue(Literal.create(false, BooleanType)), true, row1) - checkEvaluation(IsTrue(Literal.create(true, BooleanType)), true, row2) - checkEvaluation(IsNotTrue(Literal.create(true, BooleanType)), false, row2) - IsTrue(Literal.create(null, IntegerType)).checkInputDataTypes() match { - case TypeCheckResult.TypeCheckFailure(msg) => - assert(msg.contains("argument 1 requires boolean type")) - } - } - - test("isfalse and isnotfalse") { - checkEvaluation(IsFalse(Literal.create(null, BooleanType)), false, row0) - checkEvaluation(IsNotFalse(Literal.create(null, BooleanType)), true, row0) - checkEvaluation(IsFalse(Literal.create(false, BooleanType)), true, row1) - checkEvaluation(IsNotFalse(Literal.create(false, BooleanType)), false, row1) - checkEvaluation(IsFalse(Literal.create(true, BooleanType)), false, row2) - checkEvaluation(IsNotFalse(Literal.create(true, BooleanType)), true, row2) - IsFalse(Literal.create(null, IntegerType)).checkInputDataTypes() match { - case TypeCheckResult.TypeCheckFailure(msg) => - assert(msg.contains("argument 1 requires boolean type")) - } - } test("isunknown and isnotunknown") { checkEvaluation(IsUnknown(Literal.create(null, BooleanType)), true, row0) From cfa4ecb2d86ee9ab7b6801b54eeef95825823834 Mon Sep 17 00:00:00 2001 From: beliefer Date: Wed, 26 Feb 2020 16:07:44 +0800 Subject: [PATCH 3/3] Optimize code --- .../spark/sql/catalyst/expressions/PredicateSuite.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 bb894fb94c78d..1ad0a8ed758f4 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 @@ -522,9 +522,9 @@ class PredicateSuite extends SparkFunSuite with ExpressionEvalHelper { assert(expression == expected) } - val row0 = create_row(null) - test("isunknown and isnotunknown") { + val row0 = create_row(null) + checkEvaluation(IsUnknown(Literal.create(null, BooleanType)), true, row0) checkEvaluation(IsNotUnknown(Literal.create(null, BooleanType)), false, row0) IsUnknown(Literal.create(null, IntegerType)).checkInputDataTypes() match {