Moves filter normalization into BooleanSimplification rule

apache · Dec 25, 2014 · caca560 · caca560
1 parent 4ab3a58
commit caca560
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 37 deletions.
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
@@ -40,7 +40,6 @@ object DefaultOptimizer extends Optimizer {
       ConstantFolding,
       LikeSimplification,
       BooleanSimplification,
-      NormalizeFilters,
       SimplifyFilters,
       SimplifyCasts,
       SimplifyCaseConversionExpressions,
@@ -295,11 +294,16 @@ object OptimizeIn extends Rule[LogicalPlan] {
 }
 
 /**
- * Simplifies boolean expressions where the answer can be determined without evaluating both sides.
+ * Simplifies boolean expressions:
+ *
+ * 1. Simplifies expressions whose answer can be determined without evaluating both sides.
+ * 2. Eliminates / extracts common factors.
+ * 3. Removes `Not` operator.
+ *
  * Note that this rule can eliminate expressions that might otherwise have been evaluated and thus
  * is only safe when evaluations of expressions does not result in side effects.
  */
-object BooleanSimplification extends Rule[LogicalPlan] {
+object BooleanSimplification extends Rule[LogicalPlan] with PredicateHelper {
   def apply(plan: LogicalPlan): LogicalPlan = plan transform {
     case q: LogicalPlan => q transformExpressionsUp {
       case and @ And(left, right) =>
@@ -308,7 +312,9 @@ object BooleanSimplification extends Rule[LogicalPlan] {
           case (l, Literal(true, BooleanType)) => l
           case (Literal(false, BooleanType), _) => Literal(false)
           case (_, Literal(false, BooleanType)) => Literal(false)
-          case (_, _) => and
+          // a && a && a ... => a
+          case _ if splitConjunctivePredicates(and).distinct.size == 1 => left
+          case _ => and
         }
 
       case or @ Or(left, right) =>
@@ -317,7 +323,19 @@ object BooleanSimplification extends Rule[LogicalPlan] {
           case (_, Literal(true, BooleanType)) => Literal(true)
           case (Literal(false, BooleanType), r) => r
           case (l, Literal(false, BooleanType)) => l
-          case (_, _) => or
+          // a || a || a ... => a
+          case _ if splitDisjunctivePredicates(or).distinct.size == 1 => left
+          // (a && b && c && ...) || (a && b && d && ...) => a && b && (c || d || ...)
+          case _ =>
+            val lhsSet = splitConjunctivePredicates(left).toSet
+            val rhsSet = splitConjunctivePredicates(right).toSet
+            val common = lhsSet.intersect(rhsSet)
+
+            (lhsSet.diff(common).reduceOption(And) ++ rhsSet.diff(common).reduceOption(And))
+              .reduceOption(Or)
+              .map(_ :: common.toList)
+              .getOrElse(common.toList)
+              .reduce(And)
         }
 
       case not @ Not(exp) =>
@@ -348,37 +366,6 @@ object CombineFilters extends Rule[LogicalPlan] {
   }
 }
 
-/**
- * Normalizes conjunctions and disjunctions to eliminate common factors.
- */
-object NormalizeFilters extends Rule[LogicalPlan] with PredicateHelper {
-  override def apply(plan: LogicalPlan): LogicalPlan = plan transform {
-    case f @ Filter(predicate: Predicate, child) =>
-      f.copy(condition = normalizedPredicate(predicate).reduce(And))
-  }
-
-  def normalizedPredicate(predicate: Expression): Seq[Expression] = predicate match {
-    // a && a && a ... => a
-    case p @ And(e, _) if splitConjunctivePredicates(p).distinct.size == 1 => e :: Nil
-
-    // a || a || a ... => a
-    case p @ Or(e, _) if splitDisjunctivePredicates(p).distinct.size == 1 => e :: Nil
-
-    // (a && b && c && ...) || (a && b && d && ...) => a && b && (c || d || ...)
-    case Or(lhs, rhs) =>
-      val lhsSet = splitConjunctivePredicates(lhs).toSet
-      val rhsSet = splitConjunctivePredicates(rhs).toSet
-      val common = lhsSet.intersect(rhsSet)
-
-      (lhsSet.diff(common).reduceOption(And) ++ rhsSet.diff(common).reduceOption(And))
-        .reduceOption(Or)
-        .map(_ :: common.toList)
-        .getOrElse(common.toList)
-
-    case _ => predicate :: Nil
-  }
-}
-
 /**
  * Removes filters that can be evaluated trivially.  This is done either by eliding the filter for
  * cases where it will always evaluate to `true`, or substituting a dummy empty relation when the

diff --git a/...talyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/NormalizeFiltersSuite.scala b/...talyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/NormalizeFiltersSuite.scala
@@ -33,7 +33,7 @@ class NormalizeFiltersSuite extends PlanTest {
       Batch("AnalysisNodes", Once,
         EliminateAnalysisOperators),
       Batch("NormalizeFilters", FixedPoint(100),
-        NormalizeFilters,
+        BooleanSimplification,
         SimplifyFilters))
   }