Skip to content

[SPARK-27924][SQL][FOLLOW-UP] Improve ANSI SQL Boolean-Predicate #27699

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
beliefer wants to merge 3 commits into from
Closed

[SPARK-27924][SQL][FOLLOW-UP] Improve ANSI SQL Boolean-Predicate #27699

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b16b258
Improve boolean test
beliefer Feb 26, 2020
f146322
Remove UT
beliefer Feb 26, 2020
cfa4ecb
Optimize code
beliefer Feb 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 0 additions & 60 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 4 additions & 4 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
28 changes: 0 additions & 28 deletions sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/PredicateSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -523,34 +523,6 @@ class PredicateSuite extends SparkFunSuite with ExpressionEvalHelper {
}

val row0 = create_row(null)

@maropu maropu Feb 26, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move this variable into the test test("isunknown and isnotunknown")?

@beliefer beliefer Feb 26, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

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)
Expand Down