Add predicates simplification to SimplifyExpressions

It is possible that common sub-predicate occures in multiple predicates.
For instance:
 (X AND Y) OR (X AND V)
or
 (X OR Y) AND (X OR V)

In such cases it is possible to extract this common predicate:
 X AND (Y OR V)
or
 X OR (Y AND V)

This will let join predicates to be pushed to the top, therefore
allowing for faster joins.

Additionally, negations are pushed down to the bottom of expression tree.

presto-docs/src/main/sphinx/release.rst

@@ -5,6 +5,7 @@ Release Notes
 .. toctree::
     :maxdepth: 1
 
+    release/release-0.140
     release/release-0.139
     release/release-0.138
     release/release-0.137

presto-docs/src/main/sphinx/release/release-0.140.rst

@@ -0,0 +1,8 @@
+=============
+Release 0.139
+=============
+
+General Changes
+---------------
+
+* Optimize predicate expressions to minimize redundancies.

presto-main/src/main/java/com/facebook/presto/sql/ExpressionUtils.java

@@ -37,16 +37,12 @@
 
 import static com.facebook.presto.sql.tree.BooleanLiteral.FALSE_LITERAL;
 import static com.facebook.presto.sql.tree.BooleanLiteral.TRUE_LITERAL;
-import static com.facebook.presto.sql.tree.ComparisonExpression.Type.EQUAL;
-import static com.facebook.presto.sql.tree.ComparisonExpression.Type.GREATER_THAN;
-import static com.facebook.presto.sql.tree.ComparisonExpression.Type.GREATER_THAN_OR_EQUAL;
 import static com.facebook.presto.sql.tree.ComparisonExpression.Type.IS_DISTINCT_FROM;
-import static com.facebook.presto.sql.tree.ComparisonExpression.Type.LESS_THAN;
-import static com.facebook.presto.sql.tree.ComparisonExpression.Type.LESS_THAN_OR_EQUAL;
-import static com.facebook.presto.sql.tree.ComparisonExpression.Type.NOT_EQUAL;
 import static com.facebook.presto.util.ImmutableCollectors.toImmutableList;
 import static com.google.common.base.Predicates.not;
+import static com.google.common.collect.Iterables.contains;
 import static com.google.common.collect.Iterables.filter;
+import static com.google.common.collect.Iterables.transform;
 import static com.google.common.collect.Lists.newArrayList;
 import static java.util.Objects.requireNonNull;
 
