From dc04923d6ccf9f3904d6bc9b32551c85952fa382 Mon Sep 17 00:00:00 2001 From: Peter Toth Date: Fri, 3 Mar 2023 14:35:15 +0100 Subject: [PATCH 1/7] [SPARK-42500][SQL] ConstantPropagation support more cases --- .../sql/catalyst/optimizer/expressions.scala | 93 +++++++++---------- 1 file changed, 43 insertions(+), 50 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 1d756a2dcb744..54ecc9d78888f 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 @@ -18,6 +18,7 @@ package org.apache.spark.sql.catalyst.optimizer import scala.collection.immutable.HashSet +import scala.collection.mutable import scala.collection.mutable.{ArrayBuffer, Stack} import scala.util.control.NonFatal @@ -112,16 +113,13 @@ object ConstantFolding extends Rule[LogicalPlan] { object ConstantPropagation extends Rule[LogicalPlan] { def apply(plan: LogicalPlan): LogicalPlan = plan.transformUpWithPruning( _.containsAllPatterns(LITERAL, FILTER), ruleId) { - case f: Filter => - val (newCondition, _) = traverse(f.condition, replaceChildren = true, nullIsFalse = true) - if (newCondition.isDefined) { - f.copy(condition = newCondition.get) - } else { - f - } + case f: Filter => f.mapExpressions(traverse(_, replaceChildren = true, nullIsFalse = true)._1) } - type EqualityPredicates = Seq[((AttributeReference, Literal), BinaryComparison)] + // The keys are always canonicalized `AttributeReference`s, but it is easier to use `Expression` + // type keys here instead of casting `AttributeReference.canonicalized` to `AttributeReference` at + // the calling sites. + type EqualityPredicates = mutable.Map[Expression, (Literal, BinaryComparison)] /** * Traverse a condition as a tree and replace attributes with constant values. @@ -138,56 +136,52 @@ object ConstantPropagation extends Rule[LogicalPlan] { * case of `WHERE e`, null result of expression `e` means the same as if it * resulted false * @return A tuple including: - * 1. Option[Expression]: optional changed condition after traversal + * 1. Expression: optional changed condition after traversal * 2. EqualityPredicates: propagated mapping of attribute => constant */ - private def traverse(condition: Expression, replaceChildren: Boolean, nullIsFalse: Boolean) - : (Option[Expression], EqualityPredicates) = + private def traverse( + condition: Expression, + replaceChildren: Boolean, + nullIsFalse: Boolean): (Expression, EqualityPredicates) = condition match { case e @ EqualTo(left: AttributeReference, right: Literal) if safeToReplace(left, nullIsFalse) => - (None, Seq(((left, right), e))) + e -> mutable.Map(left.canonicalized -> (right, e)) case e @ EqualTo(left: Literal, right: AttributeReference) if safeToReplace(right, nullIsFalse) => - (None, Seq(((right, left), e))) + e -> mutable.Map(right.canonicalized -> (left, e)) case e @ EqualNullSafe(left: AttributeReference, right: Literal) if safeToReplace(left, nullIsFalse) => - (None, Seq(((left, right), e))) + e -> mutable.Map(left.canonicalized -> (right, e)) case e @ EqualNullSafe(left: Literal, right: AttributeReference) if safeToReplace(right, nullIsFalse) => - (None, Seq(((right, left), e))) - case a: And => - val (newLeft, equalityPredicatesLeft) = - traverse(a.left, replaceChildren = false, nullIsFalse) + e -> mutable.Map(right.canonicalized -> (left, e)) + case a @ And(left, right) => + val (newLeft, equalityPredicates) = + traverse(left, replaceChildren = false, nullIsFalse) val (newRight, equalityPredicatesRight) = - traverse(a.right, replaceChildren = false, nullIsFalse) - val equalityPredicates = equalityPredicatesLeft ++ equalityPredicatesRight - val newSelf = if (equalityPredicates.nonEmpty && replaceChildren) { - Some(And(replaceConstants(newLeft.getOrElse(a.left), equalityPredicates), - replaceConstants(newRight.getOrElse(a.right), equalityPredicates))) + traverse(right, replaceChildren = false, nullIsFalse) + // We could recognize when conflicting constants are coming from the left and right sides + // and immediately shortcut the `And` expression to `Literal.FalseLiteral`, but that case is + // not so common and actually it is the job of `ConstantFolding` and `BooleanSimplification` + // rules to deal with those optimizations. + equalityPredicates ++= equalityPredicatesRight + val newAnd = a.withNewChildren(if (equalityPredicates.nonEmpty && replaceChildren) { + val replacedNewLeft = replaceConstants(newLeft, equalityPredicates) + val replacedNewRight = replaceConstants(newRight, equalityPredicates) + Seq(replacedNewLeft, replacedNewRight) } else { - if (newLeft.isDefined || newRight.isDefined) { - Some(And(newLeft.getOrElse(a.left), newRight.getOrElse(a.right))) - } else { - None - } - } - (newSelf, equalityPredicates) + Seq(newLeft, newRight) + }) + newAnd -> equalityPredicates case o: Or => // Ignore the EqualityPredicates from children since they are only propagated through And. - val (newLeft, _) = traverse(o.left, replaceChildren = true, nullIsFalse) - val (newRight, _) = traverse(o.right, replaceChildren = true, nullIsFalse) - val newSelf = if (newLeft.isDefined || newRight.isDefined) { - Some(Or(left = newLeft.getOrElse(o.left), right = newRight.getOrElse((o.right)))) - } else { - None - } - (newSelf, Seq.empty) + o.mapChildren(traverse(_, replaceChildren = true, nullIsFalse)._1) -> mutable.Map.empty case n: Not => // Ignore the EqualityPredicates from children since they are only propagated through And. - val (newChild, _) = traverse(n.child, replaceChildren = true, nullIsFalse = false) - (newChild.map(Not), Seq.empty) - case _ => (None, Seq.empty) + n.mapChildren(traverse(_, replaceChildren = true, nullIsFalse = false)._1) -> + mutable.Map.empty + case o => o -> mutable.Map.empty } // We need to take into account if an attribute is nullable and the context of the conjunctive @@ -198,16 +192,15 @@ object ConstantPropagation extends Rule[LogicalPlan] { private def safeToReplace(ar: AttributeReference, nullIsFalse: Boolean) = !ar.nullable || nullIsFalse - private def replaceConstants(condition: Expression, equalityPredicates: EqualityPredicates) - : Expression = { - val constantsMap = AttributeMap(equalityPredicates.map(_._1)) - val predicates = equalityPredicates.map(_._2).toSet - def replaceConstants0(expression: Expression) = expression transform { - case a: AttributeReference => constantsMap.getOrElse(a, a) - } + private def replaceConstants( + condition: Expression, + equalityPredicates: EqualityPredicates): Expression = { + val predicates = equalityPredicates.values.map(_._2).toSet condition transform { - case e @ EqualTo(_, _) if !predicates.contains(e) => replaceConstants0(e) - case e @ EqualNullSafe(_, _) if !predicates.contains(e) => replaceConstants0(e) + case b: BinaryComparison if !predicates.contains(b) => b transform { + case a: AttributeReference if equalityPredicates.contains(a.canonicalized) => + equalityPredicates(a.canonicalized)._1 + } } } } From 0bb53d86a30ad9e642353ddddc6a9daac603b12a Mon Sep 17 00:00:00 2001 From: Yuming Wang Date: Tue, 21 Feb 2023 08:11:33 +0800 Subject: [PATCH 2/7] SPARK-42500: ConstantPropagation support more case --- .../optimizer/ConstantPropagationSuite.scala | 32 +++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ConstantPropagationSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ConstantPropagationSuite.scala index f5f1455f94611..106af71a9d653 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ConstantPropagationSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ConstantPropagationSuite.scala @@ -159,8 +159,9 @@ class ConstantPropagationSuite extends PlanTest { columnA === Literal(1) && columnA === Literal(2) && columnB === Add(columnA, Literal(3))) val correctAnswer = testRelation - .select(columnA) - .where(columnA === Literal(1) && columnA === Literal(2) && columnB === Literal(5)).analyze + .select(columnA, columnB) + .where(Literal.FalseLiteral) + .select(columnA).analyze comparePlans(Optimize.execute(query.analyze), correctAnswer) } @@ -186,4 +187,31 @@ class ConstantPropagationSuite extends PlanTest { .analyze comparePlans(Optimize.execute(query2), correctAnswer2) } + + test("SPARK-42500: ConstantPropagation supports more cases") { + comparePlans( + Optimize.execute(testRelation.where(columnA === 1 && columnB > columnA + 2).analyze), + testRelation.where(columnA === 1 && columnB > 3).analyze) + + comparePlans( + Optimize.execute(testRelation.where(columnA === 1 && columnA === 2).analyze), + testRelation.where(Literal.FalseLiteral).analyze) + + comparePlans( + Optimize.execute(testRelation.where(columnA === 1 && columnA === columnA + 2).analyze), + testRelation.where(Literal.FalseLiteral).analyze) + + comparePlans( + Optimize.execute( + testRelation.where((columnA === 1 || columnB === 2) && columnB === 1).analyze), + testRelation.where(columnA === 1 && columnB === 1).analyze) + + comparePlans( + Optimize.execute(testRelation.where(columnA === 1 && columnA === 1).analyze), + testRelation.where(columnA === 1).analyze) + + comparePlans( + Optimize.execute(testRelation.where(Not(columnA === 1 && columnA === columnA + 2)).analyze), + testRelation.where(Not(columnA === 1) || Not(columnA === columnA + 2)).analyze) + } } From 13872471225799a4993d5544c50f68a3c2af2316 Mon Sep 17 00:00:00 2001 From: Peter Toth Date: Mon, 6 Mar 2023 13:18:31 +0100 Subject: [PATCH 3/7] fix review findings --- .../spark/sql/catalyst/optimizer/expressions.scala | 10 +++++----- 1 file changed, 5 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 54ecc9d78888f..62b21499f9281 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 @@ -176,11 +176,12 @@ object ConstantPropagation extends Rule[LogicalPlan] { newAnd -> equalityPredicates case o: Or => // Ignore the EqualityPredicates from children since they are only propagated through And. - o.mapChildren(traverse(_, replaceChildren = true, nullIsFalse)._1) -> mutable.Map.empty + val newOr = o.mapChildren(traverse(_, replaceChildren = true, nullIsFalse)._1) + newOr -> mutable.Map.empty case n: Not => // Ignore the EqualityPredicates from children since they are only propagated through And. - n.mapChildren(traverse(_, replaceChildren = true, nullIsFalse = false)._1) -> - mutable.Map.empty + val newNot = n.mapChildren(traverse(_, replaceChildren = true, nullIsFalse = false)._1) + newNot -> mutable.Map.empty case o => o -> mutable.Map.empty } @@ -198,8 +199,7 @@ object ConstantPropagation extends Rule[LogicalPlan] { val predicates = equalityPredicates.values.map(_._2).toSet condition transform { case b: BinaryComparison if !predicates.contains(b) => b transform { - case a: AttributeReference if equalityPredicates.contains(a.canonicalized) => - equalityPredicates(a.canonicalized)._1 + case a: AttributeReference => equalityPredicates.get(a.canonicalized).map(_._1).getOrElse(a) } } } From cabb08354fa52816fb3afb34fc296e8213799341 Mon Sep 17 00:00:00 2001 From: Peter Toth Date: Tue, 7 Mar 2023 09:19:26 +0100 Subject: [PATCH 4/7] minor improvement --- .../apache/spark/sql/catalyst/optimizer/expressions.scala | 8 ++++++-- 1 file changed, 6 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 62b21499f9281..75e7400f8f502 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 @@ -157,7 +157,7 @@ object ConstantPropagation extends Rule[LogicalPlan] { if safeToReplace(right, nullIsFalse) => e -> mutable.Map(right.canonicalized -> (left, e)) case a @ And(left, right) => - val (newLeft, equalityPredicates) = + val (newLeft, equalityPredicatesLeft) = traverse(left, replaceChildren = false, nullIsFalse) val (newRight, equalityPredicatesRight) = traverse(right, replaceChildren = false, nullIsFalse) @@ -165,7 +165,11 @@ object ConstantPropagation extends Rule[LogicalPlan] { // and immediately shortcut the `And` expression to `Literal.FalseLiteral`, but that case is // not so common and actually it is the job of `ConstantFolding` and `BooleanSimplification` // rules to deal with those optimizations. - equalityPredicates ++= equalityPredicatesRight + val equalityPredicates = if (equalityPredicatesLeft.size < equalityPredicatesRight.size) { + equalityPredicatesRight ++= equalityPredicatesLeft + } else { + equalityPredicatesLeft ++= equalityPredicatesRight + } val newAnd = a.withNewChildren(if (equalityPredicates.nonEmpty && replaceChildren) { val replacedNewLeft = replaceConstants(newLeft, equalityPredicates) val replacedNewRight = replaceConstants(newRight, equalityPredicates) From 02440345fa88aac88d157f5d7660462d8e645918 Mon Sep 17 00:00:00 2001 From: Peter Toth Date: Tue, 7 Mar 2023 09:19:37 +0100 Subject: [PATCH 5/7] fix scaladoc --- .../org/apache/spark/sql/catalyst/optimizer/expressions.scala | 2 +- 1 file changed, 1 insertion(+), 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 75e7400f8f502..adffff3342151 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 @@ -136,7 +136,7 @@ object ConstantPropagation extends Rule[LogicalPlan] { * case of `WHERE e`, null result of expression `e` means the same as if it * resulted false * @return A tuple including: - * 1. Expression: optional changed condition after traversal + * 1. Expression: changed condition after traversal * 2. EqualityPredicates: propagated mapping of attribute => constant */ private def traverse( From b9f3fbb990a25b54e44352329aac0625857ad15b Mon Sep 17 00:00:00 2001 From: Peter Toth Date: Mon, 13 Mar 2023 09:26:13 +0100 Subject: [PATCH 6/7] improve constant map handling --- .../sql/catalyst/optimizer/expressions.scala | 55 ++++++++++--------- 1 file changed, 28 insertions(+), 27 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 adffff3342151..5464d9475c3bd 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 @@ -113,7 +113,8 @@ object ConstantFolding extends Rule[LogicalPlan] { object ConstantPropagation extends Rule[LogicalPlan] { def apply(plan: LogicalPlan): LogicalPlan = plan.transformUpWithPruning( _.containsAllPatterns(LITERAL, FILTER), ruleId) { - case f: Filter => f.mapExpressions(traverse(_, replaceChildren = true, nullIsFalse = true)._1) + case f: Filter => + f.mapExpressions(traverse(_, replaceChildren = true, nullIsFalse = true, None)) } // The keys are always canonicalized `AttributeReference`s, but it is easier to use `Expression` @@ -135,58 +136,58 @@ object ConstantPropagation extends Rule[LogicalPlan] { * @param nullIsFalse whether a boolean expression result can be considered to false e.g. in the * case of `WHERE e`, null result of expression `e` means the same as if it * resulted false - * @return A tuple including: - * 1. Expression: changed condition after traversal - * 2. EqualityPredicates: propagated mapping of attribute => constant + * @param equalityPredicates optional [[EqualityPredicates]] map to collect attribute => constant + * mapping in adjacent [[And]]]s + * @return changed condition after traversal */ private def traverse( condition: Expression, replaceChildren: Boolean, - nullIsFalse: Boolean): (Expression, EqualityPredicates) = + nullIsFalse: Boolean, + equalityPredicates: Option[EqualityPredicates]): Expression = condition match { case e @ EqualTo(left: AttributeReference, right: Literal) if safeToReplace(left, nullIsFalse) => - e -> mutable.Map(left.canonicalized -> (right, e)) + equalityPredicates.foreach(_ += left.canonicalized -> (right, e)) + e case e @ EqualTo(left: Literal, right: AttributeReference) if safeToReplace(right, nullIsFalse) => - e -> mutable.Map(right.canonicalized -> (left, e)) + equalityPredicates.foreach(_ += right.canonicalized -> (left, e)) + e case e @ EqualNullSafe(left: AttributeReference, right: Literal) if safeToReplace(left, nullIsFalse) => - e -> mutable.Map(left.canonicalized -> (right, e)) + equalityPredicates.foreach(_ += left.canonicalized -> (right, e)) + e case e @ EqualNullSafe(left: Literal, right: AttributeReference) if safeToReplace(right, nullIsFalse) => - e -> mutable.Map(right.canonicalized -> (left, e)) + equalityPredicates.foreach(_ += right.canonicalized -> (left, e)) + e case a @ And(left, right) => - val (newLeft, equalityPredicatesLeft) = - traverse(left, replaceChildren = false, nullIsFalse) - val (newRight, equalityPredicatesRight) = - traverse(right, replaceChildren = false, nullIsFalse) + val newEqualityPredicates: Option[EqualityPredicates] = if (replaceChildren) { + Some(mutable.Map.empty) + } else { + equalityPredicates + } + val newLeft = traverse(left, replaceChildren = false, nullIsFalse, newEqualityPredicates) + val newRight = traverse(right, replaceChildren = false, nullIsFalse, newEqualityPredicates) // We could recognize when conflicting constants are coming from the left and right sides // and immediately shortcut the `And` expression to `Literal.FalseLiteral`, but that case is // not so common and actually it is the job of `ConstantFolding` and `BooleanSimplification` // rules to deal with those optimizations. - val equalityPredicates = if (equalityPredicatesLeft.size < equalityPredicatesRight.size) { - equalityPredicatesRight ++= equalityPredicatesLeft - } else { - equalityPredicatesLeft ++= equalityPredicatesRight - } - val newAnd = a.withNewChildren(if (equalityPredicates.nonEmpty && replaceChildren) { - val replacedNewLeft = replaceConstants(newLeft, equalityPredicates) - val replacedNewRight = replaceConstants(newRight, equalityPredicates) + a.withNewChildren(if (newEqualityPredicates.get.nonEmpty && replaceChildren) { + val replacedNewLeft = replaceConstants(newLeft, newEqualityPredicates.get) + val replacedNewRight = replaceConstants(newRight, newEqualityPredicates.get) Seq(replacedNewLeft, replacedNewRight) } else { Seq(newLeft, newRight) }) - newAnd -> equalityPredicates case o: Or => // Ignore the EqualityPredicates from children since they are only propagated through And. - val newOr = o.mapChildren(traverse(_, replaceChildren = true, nullIsFalse)._1) - newOr -> mutable.Map.empty + o.mapChildren(traverse(_, replaceChildren = true, nullIsFalse, None)) case n: Not => // Ignore the EqualityPredicates from children since they are only propagated through And. - val newNot = n.mapChildren(traverse(_, replaceChildren = true, nullIsFalse = false)._1) - newNot -> mutable.Map.empty - case o => o -> mutable.Map.empty + n.mapChildren(traverse(_, replaceChildren = true, nullIsFalse = false, None)) + case o => o } // We need to take into account if an attribute is nullable and the context of the conjunctive From ecd650ae8eca1054663ad0612eabf7fd08d6fc60 Mon Sep 17 00:00:00 2001 From: Peter Toth Date: Wed, 15 Mar 2023 15:17:22 +0100 Subject: [PATCH 7/7] improve constant map handling 2 --- .../sql/catalyst/optimizer/expressions.scala | 21 +++++++------------ 1 file changed, 8 insertions(+), 13 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 5464d9475c3bd..580840b357624 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 @@ -114,7 +114,7 @@ object ConstantPropagation extends Rule[LogicalPlan] { def apply(plan: LogicalPlan): LogicalPlan = plan.transformUpWithPruning( _.containsAllPatterns(LITERAL, FILTER), ruleId) { case f: Filter => - f.mapExpressions(traverse(_, replaceChildren = true, nullIsFalse = true, None)) + f.mapExpressions(traverse(_, nullIsFalse = true, None)) } // The keys are always canonicalized `AttributeReference`s, but it is easier to use `Expression` @@ -132,7 +132,6 @@ object ConstantPropagation extends Rule[LogicalPlan] { * - On matching [[Or]] or [[Not]], recursively traverse each children, propagate empty mapping. * - Otherwise, stop traversal and propagate empty mapping. * @param condition condition to be traversed - * @param replaceChildren whether to replace attributes with constant values in children * @param nullIsFalse whether a boolean expression result can be considered to false e.g. in the * case of `WHERE e`, null result of expression `e` means the same as if it * resulted false @@ -142,7 +141,6 @@ object ConstantPropagation extends Rule[LogicalPlan] { */ private def traverse( condition: Expression, - replaceChildren: Boolean, nullIsFalse: Boolean, equalityPredicates: Option[EqualityPredicates]): Expression = condition match { @@ -163,18 +161,15 @@ object ConstantPropagation extends Rule[LogicalPlan] { equalityPredicates.foreach(_ += right.canonicalized -> (left, e)) e case a @ And(left, right) => - val newEqualityPredicates: Option[EqualityPredicates] = if (replaceChildren) { - Some(mutable.Map.empty) - } else { - equalityPredicates - } - val newLeft = traverse(left, replaceChildren = false, nullIsFalse, newEqualityPredicates) - val newRight = traverse(right, replaceChildren = false, nullIsFalse, newEqualityPredicates) + val newEqualityPredicates: Option[EqualityPredicates] = + equalityPredicates.orElse(Some(mutable.Map.empty)) + val newLeft = traverse(left, nullIsFalse, newEqualityPredicates) + val newRight = traverse(right, nullIsFalse, newEqualityPredicates) // We could recognize when conflicting constants are coming from the left and right sides // and immediately shortcut the `And` expression to `Literal.FalseLiteral`, but that case is // not so common and actually it is the job of `ConstantFolding` and `BooleanSimplification` // rules to deal with those optimizations. - a.withNewChildren(if (newEqualityPredicates.get.nonEmpty && replaceChildren) { + a.withNewChildren(if (equalityPredicates.isEmpty && newEqualityPredicates.get.nonEmpty) { val replacedNewLeft = replaceConstants(newLeft, newEqualityPredicates.get) val replacedNewRight = replaceConstants(newRight, newEqualityPredicates.get) Seq(replacedNewLeft, replacedNewRight) @@ -183,10 +178,10 @@ object ConstantPropagation extends Rule[LogicalPlan] { }) case o: Or => // Ignore the EqualityPredicates from children since they are only propagated through And. - o.mapChildren(traverse(_, replaceChildren = true, nullIsFalse, None)) + o.mapChildren(traverse(_, nullIsFalse, None)) case n: Not => // Ignore the EqualityPredicates from children since they are only propagated through And. - n.mapChildren(traverse(_, replaceChildren = true, nullIsFalse = false, None)) + n.mapChildren(traverse(_, nullIsFalse = false, None)) case o => o }