[SPARK-33884][SQL] Simplify CaseWhenclauses with (true and false) and (false and true) #30898

wangyum · 2020-12-23T02:10:56Z

What changes were proposed in this pull request?

This pr simplify CaseWhenclauses with (true and false) and (false and true):

Expression	cond.nullable	After simplify
case when cond then true else false end	true	cond <=> true
case when cond then true else false end	false	cond
case when cond then false else true end	true	!(cond <=> true)
case when cond then false else true end	false	!cond

Why are the changes needed?

Improve query performance.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Unit test.

SparkQA · 2020-12-23T03:23:42Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37850/

SparkQA · 2020-12-23T03:28:49Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37850/

SparkQA · 2020-12-23T04:31:19Z

Test build #133252 has finished for PR 30898 at commit 44733f8.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

# Conflicts: # sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/PushFoldableIntoBranchesSuite.scala # sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala

SparkQA · 2020-12-24T06:53:49Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37933/

SparkQA · 2020-12-24T07:27:05Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37933/

SparkQA · 2020-12-24T10:05:28Z

Test build #133342 has finished for PR 30898 at commit 15c74ca.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
static class MergeShuffleFile
class ContextAwareIterator[+T](val context: TaskContext, val delegate: Iterator[T])
implicit class DslAttr(attr: UnresolvedAttribute) extends ImplicitAttribute

SparkQA · 2020-12-25T05:15:36Z

Test build #133369 has finished for PR 30898 at commit 91ec8b2.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-12-25T05:36:12Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37960/

SparkQA · 2020-12-25T06:04:21Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37960/

maropu · 2020-12-25T06:24:20Z

Looks fine if the tests pass.

SparkQA · 2020-12-25T06:42:27Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37963/

SparkQA · 2020-12-25T07:19:13Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37963/

SparkQA · 2020-12-25T10:33:55Z

Test build #133372 has finished for PR 30898 at commit 871d29f.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

HyukjinKwon · 2020-12-28T03:42:47Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala

@@ -475,6 +475,8 @@ object SimplifyConditionals extends Rule[LogicalPlan] with PredicateHelper {
      case If(TrueLiteral, trueValue, _) => trueValue
      case If(FalseLiteral, _, falseValue) => falseValue
      case If(Literal(null, _), _, falseValue) => falseValue
+      case If(_, TrueLiteral, TrueLiteral) => TrueLiteral
+      case If(_, FalseLiteral, FalseLiteral) => FalseLiteral
      case If(cond, TrueLiteral, FalseLiteral) => cond
      case If(cond, FalseLiteral, TrueLiteral) => Not(cond)
      case If(cond, trueValue, falseValue)


@wangyum, doesn't this cover the case above by this case match?

This is only for deterministic expressions.

The conditions you added here also should only for deterministic ones then because cond will not be executed, see #21848 (comment) as an example.

BTW, the tests you added at https://github.com/apache/spark/pull/30898/files#diff-c18cf81d53bb8508802317a785a74f2dadb0487d88e0164efb36f89ce64973e3R210-R213 will pass because of the case match here.

cloud-fan · 2020-12-28T05:27:47Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala

@@ -475,6 +475,8 @@ object SimplifyConditionals extends Rule[LogicalPlan] with PredicateHelper {
      case If(TrueLiteral, trueValue, _) => trueValue
      case If(FalseLiteral, _, falseValue) => falseValue
      case If(Literal(null, _), _, falseValue) => falseValue
+      case If(_, TrueLiteral, TrueLiteral) => TrueLiteral


This skips evaluating the cond, which can be wrong if cond is non-deterministic.

SparkQA · 2020-12-28T06:48:06Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38019/

SparkQA · 2020-12-28T06:52:46Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38019/

cloud-fan · 2020-12-28T07:18:35Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala

@@ -484,6 +484,9 @@ object SimplifyConditionals extends Rule[LogicalPlan] with PredicateHelper {
      case If(cond, FalseLiteral, l @ Literal(null, _)) if !cond.nullable => And(Not(cond), l)
      case If(cond, TrueLiteral, l @ Literal(null, _)) if !cond.nullable => Or(cond, l)

+      case CaseWhen(Seq((cond, TrueLiteral)), Some(FalseLiteral)) => cond


do we already optimize single-branch CASE WHEN to IF and get this optimized?

No. https://issues.apache.org/jira/browse/SPARK-24892

OK so this follows #21850 (comment) , LGTM

SparkQA · 2020-12-28T10:38:25Z

Test build #133428 has finished for PR 30898 at commit ae3b284.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

wangyum · 2020-12-29T01:47:23Z

...talyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala

@@ -243,4 +243,40 @@ class SimplifyConditionalSuite extends PlanTest with ExpressionEvalHelper with P
        Literal.create(null, IntegerType))
    }
  }
