-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-33861][SQL] Simplify conditional in predicate #30865
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.
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
Changes from 7 commits
f786f18
45fd7a5
448faf0
94551ea
c90227a
cccbaf1
8bd9ef9
878beb0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,82 @@ | ||||||||||
| /* | ||||||||||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||||||||||
| * contributor license agreements. See the NOTICE file distributed with | ||||||||||
| * this work for additional information regarding copyright ownership. | ||||||||||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||||||||||
| * (the "License"); you may not use this file except in compliance with | ||||||||||
| * the License. You may obtain a copy of the License at | ||||||||||
| * | ||||||||||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||||||||||
| * | ||||||||||
| * Unless required by applicable law or agreed to in writing, software | ||||||||||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||||||||||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||||||
| * See the License for the specific language governing permissions and | ||||||||||
| * limitations under the License. | ||||||||||
| */ | ||||||||||
|
|
||||||||||
| package org.apache.spark.sql.catalyst.optimizer | ||||||||||
|
|
||||||||||
| import org.apache.spark.sql.catalyst.expressions.{And, CaseWhen, Expression, If, Literal, Not, Or} | ||||||||||
| import org.apache.spark.sql.catalyst.expressions.Literal.{FalseLiteral, TrueLiteral} | ||||||||||
| import org.apache.spark.sql.catalyst.plans.logical._ | ||||||||||
| import org.apache.spark.sql.catalyst.rules.Rule | ||||||||||
| import org.apache.spark.sql.types.BooleanType | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * A rule that converting conditional expressions to predicate expressions, if possible, in the | ||||||||||
| * search condition of the WHERE/HAVING/ON(JOIN) clauses, which contain an implicit Boolean operator | ||||||||||
| * "(search condition) = TRUE". After this converting, we can potentially push the filter down to | ||||||||||
| * the data source. | ||||||||||
| * | ||||||||||
| * Supported cases are: | ||||||||||
| * - IF(cond, trueVal, false) => AND(cond, trueVal) | ||||||||||
| * - IF(cond, trueVal, true) => OR(NOT(cond), trueVal) | ||||||||||
| * - IF(cond, false, falseVal) => AND(NOT(cond), elseVal) | ||||||||||
| * - IF(cond, true, falseVal) => OR(cond, elseVal) | ||||||||||
| * - CASE WHEN cond THEN trueVal ELSE false END => AND(cond, trueVal) | ||||||||||
| * - CASE WHEN cond THEN trueVal END => AND(cond, trueVal) | ||||||||||
| * - CASE WHEN cond THEN trueVal ELSE null END => AND(cond, trueVal) | ||||||||||
| * - CASE WHEN cond THEN trueVal ELSE true END => OR(NOT(cond), trueVal) | ||||||||||
| * - CASE WHEN cond THEN false ELSE elseVal END => AND(NOT(cond), elseVal) | ||||||||||
| * - CASE WHEN cond THEN false END => false | ||||||||||
| * - CASE WHEN cond THEN true ELSE elseVal END => OR(cond, elseVal) | ||||||||||
| * - CASE WHEN cond THEN true END => cond | ||||||||||
| */ | ||||||||||
| object SimplifyConditionalsInPredicate extends Rule[LogicalPlan] { | ||||||||||
cloud-fan marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
|
|
||||||||||
| def apply(plan: LogicalPlan): LogicalPlan = plan transform { | ||||||||||
| case f @ Filter(cond, _) => f.copy(condition = simplifyConditional(cond)) | ||||||||||
| case j @ Join(_, _, _, Some(cond), _) => j.copy(condition = Some(simplifyConditional(cond))) | ||||||||||
| case d @ DeleteFromTable(_, Some(cond)) => d.copy(condition = Some(simplifyConditional(cond))) | ||||||||||
| case u @ UpdateTable(_, _, Some(cond)) => u.copy(condition = Some(simplifyConditional(cond))) | ||||||||||
| } | ||||||||||
|
|
||||||||||
| private def simplifyConditional(e: Expression): Expression = e match { | ||||||||||
| case And(left, right) => And(simplifyConditional(left), simplifyConditional(right)) | ||||||||||
| case Or(left, right) => Or(simplifyConditional(left), simplifyConditional(right)) | ||||||||||
| case If(cond, trueValue, FalseLiteral) => And(cond, trueValue) | ||||||||||
| case If(cond, trueValue, TrueLiteral) => Or(Not(cond), trueValue) | ||||||||||
| case If(cond, FalseLiteral, falseValue) => And(Not(cond), falseValue) | ||||||||||
| case If(cond, TrueLiteral, falseValue) => Or(cond, falseValue) | ||||||||||
| case CaseWhen(Seq((cond, trueValue)), | ||||||||||
| Some(FalseLiteral) | Some(Literal(null, BooleanType)) | None) => | ||||||||||
| And(cond, trueValue) | ||||||||||
| case CaseWhen(Seq((cond, trueValue)), Some(TrueLiteral)) => | ||||||||||
| Or(Not(cond), trueValue) | ||||||||||
| case CaseWhen(Seq((_, FalseLiteral)), Some(FalseLiteral) | None) => | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This skips evaluating the condition, and we should make sure the condition is deterministic. @wangyum can you fix it?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, will fix it later.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and also it matters if
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to make these condition deterministic? spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala Lines 295 to 298 in 26d8df3
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||||||
| FalseLiteral | ||||||||||
| case CaseWhen(Seq((cond, FalseLiteral)), Some(elseValue)) => | ||||||||||
| And(Not(cond), elseValue) | ||||||||||
| case CaseWhen(Seq((cond, TrueLiteral)), Some(FalseLiteral) | None) => | ||||||||||
| cond | ||||||||||
| case CaseWhen(Seq((cond, TrueLiteral)), Some(elseValue)) => | ||||||||||
| Or(cond, elseValue) | ||||||||||
| case e if e.dataType == BooleanType => e | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can add an assert. The analyzer should guarantee it.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean something like
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems the added assert can never be triggered. |
||||||||||
| case e => | ||||||||||
| assert(e.dataType != BooleanType, | ||||||||||
| "Expected a Boolean type expression in SimplifyConditionalsInPredicate, " + | ||||||||||
| s"but got the type `${e.dataType.catalogString}` in `${e.sql}`.") | ||||||||||
| e | ||||||||||
| } | ||||||||||
| } | ||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,237 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.apache.spark.sql.catalyst.optimizer | ||
|
|
||
| import org.apache.spark.sql.AnalysisException | ||
| 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, CaseWhen, Expression, If, IsNotNull, Literal, Or} | ||
| 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.{DeleteFromTable, LocalRelation, LogicalPlan, UpdateTable} | ||
| import org.apache.spark.sql.catalyst.rules.RuleExecutor | ||
| import org.apache.spark.sql.types.{BooleanType, IntegerType} | ||
|
|
||
| class SimplifyConditionalsInPredicateSuite extends PlanTest { | ||
|
|
||
| object Optimize extends RuleExecutor[LogicalPlan] { | ||
| val batches = | ||
| Batch("SimplifyConditionalsInPredicate", FixedPoint(10), | ||
| NullPropagation, | ||
| ConstantFolding, | ||
| BooleanSimplification, | ||
| SimplifyConditionals, | ||
| SimplifyConditionalsInPredicate) :: Nil | ||
| } | ||
|
|
||
| private val testRelation = | ||
| LocalRelation('i.int, 'b.boolean, 'a.array(IntegerType), 'm.map(IntegerType, IntegerType)) | ||
| private val anotherTestRelation = LocalRelation('d.int) | ||
|
|
||
| test("IF(cond, trueVal, false) => AND(cond, trueVal)") { | ||
| val originalCond = If( | ||
| UnresolvedAttribute("i") > Literal(10), | ||
| UnresolvedAttribute("b"), | ||
| FalseLiteral) | ||
| val expectedCond = And( | ||
| UnresolvedAttribute("i") > Literal(10), | ||
| UnresolvedAttribute("b")) | ||
| testFilter(originalCond, expectedCond = expectedCond) | ||
| testJoin(originalCond, expectedCond = expectedCond) | ||
| testDelete(originalCond, expectedCond = expectedCond) | ||
| testUpdate(originalCond, expectedCond = expectedCond) | ||
| testProjection(originalCond, expectedExpr = originalCond) | ||
| } | ||
|
|
||
| test("IF(cond, trueVal, true) => OR(NOT(cond), trueVal)") { | ||
| val originalCond = If( | ||
| UnresolvedAttribute("i") > Literal(10), | ||
| UnresolvedAttribute("b"), | ||
| TrueLiteral) | ||
| val expectedCond = Or( | ||
| UnresolvedAttribute("i") <= Literal(10), | ||
| UnresolvedAttribute("b")) | ||
| testFilter(originalCond, expectedCond = expectedCond) | ||
| testJoin(originalCond, expectedCond = expectedCond) | ||
| testDelete(originalCond, expectedCond = expectedCond) | ||
| testUpdate(originalCond, expectedCond = expectedCond) | ||
| testProjection(originalCond, expectedExpr = originalCond) | ||
| } | ||
|
|
||
| test("IF(cond, false, falseVal) => AND(NOT(cond), elseVal)") { | ||
| val originalCond = If( | ||
| UnresolvedAttribute("i") > Literal(10), | ||
| FalseLiteral, | ||
| UnresolvedAttribute("b")) | ||
| val expectedCond = And( | ||
| UnresolvedAttribute("i") <= Literal(10), | ||
| UnresolvedAttribute("b")) | ||
| testFilter(originalCond, expectedCond = expectedCond) | ||
| testJoin(originalCond, expectedCond = expectedCond) | ||
| testDelete(originalCond, expectedCond = expectedCond) | ||
| testUpdate(originalCond, expectedCond = expectedCond) | ||
| testProjection(originalCond, expectedExpr = originalCond) | ||
| } | ||
|
|
||
| test("IF(cond, true, falseVal) => OR(cond, elseVal)") { | ||
| val originalCond = If( | ||
| UnresolvedAttribute("i") > Literal(10), | ||
| TrueLiteral, | ||
| UnresolvedAttribute("b")) | ||
| val expectedCond = Or( | ||
| UnresolvedAttribute("i") > Literal(10), | ||
| UnresolvedAttribute("b")) | ||
| testFilter(originalCond, expectedCond = expectedCond) | ||
| testJoin(originalCond, expectedCond = expectedCond) | ||
| testDelete(originalCond, expectedCond = expectedCond) | ||
| testUpdate(originalCond, expectedCond = expectedCond) | ||
| testProjection(originalCond, expectedExpr = originalCond) | ||
| } | ||
|
|
||
| test("CASE WHEN cond THEN trueVal ELSE false END => AND(cond, trueVal)") { | ||
| Seq(Some(FalseLiteral), None, Some(Literal(null, BooleanType))).foreach { elseExp => | ||
| val originalCond = CaseWhen( | ||
| Seq((UnresolvedAttribute("i") > Literal(10), UnresolvedAttribute("b"))), | ||
| elseExp) | ||
| val expectedCond = And( | ||
| UnresolvedAttribute("i") > Literal(10), | ||
| UnresolvedAttribute("b")) | ||
| testFilter(originalCond, expectedCond = expectedCond) | ||
| testJoin(originalCond, expectedCond = expectedCond) | ||
| testDelete(originalCond, expectedCond = expectedCond) | ||
| testUpdate(originalCond, expectedCond = expectedCond) | ||
| testProjection(originalCond, expectedExpr = originalCond) | ||
| } | ||
| } | ||
|
|
||
| test("CASE WHEN cond THEN trueVal ELSE true END => OR(NOT(cond), trueVal)") { | ||
| val originalCond = CaseWhen( | ||
| Seq((UnresolvedAttribute("i") > Literal(10), UnresolvedAttribute("b"))), | ||
| TrueLiteral) | ||
| val expectedCond = Or( | ||
| UnresolvedAttribute("i") <= Literal(10), | ||
| UnresolvedAttribute("b")) | ||
| testFilter(originalCond, expectedCond = expectedCond) | ||
| testJoin(originalCond, expectedCond = expectedCond) | ||
| testDelete(originalCond, expectedCond = expectedCond) | ||
| testUpdate(originalCond, expectedCond = expectedCond) | ||
| testProjection(originalCond, expectedExpr = originalCond) | ||
| } | ||
|
|
||
| test("CASE WHEN cond THEN false ELSE elseVal END => AND(NOT(cond), elseVal)") { | ||
| val originalCond = CaseWhen( | ||
| Seq((UnresolvedAttribute("i") > Literal(10), FalseLiteral)), | ||
| UnresolvedAttribute("b")) | ||
| val expectedCond = And( | ||
| UnresolvedAttribute("i") <= Literal(10), | ||
| UnresolvedAttribute("b")) | ||
| testFilter(originalCond, expectedCond = expectedCond) | ||
| testJoin(originalCond, expectedCond = expectedCond) | ||
| testDelete(originalCond, expectedCond = expectedCond) | ||
| testUpdate(originalCond, expectedCond = expectedCond) | ||
| testProjection(originalCond, expectedExpr = originalCond) | ||
| } | ||
|
|
||
| test("CASE WHEN cond THEN false END => false") { | ||
| val originalCond = CaseWhen( | ||
| Seq((UnresolvedAttribute("i") > Literal(10), FalseLiteral))) | ||
| testFilter(originalCond, expectedCond = FalseLiteral) | ||
| testJoin(originalCond, expectedCond = FalseLiteral) | ||
| testDelete(originalCond, expectedCond = FalseLiteral) | ||
| testUpdate(originalCond, expectedCond = FalseLiteral) | ||
| testProjection(originalCond, expectedExpr = originalCond) | ||
| } | ||
|
|
||
| test("CASE WHEN cond THEN true ELSE elseVal END => OR(cond, elseVal)") { | ||
| val originalCond = CaseWhen( | ||
| Seq((UnresolvedAttribute("i") > Literal(10), TrueLiteral)), | ||
| UnresolvedAttribute("b")) | ||
| val expectedCond = Or( | ||
| UnresolvedAttribute("i") > Literal(10), | ||
| UnresolvedAttribute("b")) | ||
| testFilter(originalCond, expectedCond = expectedCond) | ||
| testJoin(originalCond, expectedCond = expectedCond) | ||
| testDelete(originalCond, expectedCond = expectedCond) | ||
| testUpdate(originalCond, expectedCond = expectedCond) | ||
| testProjection(originalCond, expectedExpr = originalCond) | ||
| } | ||
|
|
||
| test("CASE WHEN cond THEN true END => cond") { | ||
| val originalCond = CaseWhen( | ||
| Seq((UnresolvedAttribute("i") > Literal(10), TrueLiteral))) | ||
| val expectedCond = UnresolvedAttribute("i") > Literal(10) | ||
| testFilter(originalCond, expectedCond = expectedCond) | ||
| testJoin(originalCond, expectedCond = expectedCond) | ||
| testDelete(originalCond, expectedCond = expectedCond) | ||
| testUpdate(originalCond, expectedCond = expectedCond) | ||
| testProjection(originalCond, expectedExpr = originalCond) | ||
| } | ||
|
|
||
| test("Simplify conditional in conditions of CaseWhen inside another CaseWhen") { | ||
| val nestedCaseWhen = CaseWhen( | ||
| Seq((UnresolvedAttribute("i") > Literal(10)) -> UnresolvedAttribute("b")), | ||
| FalseLiteral) | ||
| val originalCond = CaseWhen(Seq(IsNotNull(nestedCaseWhen) -> FalseLiteral)) | ||
| val expectedCond = FalseLiteral | ||
|
|
||
| testFilter(originalCond, expectedCond = expectedCond) | ||
| testJoin(originalCond, expectedCond = expectedCond) | ||
| testDelete(originalCond, expectedCond = expectedCond) | ||
| testUpdate(originalCond, expectedCond = expectedCond) | ||
| testProjection(originalCond, expectedExpr = originalCond) | ||
| } | ||
|
|
||
| test("Not expected type - SimplifyConditionalsInPredicate") { | ||
| val e = intercept[AnalysisException] { | ||
| testFilter(originalCond = Literal(null, IntegerType), expectedCond = FalseLiteral) | ||
| }.getMessage | ||
| assert(e.contains("'CAST(NULL AS INT)' of type int is not a boolean")) | ||
| } | ||
|
|
||
| private def testFilter(originalCond: Expression, expectedCond: Expression): Unit = { | ||
| test((rel, exp) => rel.where(exp), originalCond, expectedCond) | ||
| } | ||
|
|
||
| private def testJoin(originalCond: Expression, expectedCond: Expression): Unit = { | ||
| test((rel, exp) => rel.join(anotherTestRelation, Inner, Some(exp)), originalCond, expectedCond) | ||
| } | ||
|
|
||
| private def testProjection(originalExpr: Expression, expectedExpr: Expression): Unit = { | ||
| test((rel, exp) => rel.select(exp), originalExpr, expectedExpr) | ||
| } | ||
|
|
||
| private def testDelete(originalCond: Expression, expectedCond: Expression): Unit = { | ||
| test((rel, expr) => DeleteFromTable(rel, Some(expr)), originalCond, expectedCond) | ||
| } | ||
|
|
||
| private def testUpdate(originalCond: Expression, expectedCond: Expression): Unit = { | ||
| test((rel, expr) => UpdateTable(rel, Seq.empty, Some(expr)), originalCond, expectedCond) | ||
| } | ||
|
|
||
| private def test( | ||
| func: (LogicalPlan, Expression) => LogicalPlan, | ||
| originalExpr: Expression, | ||
| expectedExpr: Expression): Unit = { | ||
|
|
||
| val originalPlan = func(testRelation, originalExpr).analyze | ||
| val optimizedPlan = Optimize.execute(originalPlan) | ||
| val expectedPlan = func(testRelation, expectedExpr).analyze | ||
| comparePlans(optimizedPlan, expectedPlan) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that converting->that converts