spark-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From viirya <...@git.apache.org>
Subject [GitHub] spark pull request #19912: [SPARK-22719][SQL]Refactor ConstantPropagation
Date Thu, 07 Dec 2017 02:16:57 GMT
Github user viirya commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19912#discussion_r155415232
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala
---
    @@ -64,50 +64,91 @@ object ConstantFolding extends Rule[LogicalPlan] {
      * }}}
      *
      * Approach used:
    - * - Start from AND operator as the root
    - * - Get all the children conjunctive predicates which are EqualTo / EqualNullSafe such
that they
    - *   don't have a `NOT` or `OR` operator in them
      * - Populate a mapping of attribute => constant value by looking at all the equals
predicates
      * - Using this mapping, replace occurrence of the attributes with the corresponding
constant values
      *   in the AND node.
      */
     object ConstantPropagation extends Rule[LogicalPlan] with PredicateHelper {
    -  private def containsNonConjunctionPredicates(expression: Expression): Boolean = expression.find
{
    -    case _: Not | _: Or => true
    -    case _ => false
    -  }.isDefined
    -
       def apply(plan: LogicalPlan): LogicalPlan = plan transform {
    -    case f: Filter => f transformExpressionsUp {
    -      case and: And =>
    -        val conjunctivePredicates =
    -          splitConjunctivePredicates(and)
    -            .filter(expr => expr.isInstanceOf[EqualTo] || expr.isInstanceOf[EqualNullSafe])
    -            .filterNot(expr => containsNonConjunctionPredicates(expr))
    -
    -        val equalityPredicates = conjunctivePredicates.collect {
    -          case e @ EqualTo(left: AttributeReference, right: Literal) => ((left, right),
e)
    -          case e @ EqualTo(left: Literal, right: AttributeReference) => ((right, left),
e)
    -          case e @ EqualNullSafe(left: AttributeReference, right: Literal) => ((left,
right), e)
    -          case e @ EqualNullSafe(left: Literal, right: AttributeReference) => ((right,
left), e)
    -        }
    -
    -        val constantsMap = AttributeMap(equalityPredicates.map(_._1))
    -        val predicates = equalityPredicates.map(_._2).toSet
    +    case f: Filter =>
    +      val (newCondition, _) = traverse(f.condition, replaceChildren = true)
    +      if (newCondition.isDefined) {
    +        f.copy(condition = newCondition.get)
    +      } else {
    +        f
    +      }
    +  }
     
    -        def replaceConstants(expression: Expression) = expression transform {
    -          case a: AttributeReference =>
    -            constantsMap.get(a) match {
    -              case Some(literal) => literal
    -              case None => a
    -            }
    +  /**
    +   * Traverse a condition as a tree and replace attributes with constant values.
    +   * - If the child of [[And]] is [[EqualTo]] or [[EqualNullSafe]], propagate the mapping
    +   *   of attribute => constant.
    +   * - If the current [[And]] node is not child of another [[And]], replace occurrence
of the
    +   *   attributes with the corresponding constant values in both children with propagated
mapping.
    +   * @param condition condition to be traversed
    +   * @param replaceChildren whether to replace attributes with the corresponding constant
values
    +   */
    +  private def traverse(condition: Expression, replaceChildren: Boolean)
    +    : (Option[Expression], Seq[((AttributeReference, Literal), BinaryComparison)]) =
    --- End diff --
    
    Maybe define a type alias for `Seq[((AttributeReference, Literal), BinaryComparison)]`.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Mime
View raw message