+
+  test("SPARK-33884: simplify CaseWhen clauses with (true and false) and (false and true)") {


The test adapt from

spark/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala

Lines 203 to 237 in c2eac1d

test("SPARK-33845: remove unnecessary if when the outputs are boolean type") {

// verify the boolean equivalence of all transformations involved

val fields = Seq(

'cond.boolean.notNull,

'cond_nullable.boolean,

'a.boolean,

'b.boolean

)

val Seq(cond, cond_nullable, a, b) = fields.zipWithIndex.map { case (f, i) => f.at(i) }

val exprs = Seq(

// actual expressions of the transformations: original -> transformed

If(cond, true, false) -> cond,

If(cond, false, true) -> !cond,

If(cond_nullable, true, false) -> (cond_nullable <=> true),

If(cond_nullable, false, true) -> (!(cond_nullable <=> true)))

// check plans

for ((originalExpr, expectedExpr) <- exprs) {

assertEquivalent(originalExpr, expectedExpr)

}

// check evaluation

val binaryBooleanValues = Seq(true, false)

val ternaryBooleanValues = Seq(true, false, null)

for (condVal <- binaryBooleanValues;

condNullableVal <- ternaryBooleanValues;

aVal <- ternaryBooleanValues;

bVal <- ternaryBooleanValues;

(originalExpr, expectedExpr) <- exprs) {

val inputRow = create_row(condVal, condNullableVal, aVal, bVal)

val optimizedVal = evaluateWithoutCodegen(expectedExpr, inputRow)

checkEvaluation(originalExpr, optimizedVal, inputRow)

}

}

SparkQA · 2020-12-29T02:56:46Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38051/

SparkQA · 2020-12-29T03:30:24Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38051/

SparkQA · 2020-12-29T06:57:35Z

Test build #133462 has finished for PR 30898 at commit d3b072e.

This patch fails SparkR unit tests.
This patch merges cleanly.
This patch adds no public classes.

cloud-fan · 2020-12-29T07:09:10Z

thanks, merging to master!

wangyum added 2 commits December 23, 2020 09:36

simplify CaseWhen when one clause is null and another is boolean

c78882a

Add test to PushFoldableIntoBranchesSuite

44733f8

github-actions bot added the SQL label Dec 23, 2020

Merge remote-tracking branch 'upstream/master' into SPARK-33884

15c74ca

# Conflicts: # sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/PushFoldableIntoBranchesSuite.scala # sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala

Merge remote-tracking branch 'upstream/master' into SPARK-33884

afbded1

fix

91ec8b2

wangyum changed the title ~~[SPARK-33884][SQL] Simplify CaseWhen when one clause is null and another is boolean~~ [SPARK-33884][SQL] Smplify conditional if all branches are foldable boolean type Dec 25, 2020

wangyum changed the title ~~[SPARK-33884][SQL] Smplify conditional if all branches are foldable boolean type~~ [SPARK-33884][SQL] Simplify conditional if all branches are foldable boolean type Dec 25, 2020

fix

871d29f

wangyum requested review from cloud-fan, dbtsai, dongjoon-hyun and HyukjinKwon December 25, 2020 12:02

HyukjinKwon reviewed Dec 28, 2020

View reviewed changes

cloud-fan reviewed Dec 28, 2020

View reviewed changes

fix

ae3b284

wangyum changed the title ~~[SPARK-33884][SQL] Simplify conditional if all branches are foldable boolean type~~ [SPARK-33884][SQL] Simplify CaseWhenclauses with (true and false) and (false and true) Dec 28, 2020

cloud-fan reviewed Dec 28, 2020

View reviewed changes

cloud-fan approved these changes Dec 28, 2020

View reviewed changes

wangyum added 2 commits December 29, 2020 09:17

Merge remote-tracking branch 'upstream/master' into SPARK-33884

186a3d5

fix

d3b072e

wangyum commented Dec 29, 2020

View reviewed changes

cloud-fan approved these changes Dec 29, 2020

View reviewed changes

cloud-fan closed this in f7bdea3 Dec 29, 2020

wangyum deleted the SPARK-33884 branch December 29, 2020 09:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-33884][SQL] Simplify CaseWhenclauses with (true and false) and (false and true) #30898