@@ -56,24 +52,26 @@ private ExpressionUtils() {}
 
     public static List<Expression> extractConjuncts(Expression expression)
     {
-        if (expression instanceof LogicalBinaryExpression && ((LogicalBinaryExpression) expression).getType() == LogicalBinaryExpression.Type.AND) {
-            LogicalBinaryExpression and = (LogicalBinaryExpression) expression;
-            return ImmutableList.<Expression>builder()
-                    .addAll(extractConjuncts(and.getLeft()))
-                    .addAll(extractConjuncts(and.getRight()))
-                    .build();
-        }
-
-        return ImmutableList.of(expression);
+        return extractPredicates(LogicalBinaryExpression.Type.AND, expression);
     }
 
     public static List<Expression> extractDisjuncts(Expression expression)
     {
-        if (expression instanceof LogicalBinaryExpression && ((LogicalBinaryExpression) expression).getType() == LogicalBinaryExpression.Type.OR) {
-            LogicalBinaryExpression or = (LogicalBinaryExpression) expression;
+        return extractPredicates(LogicalBinaryExpression.Type.OR, expression);
+    }
+
+    public static List<Expression> extractPredicates(LogicalBinaryExpression expression)
+    {
+        return extractPredicates(expression.getType(), expression);
+    }
+
+    public static List<Expression> extractPredicates(LogicalBinaryExpression.Type type, Expression expression)
+    {
+        if (expression instanceof LogicalBinaryExpression && ((LogicalBinaryExpression) expression).getType() == type) {
+            LogicalBinaryExpression logicalBinaryExpression = (LogicalBinaryExpression) expression;
             return ImmutableList.<Expression>builder()
-                    .addAll(extractDisjuncts(or.getLeft()))
-                    .addAll(extractDisjuncts(or.getRight()))
+                    .addAll(extractPredicates(type, logicalBinaryExpression.getLeft()))
+                    .addAll(extractPredicates(type, logicalBinaryExpression.getRight()))
                     .build();
         }
 
@@ -114,6 +112,20 @@ public static Expression binaryExpression(LogicalBinaryExpression.Type type, Ite
         return queue.remove();
     }
 
+    public static Expression combinePredicates(LogicalBinaryExpression.Type type, Expression... expressions)
+    {
+        return combinePredicates(type, Arrays.asList(expressions));
+    }
+
+    public static Expression combinePredicates(LogicalBinaryExpression.Type type, Iterable<Expression> expressions)
+    {
+        if (type == LogicalBinaryExpression.Type.AND) {
+            return combineConjuncts(expressions);
+        }
+
+        return combineDisjuncts(expressions);
+    }
+
     public static Expression combineConjuncts(Expression... expressions)
     {
         return combineConjuncts(Arrays.asList(expressions));
@@ -129,11 +141,16 @@ public static Expression combineConjunctsWithDefault(Iterable<Expression> expres
         requireNonNull(expressions, "expressions is null");
 
         // Flatten all the expressions into their component conjuncts
-        expressions = Iterables.concat(Iterables.transform(expressions, ExpressionUtils::extractConjuncts));
+        expressions = Iterables.concat(transform(expressions, ExpressionUtils::extractConjuncts));
 
         // Strip out all true literal conjuncts
-        expressions = Iterables.filter(expressions, not(Predicates.<Expression>equalTo(TRUE_LITERAL)));
+        expressions = filter(expressions, not(Predicates.<Expression>equalTo(TRUE_LITERAL)));
         expressions = removeDuplicates(expressions);
+
+        if (contains(expressions, FALSE_LITERAL)) {
+            return FALSE_LITERAL;
+        }
+
         return Iterables.isEmpty(expressions) ? emptyDefault : and(expressions);
     }
 
@@ -152,11 +169,16 @@ public static Expression combineDisjunctsWithDefault(Iterable<Expression> expres
         requireNonNull(expressions, "expressions is null");
 
         // Flatten all the expressions into their component disjuncts
-        expressions = Iterables.concat(Iterables.transform(expressions, ExpressionUtils::extractDisjuncts));
+        expressions = Iterables.concat(transform(expressions, ExpressionUtils::extractDisjuncts));
 
         // Strip out all false literal disjuncts
-        expressions = Iterables.filter(expressions, not(Predicates.<Expression>equalTo(FALSE_LITERAL)));
+        expressions = filter(expressions, not(Predicates.<Expression>equalTo(FALSE_LITERAL)));
         expressions = removeDuplicates(expressions);
+
+        if (contains(expressions, TRUE_LITERAL)) {
+            return TRUE_LITERAL;
+        }
+
         return Iterables.isEmpty(expressions) ? emptyDefault : or(expressions);
     }
 
@@ -173,28 +195,6 @@ public static Expression stripDeterministicConjuncts(Expression expression)
                 .collect(toImmutableList()));
     }
 
-    public static ComparisonExpression.Type flipComparison(ComparisonExpression.Type type)
-    {
-        switch (type) {
-            case EQUAL:
-                return EQUAL;
-            case NOT_EQUAL:
-                return NOT_EQUAL;
-            case LESS_THAN:
-                return GREATER_THAN;
-            case LESS_THAN_OR_EQUAL:
-                return GREATER_THAN_OR_EQUAL;
-            case GREATER_THAN:
-                return LESS_THAN;
-            case GREATER_THAN_OR_EQUAL:
-                return LESS_THAN_OR_EQUAL;
-            case IS_DISTINCT_FROM:
-                return IS_DISTINCT_FROM;
-            default:
-                throw new IllegalArgumentException("Unsupported comparison: " + type);
-        }
-    }
-
     public static Function<Expression, Expression> expressionOrNullSymbols(final Predicate<Symbol>... nullSymbolScopes)
     {
         return expression -> {
@@ -222,41 +222,21 @@ public static Function<Expression, Expression> expressionOrNullSymbols(final Pre
     private static Iterable<Expression> removeDuplicates(Iterable<Expression> expressions)
     {
         // Capture all non-deterministic predicates
-        Iterable<Expression> nonDeterministicDisjuncts = Iterables.filter(expressions, not(DeterminismEvaluator::isDeterministic));
+        Iterable<Expression> nonDeterministicDisjuncts = filter(expressions, not(DeterminismEvaluator::isDeterministic));
 
         // Capture and de-dupe all deterministic predicates
-        Iterable<Expression> deterministicDisjuncts = ImmutableSet.copyOf(Iterables.filter(expressions, DeterminismEvaluator::isDeterministic));
+        Iterable<Expression> deterministicDisjuncts = ImmutableSet.copyOf(filter(expressions, DeterminismEvaluator::isDeterministic));
 
         return Iterables.concat(nonDeterministicDisjuncts, deterministicDisjuncts);
     }
 
-    private static ComparisonExpression.Type negate(ComparisonExpression.Type type)
-    {
-        switch (type) {
-            case EQUAL:
-                return NOT_EQUAL;
-            case NOT_EQUAL:
-                return EQUAL;
-            case LESS_THAN:
-                return GREATER_THAN_OR_EQUAL;
-            case LESS_THAN_OR_EQUAL:
-                return GREATER_THAN;
-            case GREATER_THAN:
-                return LESS_THAN_OR_EQUAL;
-            case GREATER_THAN_OR_EQUAL:
-                return LESS_THAN;
-            default:
-                throw new IllegalArgumentException("Unsupported comparison: " + type);
-        }
-    }
-
     public static Expression normalize(Expression expression)
     {
         if (expression instanceof NotExpression) {
             NotExpression not = (NotExpression) expression;
             if (not.getValue() instanceof ComparisonExpression && ((ComparisonExpression) not.getValue()).getType() != IS_DISTINCT_FROM) {
                 ComparisonExpression comparison = (ComparisonExpression) not.getValue();
-                return new ComparisonExpression(negate(comparison.getType()), comparison.getLeft(), comparison.getRight());
+                return new ComparisonExpression(comparison.getType().negate(), comparison.getLeft(), comparison.getRight());
             }
         }
         return expression;

presto-main/src/main/java/com/facebook/presto/sql/planner/DomainTranslator.java

@@ -59,7 +59,6 @@
 import static com.facebook.presto.sql.ExpressionUtils.and;
 import static com.facebook.presto.sql.ExpressionUtils.combineConjuncts;
 import static com.facebook.presto.sql.ExpressionUtils.combineDisjunctsWithDefault;
-import static com.facebook.presto.sql.ExpressionUtils.flipComparison;
 import static com.facebook.presto.sql.ExpressionUtils.or;
 import static com.facebook.presto.sql.planner.LiteralInterpreter.toExpression;
 import static com.facebook.presto.sql.tree.BooleanLiteral.FALSE_LITERAL;
@@ -545,7 +544,7 @@ private static Optional<NormalizedSimpleComparison> toNormalizedSimpleComparison
             return Optional.of(new NormalizedSimpleComparison((QualifiedNameReference) left, comparison.getType(), new NullableValue(expressionTypes.get(comparison.getRight()), right)));
         }
         if (right instanceof QualifiedNameReference && !(left instanceof Expression)) {
-            return Optional.of(new NormalizedSimpleComparison((QualifiedNameReference) right, flipComparison(comparison.getType()), new NullableValue(expressionTypes.get(comparison.getLeft()), left)));
+            return Optional.of(new NormalizedSimpleComparison((QualifiedNameReference) right, comparison.getType().flip(), new NullableValue(expressionTypes.get(comparison.getLeft()), left)));
         }
         return Optional.empty();
     }

presto-main/src/main/java/com/facebook/presto/sql/planner/RelationPlanner.java

@@ -79,7 +79,6 @@
 
 import static com.facebook.presto.spi.type.BigintType.BIGINT;
 import static com.facebook.presto.spi.type.BooleanType.BOOLEAN;
-import static com.facebook.presto.sql.ExpressionUtils.flipComparison;
 import static com.facebook.presto.sql.analyzer.SemanticErrorCode.NOT_SUPPORTED;
 import static com.facebook.presto.sql.planner.ExpressionInterpreter.evaluateConstantExpression;
 import static com.facebook.presto.sql.tree.ComparisonExpression.Type.EQUAL;
@@ -258,7 +257,7 @@ protected RelationPlan visitJoin(Join node, Void context)
                 else if (firstDependencies.stream().allMatch(right.canResolvePredicate()) && secondDependencies.stream().allMatch(left.canResolvePredicate())) {
                     leftExpression = comparison.getRight();
                     rightExpression = comparison.getLeft();
-                    comparisonType = flipComparison(comparisonType);
+                    comparisonType = comparisonType.flip();
                 }
                 else {
                     // must have a complex expression that involves both tuples on one side of the comparison expression (e.g., coalesce(left.x, right.x) = 1)