[SPARK-33884][SQL] Simplify CaseWhenclauses with (true and false) and (false and true) #30898

wangyum commented Dec 23, 2020 •

edited

Loading

SparkQA commented Dec 23, 2020

SparkQA commented Dec 23, 2020

SparkQA commented Dec 23, 2020

SparkQA commented Dec 24, 2020

SparkQA commented Dec 24, 2020

SparkQA commented Dec 24, 2020

SparkQA commented Dec 25, 2020

SparkQA commented Dec 25, 2020

SparkQA commented Dec 25, 2020

maropu commented Dec 25, 2020

SparkQA commented Dec 25, 2020

SparkQA commented Dec 25, 2020

SparkQA commented Dec 25, 2020

HyukjinKwon Dec 28, 2020

wangyum Dec 28, 2020

HyukjinKwon Dec 28, 2020

HyukjinKwon Dec 28, 2020

cloud-fan Dec 28, 2020

SparkQA commented Dec 28, 2020

SparkQA commented Dec 28, 2020

cloud-fan Dec 28, 2020

wangyum Dec 28, 2020

cloud-fan Dec 28, 2020

SparkQA commented Dec 28, 2020

wangyum Dec 29, 2020

SparkQA commented Dec 29, 2020

SparkQA commented Dec 29, 2020

SparkQA commented Dec 29, 2020

cloud-fan commented Dec 29, 2020

	test("SPARK-33845: remove unnecessary if when the outputs are boolean type") {
	// verify the boolean equivalence of all transformations involved
	val fields = Seq(
	'cond.boolean.notNull,
	'cond_nullable.boolean,
	'a.boolean,
	'b.boolean
	)
	val Seq(cond, cond_nullable, a, b) = fields.zipWithIndex.map { case (f, i) => f.at(i) }

	val exprs = Seq(
	// actual expressions of the transformations: original -> transformed
	If(cond, true, false) -> cond,
	If(cond, false, true) -> !cond,
	If(cond_nullable, true, false) -> (cond_nullable <=> true),
	If(cond_nullable, false, true) -> (!(cond_nullable <=> true)))

	// check plans
	for ((originalExpr, expectedExpr) <- exprs) {
	assertEquivalent(originalExpr, expectedExpr)
	}

	// check evaluation
	val binaryBooleanValues = Seq(true, false)
	val ternaryBooleanValues = Seq(true, false, null)
	for (condVal <- binaryBooleanValues;
	condNullableVal <- ternaryBooleanValues;
	aVal <- ternaryBooleanValues;
	bVal <- ternaryBooleanValues;
	(originalExpr, expectedExpr) <- exprs) {
	val inputRow = create_row(condVal, condNullableVal, aVal, bVal)
	val optimizedVal = evaluateWithoutCodegen(expectedExpr, inputRow)
	checkEvaluation(originalExpr, optimizedVal, inputRow)
	}
	}

[SPARK-33884][SQL] Simplify CaseWhenclauses with (true and false) and (false and true) #30898

[SPARK-33884][SQL] Simplify CaseWhenclauses with (true and false) and (false and true) #30898

Conversation

wangyum commented Dec 23, 2020 • edited Loading

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

SparkQA commented Dec 23, 2020

SparkQA commented Dec 23, 2020

SparkQA commented Dec 23, 2020

SparkQA commented Dec 24, 2020

SparkQA commented Dec 24, 2020

SparkQA commented Dec 24, 2020

SparkQA commented Dec 25, 2020

SparkQA commented Dec 25, 2020

SparkQA commented Dec 25, 2020

maropu commented Dec 25, 2020

SparkQA commented Dec 25, 2020

SparkQA commented Dec 25, 2020

SparkQA commented Dec 25, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Dec 28, 2020

SparkQA commented Dec 28, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Dec 28, 2020

Choose a reason for hiding this comment

SparkQA commented Dec 29, 2020

SparkQA commented Dec 29, 2020

SparkQA commented Dec 29, 2020

cloud-fan commented Dec 29, 2020

wangyum commented Dec 23, 2020 •

edited

Loading