SQL: Reduce number of ranges generated for comparisons (#30267)

* SQL: Reduce number of ranges generated for comparisons

Rewrote optimization rule for combining ranges by improving the
detection of binary comparisons in a tree to better combine
them in a range, regardless of their place inside an expression.
Additionally, improve the comparisons of Numbers of different types
Also, improve reassembly of conjunction/disjunction into balanced
trees.
Do not promote BinaryComparisons to Ranges since it introduces NULL
boundaries and thus a corner-case that needs too much handling
Compare BinaryComparisons directly between themselves and to Ranges

Fix #30017

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/BinaryComparison.java

@@ -9,7 +9,6 @@ import org.elasticsearch.xpack.sql.expression.BinaryOperator;
 import org.elasticsearch.xpack.sql.expression.Expression;
 import org.elasticsearch.xpack.sql.tree.Location;
 import org.elasticsearch.xpack.sql.type.DataType;
-import org.elasticsearch.xpack.sql.type.DataTypes;
 
 // marker class to indicate operations that rely on values
 public abstract class BinaryComparison extends BinaryOperator {
@@ -33,11 +32,42 @@ public abstract class BinaryComparison extends BinaryOperator {
         return DataType.BOOLEAN;
     }
 
+    /**
+     * Compares two expression arguments (typically Numbers), if possible.
+     * Otherwise returns null (the arguments are not comparable or at least
+     * one of them is null).
+     */
     @SuppressWarnings({ "rawtypes", "unchecked" })
-    static Integer compare(Object left, Object right) {
+    public static Integer compare(Object l, Object r) {
-        if (left instanceof Comparable && right instanceof Comparable) {
+        // typical number comparison
-            return Integer.valueOf(((Comparable) left).compareTo(right));
+        if (l instanceof Number && r instanceof Number) {
+            return compare((Number) l, (Number) r);
         }
+
+        if (l instanceof Comparable && r instanceof Comparable) {
+            try {
+                return Integer.valueOf(((Comparable) l).compareTo(r));
+            } catch (ClassCastException cce) {
+                // when types are not compatible, cce is thrown
+                // fall back to null
+                return null;
+            }
+        }
+
         return null;
     }
+
+    static Integer compare(Number l, Number r) {
+        if (l instanceof Double || r instanceof Double) {
+            return Double.compare(l.doubleValue(), r.doubleValue());
+        }
+        if (l instanceof Float || r instanceof Float) {
+            return Float.compare(l.floatValue(), r.floatValue());
+        }
+        if (l instanceof Long || r instanceof Long) {
+            return Long.compare(l.longValue(), r.longValue());
+        }
+
+        return Integer.valueOf(Integer.compare(l.intValue(), r.intValue()));
+    }
 }

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/Predicates.java

@@ -9,8 +9,11 @@ import org.elasticsearch.xpack.sql.expression.Expression;
 import org.elasticsearch.xpack.sql.plan.logical.LogicalPlan;
 
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
+import java.util.function.BiFunction;
+
+import static java.util.Collections.emptyList;
+import static java.util.Collections.singletonList;
 
 public abstract class Predicates {
 
@@ -22,7 +25,7 @@ public abstract class Predicates {
             list.addAll(splitAnd(and.right()));
             return list;
         }
-        return Collections.singletonList(exp);
+        return singletonList(exp);
     }
 
     public static List<Expression> splitOr(Expression exp) {
@@ -33,15 +36,51 @@ public abstract class Predicates {
             list.addAll(splitOr(or.right()));
             return list;
         }
-        return Collections.singletonList(exp);
+        return singletonList(exp);
     }
 
     public static Expression combineOr(List<Expression> exps) {
-        return exps.stream().reduce((l, r) -> new Or(l.location(), l, r)).orElse(null);
+        return combine(exps, (l, r) -> new Or(l.location(), l, r));
     }
 
     public static Expression combineAnd(List<Expression> exps) {
-        return exps.stream().reduce((l, r) -> new And(l.location(), l, r)).orElse(null);
+        return combine(exps, (l, r) -> new And(l.location(), l, r));
+    }
+
+    /**
+     * Build a binary 'pyramid' from the given list:
+     * <pre>
+     *       AND
+     *      /   \
+     *   AND     AND
+     *  /   \   /   \
+     * A     B C     D
+     * </pre>
+     * 
+     * using the given combiner.
+     * 
+     * While a bit longer, this method creates a balanced tree as oppose to a plain
+     * recursive approach which creates an unbalanced one (either to the left or right).
+     */
+    private static Expression combine(List<Expression> exps, BiFunction<Expression, Expression, Expression> combiner) {
+        if (exps.isEmpty()) {
+            return null;
+        }
+
+        // clone the list (to modify it)
+        List<Expression> result = new ArrayList<>(exps);
+
+        while (result.size() > 1) {
+            // combine (in place) expressions in pairs
+            // NB: this loop modifies the list (just like an array)
+            for (int i = 0; i < result.size() - 1; i++) {
+                Expression l = result.remove(i);
+                Expression r = result.remove(i);
+                result.add(i, combiner.apply(l, r));
+            }
+        }
+
+        return result.get(0);
     }
 
     public static List<Expression> inCommon(List<Expression> l, List<Expression> r) {
@@ -53,7 +92,7 @@ public abstract class Predicates {
                 }
             }
         }
-        return common.isEmpty() ? Collections.emptyList() : common;
+        return common.isEmpty() ? emptyList() : common;
     }
 
     public static List<Expression> subtract(List<Expression> from, List<Expression> r) {
@@ -65,7 +104,7 @@ public abstract class Predicates {
                 }
             }
         }
-        return diff.isEmpty() ? Collections.emptyList() : diff;
+        return diff.isEmpty() ? emptyList() : diff;
     }
 
 

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/Range.java

@@ -9,7 +9,6 @@ import org.elasticsearch.xpack.sql.expression.Expression;
 import org.elasticsearch.xpack.sql.tree.Location;
 import org.elasticsearch.xpack.sql.tree.NodeInfo;
 import org.elasticsearch.xpack.sql.type.DataType;
-import org.elasticsearch.xpack.sql.type.DataTypes;
 
 import java.util.Arrays;
 import java.util.List;
@@ -66,11 +65,19 @@ public class Range extends Expression {
 
     @Override
     public boolean foldable() {
-        return value.foldable() && lower.foldable() && upper.foldable();
+        if (lower.foldable() && upper.foldable()) {
+            return areBoundariesInvalid() || value.foldable();
+        }
+
+        return false;
     }
 
     @Override
     public Object fold() {
+        if (areBoundariesInvalid()) {
+            return Boolean.FALSE;
+        }
+
         Object val = value.fold();
         Integer lowerCompare = BinaryComparison.compare(lower.fold(), val);
         Integer upperCompare = BinaryComparison.compare(val, upper().fold());
@@ -79,6 +86,16 @@ public class Range extends Expression {
         return lowerComparsion && upperComparsion;
     }
 
+    /**
+     * Check whether the boundaries are invalid ( upper < lower) or not.
+     * If they do, the value does not have to be evaluate.
+     */
+    private boolean areBoundariesInvalid() {
+        Integer compare = BinaryComparison.compare(lower.fold(), upper.fold());
+        // upper < lower OR upper == lower and the range doesn't contain any equals
+        return compare != null && (compare > 0 || (compare == 0 && (!includeLower || !includeUpper)));
+    }
+
     @Override
     public boolean nullable() {
         return value.nullable() && lower.nullable() && upper.nullable();
@@ -122,4 +139,4 @@ public class Range extends Expression {
         sb.append(upper);
         return sb.toString();
     }
-}
+}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/optimizer/Optimizer.java

@@ -47,6 +47,7 @@ import org.elasticsearch.xpack.sql.expression.predicate.LessThan;
 import org.elasticsearch.xpack.sql.expression.predicate.LessThanOrEqual;
 import org.elasticsearch.xpack.sql.expression.predicate.Not;
 import org.elasticsearch.xpack.sql.expression.predicate.Or;
+import org.elasticsearch.xpack.sql.expression.predicate.Predicates;
 import org.elasticsearch.xpack.sql.expression.predicate.Range;
 import org.elasticsearch.xpack.sql.plan.logical.Aggregate;
 import org.elasticsearch.xpack.sql.plan.logical.Filter;
@@ -60,6 +61,7 @@ import org.elasticsearch.xpack.sql.rule.Rule;
 import org.elasticsearch.xpack.sql.rule.RuleExecutor;
 import org.elasticsearch.xpack.sql.session.EmptyExecutable;
 import org.elasticsearch.xpack.sql.session.SingletonExecutable;
+import org.elasticsearch.xpack.sql.util.CollectionUtils;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -121,9 +123,11 @@ public class Optimizer extends RuleExecutor<LogicalPlan> {
                 new ConstantFolding(),
                 // boolean
                 new BooleanSimplification(),
-                new BinaryComparisonSimplification(),
                 new BooleanLiteralsOnTheRight(),
-                new CombineComparisonsIntoRange(),
+                new BinaryComparisonSimplification(),
+                // needs to occur before BinaryComparison combinations (see class)
+                new PropagateEquals(),
+                new CombineBinaryComparisons(),
                 // prune/elimination
                 new PruneFilters(),
                 new PruneOrderBy(),
@@ -1231,7 +1235,7 @@ public class Optimizer extends RuleExecutor<LogicalPlan> {
     static class BinaryComparisonSimplification extends OptimizerExpressionRule {
 
         BinaryComparisonSimplification() {
-            super(TransformDirection.UP);
+            super(TransformDirection.DOWN);
         }
 
         @Override
@@ -1277,47 +1281,483 @@ public class Optimizer extends RuleExecutor<LogicalPlan> {
         }
     }
 
-    static class CombineComparisonsIntoRange extends OptimizerExpressionRule {
+    /**
+     * Propagate Equals to eliminate conjuncted Ranges.
+     * When encountering a different Equals or non-containing {@link Range}, the conjunction becomes false.
+     * When encountering a containing {@link Range}, the range gets eliminated by the equality.
+     * 
+     * This rule doesn't perform any promotion of {@link BinaryComparison}s, that is handled by
+     * {@link CombineBinaryComparisons} on purpose as the resulting Range might be foldable
+     * (which is picked by the folding rule on the next run).
+     */
+    static class PropagateEquals extends OptimizerExpressionRule {
 
-        CombineComparisonsIntoRange() {
+        PropagateEquals() {
-            super(TransformDirection.UP);
+            super(TransformDirection.DOWN);
         }
 
         @Override
         protected Expression rule(Expression e) {
-            return e instanceof And ? combine((And) e) : e;
+            if (e instanceof And) {
+                return propagate((And) e);
+            }
+            return e;
         }
 
-        private Expression combine(And and) {
+        // combine conjunction
-            Expression l = and.left();
+        private Expression propagate(And and) {
-            Expression r = and.right();
+            List<Range> ranges = new ArrayList<>();
+            List<Equals> equals = new ArrayList<>();
+            List<Expression> exps = new ArrayList<>();
 
-            if (l instanceof BinaryComparison && r instanceof BinaryComparison) {
+            boolean changed = false;
-                // if the same operator is used
-                BinaryComparison lb = (BinaryComparison) l;
-                BinaryComparison rb = (BinaryComparison) r;
 
-
+            for (Expression ex : Predicates.splitAnd(and)) {
-                if (lb.left().equals(((BinaryComparison) r).left()) && lb.right() instanceof Literal && rb.right() instanceof Literal) {
+                if (ex instanceof Range) {
-                    // >/>= AND </<=
+                    ranges.add((Range) ex);
-                    if ((l instanceof GreaterThan || l instanceof GreaterThanOrEqual)
+                } else if (ex instanceof Equals) {
-                            && (r instanceof LessThan || r instanceof LessThanOrEqual)) {
+                    Equals otherEq = (Equals) ex;
-                        return new Range(and.location(), lb.left(), lb.right(), l instanceof GreaterThanOrEqual, rb.right(),
+                    // equals on different values evaluate to FALSE
-                                r instanceof LessThanOrEqual);
+                    if (otherEq.right().foldable()) {
-                    }
+                        for (Equals eq : equals) {
-                    // </<= AND >/>=
+                            // cannot evaluate equals so skip it
-                    else if ((r instanceof GreaterThan || r instanceof GreaterThanOrEqual)
+                            if (!eq.right().foldable()) {
-                            && (l instanceof LessThan || l instanceof LessThanOrEqual)) {
+                                continue;
-                        return new Range(and.location(), rb.left(), rb.right(), r instanceof GreaterThanOrEqual, lb.right(),
+                            }
-                                l instanceof LessThanOrEqual);
+                            if (otherEq.left().semanticEquals(eq.left())) {
+                                if (eq.right().foldable() && otherEq.right().foldable()) {
+                                    Integer comp = BinaryComparison.compare(eq.right().fold(), otherEq.right().fold());
+                                    if (comp != null) {
+                                        // var cannot be equal to two different values at the same time
+                                        if (comp != 0) {
+                                            return FALSE;
+                                        }
+                                    }
+                                }
+                            }
+                        }
                     }
+                    equals.add(otherEq);
+                } else {
+                    exps.add(ex);
                 }
             }
 
-            return and;
+            // check
+            for (Equals eq : equals) {
+                // cannot evaluate equals so skip it
+                if (!eq.right().foldable()) {
+                    continue;
+                }
+                Object eqValue = eq.right().fold();
+                
+                for (int i = 0; i < ranges.size(); i++) {
+                    Range range = ranges.get(i);
+
+                    if (range.value().semanticEquals(eq.left())) {
+                        // if equals is outside the interval, evaluate the whole expression to FALSE
+                        if (range.lower().foldable()) {
+                            Integer compare = BinaryComparison.compare(range.lower().fold(), eqValue);
+                            if (compare != null && (
+                                 // eq outside the lower boundary
+                                 compare > 0 ||
+                                 // eq matches the boundary but should not be included
+                                 (compare == 0 && !range.includeLower()))
+                                ) {
+                                return FALSE;
+                            }
+                        }
+                        if (range.upper().foldable()) {
+                            Integer compare = BinaryComparison.compare(range.upper().fold(), eqValue);
+                            if (compare != null && (
+                                 // eq outside the upper boundary
+                                 compare < 0 ||
+                                 // eq matches the boundary but should not be included
+                                 (compare == 0 && !range.includeUpper()))
+                                ) {
+                                return FALSE;
+                            }
+                        }
+                        
+                        // it's in the range and thus, remove it
+                        ranges.remove(i);
+                        changed = true;
+                    }
+                }
+            }
+            
+            return changed ? Predicates.combineAnd(CollectionUtils.combine(exps, equals, ranges)) : and;
         }
     }
 
+    static class CombineBinaryComparisons extends OptimizerExpressionRule {
+
+        CombineBinaryComparisons() {
+            super(TransformDirection.DOWN);
+        }
+
+        @Override
+        protected Expression rule(Expression e) {
+            if (e instanceof And) {
+                return combine((And) e);
+            } else if (e instanceof Or) {
+                return combine((Or) e);
+            }
+            return e;
+        }
+        
+        // combine conjunction
+        private Expression combine(And and) {
+            List<Range> ranges = new ArrayList<>();
+            List<BinaryComparison> bcs = new ArrayList<>();
+            List<Expression> exps = new ArrayList<>();
+
+            boolean changed = false;
+
+            for (Expression ex : Predicates.splitAnd(and)) {
+                if (ex instanceof Range) {
+                    Range r = (Range) ex;
+                    if (findExistingRange(r, ranges, true)) {
+                        changed = true;
+                    } else {
+                        ranges.add(r);
+                    }
+                } else if (ex instanceof BinaryComparison && !(ex instanceof Equals)) {
+                    BinaryComparison bc = (BinaryComparison) ex;
+
+                    if (bc.right().foldable() && (findConjunctiveComparisonInRange(bc, ranges) || findExistingComparison(bc, bcs, true))) {
+                        changed = true;
+                    } else {
+                        bcs.add(bc);
+                    }
+                } else {
+                    exps.add(ex);
+                }
+            }
+            
+            // finally try combining any left BinaryComparisons into possible Ranges
+            // this could be a different rule but it's clearer here wrt the order of comparisons
+
+            for (int i = 0; i < bcs.size() - 1; i++) {
+                BinaryComparison main = bcs.get(i);
+
+                for (int j = i + 1; j < bcs.size(); j++) {
+                    BinaryComparison other = bcs.get(j);
+                    
+                    if (main.left().semanticEquals(other.left())) {
+                        // >/>= AND </<=
+                        if ((main instanceof GreaterThan || main instanceof GreaterThanOrEqual)
+                                && (other instanceof LessThan || other instanceof LessThanOrEqual)) {
+                            bcs.remove(j);
+                            bcs.remove(i);
+                            
+                            ranges.add(new Range(and.location(), main.left(),
+                                    main.right(), main instanceof GreaterThanOrEqual,
+                                    other.right(), other instanceof LessThanOrEqual));
+
+                            changed = true;
+                        }
+                        // </<= AND >/>=
+                        else if ((other instanceof GreaterThan || other instanceof GreaterThanOrEqual)
+                                && (main instanceof LessThan || main instanceof LessThanOrEqual)) {
+                            bcs.remove(j);
+                            bcs.remove(i);
+                            
+                            ranges.add(new Range(and.location(), main.left(),
+                                    other.right(), other instanceof GreaterThanOrEqual,
+                                    main.right(), main instanceof LessThanOrEqual));
+
+                            changed = true;
+                        }
+                    }
+                }
+            }
+            
+            
+            return changed ? Predicates.combineAnd(CollectionUtils.combine(exps, bcs, ranges)) : and;
+        }
+
+        // combine disjunction
+        private Expression combine(Or or) {
+            List<BinaryComparison> bcs = new ArrayList<>();
+            List<Range> ranges = new ArrayList<>();
+            List<Expression> exps = new ArrayList<>();
+
+            boolean changed = false;
+
+            for (Expression ex : Predicates.splitOr(or)) {
+                if (ex instanceof Range) {
+                    Range r = (Range) ex;
+                    if (findExistingRange(r, ranges, false)) {
+                        changed = true;
+                    } else {
+                        ranges.add(r);
+                    }
+                } else if (ex instanceof BinaryComparison) {
+                    BinaryComparison bc = (BinaryComparison) ex;
+                    if (bc.right().foldable() && findExistingComparison(bc, bcs, false)) {
+                        changed = true;
+                    } else {
+                        bcs.add(bc);
+                    }
+                } else {
+                    exps.add(ex);
+                }
+            }
+
+            return changed ? Predicates.combineOr(CollectionUtils.combine(exps, bcs, ranges)) : or;
+        }
+
+        private static boolean findExistingRange(Range main, List<Range> ranges, boolean conjunctive) {
+            if (!main.lower().foldable() && !main.upper().foldable()) {
+                return false;
+            }
+            // NB: the loop modifies the list (hence why the int is used)
+            for (int i = 0; i < ranges.size(); i++) {
+                Range other = ranges.get(i);
+
+                if (main.value().semanticEquals(other.value())) {
+
+                    // make sure the comparison was done
+                    boolean compared = false;
+
+                    boolean lower = false;
+                    boolean upper = false;
+                    // boundary equality (useful to differentiate whether a range is included or not)
+                    // and thus whether it should be preserved or ignored
+                    boolean lowerEq = false;
+                    boolean upperEq = false;
+
+                    // evaluate lower
+                    if (main.lower().foldable() && other.lower().foldable()) {
+                        compared = true;
+
+                        Integer comp = BinaryComparison.compare(main.lower().fold(), other.lower().fold());
+                        // values are comparable
+                        if (comp != null) {
+                            // boundary equality
+                            lowerEq = comp == 0 && main.includeLower() == other.includeLower();
+                            // AND
+                            if (conjunctive) {
+                                        // (2 < a < 3) AND (1 < a < 3) -> (1 < a < 3)
+                                lower = comp > 0 ||
+                                        // (2 < a < 3) AND (2 < a <= 3) -> (2 < a < 3)
+                                        (comp == 0 && !main.includeLower() && other.includeLower());
+                            }
+                            // OR
+                            else {
+                                        // (1 < a < 3) OR (2 < a < 3) -> (1 < a < 3)
+                                lower = comp < 0 ||
+                                        // (2 <= a < 3) OR (2 < a < 3) -> (2 <= a < 3)
+                                        (comp == 0 && main.includeLower() && !other.includeLower()) || lowerEq;
+                            }
+                        }
+                    }
+                    // evaluate upper
+                    if (main.upper().foldable() && other.upper().foldable()) {
+                        compared = true;
+
+                        Integer comp = BinaryComparison.compare(main.upper().fold(), other.upper().fold());
+                        // values are comparable
+                        if (comp != null) {
+                            // boundary equality
+                            upperEq = comp == 0 && main.includeUpper() == other.includeUpper();
+
+                            // AND
+                            if (conjunctive) {
+                                        // (1 < a < 2) AND (1 < a < 3) -> (1 < a < 2)
+                                upper = comp < 0 ||
+                                        // (1 < a < 2) AND (1 < a <= 2) -> (1 < a < 2)
+                                        (comp == 0 && !main.includeUpper() && other.includeUpper());
+                            }
+                            // OR
+                            else {
+                                        // (1 < a < 3) OR (1 < a < 2) -> (1 < a < 3)
+                                upper = comp > 0 ||
+                                        // (1 < a <= 3) OR (1 < a < 3) -> (2 < a < 3)
+                                        (comp == 0 && main.includeUpper() && !other.includeUpper()) || upperEq;
+                            }
+                        }
+                    }
+
+                    // AND - at least one of lower or upper
+                    if (conjunctive) {
+                        // can tighten range
+                        if (lower || upper) {
+                            ranges.remove(i);
+                            ranges.add(i,
+                                    new Range(main.location(), main.value(),
+                                            lower ? main.lower() : other.lower(),
+                                            lower ? main.includeLower() : other.includeLower(),
+                                            upper ? main.upper() : other.upper(),
+                                            upper ? main.includeUpper() : other.includeUpper()));
+                        }
+
+                        // range was comparable
+                        return compared;
+                    }
+                    // OR - needs both upper and lower to loosen range
+                    else {
+                        // can loosen range
+                        if (lower && upper) {
+                            ranges.remove(i);
+                            ranges.add(i,
+                                    new Range(main.location(), main.value(),
+                                            lower ? main.lower() : other.lower(),
+                                            lower ? main.includeLower() : other.includeLower(),
+                                            upper ? main.upper() : other.upper(),
+                                            upper ? main.includeUpper() : other.includeUpper()));
+                            return true;
+                        }
+
+                        // if the range in included, no need to add it
+                        return compared && (!((lower && !lowerEq) || (upper && !upperEq)));
+                    }
+                }
+            }
+            return false;
+        }
+
+        private boolean findConjunctiveComparisonInRange(BinaryComparison main, List<Range> ranges) {
+            Object value = main.right().fold();
+            
+            // NB: the loop modifies the list (hence why the int is used)
+            for (int i = 0; i < ranges.size(); i++) {
+                Range other = ranges.get(i);
+                
+                if (main.left().semanticEquals(other.value())) {
+ 
+                    if (main instanceof GreaterThan || main instanceof GreaterThanOrEqual) {
+                        if (other.lower().foldable()) {
+                            Integer comp = BinaryComparison.compare(value, other.lower().fold());
+                            if (comp != null) {
+                                // 2 < a AND (2 <= a < 3) -> 2 < a < 3
+                                boolean lowerEq = comp == 0 && other.includeLower() && main instanceof GreaterThan;
+                                 // 2 < a AND (1 < a < 3) -> 2 < a < 3
+                                boolean lower = comp > 0 || lowerEq;
+                                
+                                if (lower) {
+                                    ranges.remove(i);
+                                    ranges.add(i,
+                                            new Range(other.location(), other.value(),
+                                                    main.right(), lowerEq ? true : other.includeLower(),
+                                                    other.upper(), other.includeUpper()));
+                                }
+
+                                // found a match
+                                return true;
+                            }
+                        }
+                    } else if (main instanceof LessThan || main instanceof LessThanOrEqual) {
+                        if (other.lower().foldable()) {
+                            Integer comp = BinaryComparison.compare(value, other.lower().fold());
+                            if (comp != null) {
+                                // a < 2 AND (1 < a <= 2) -> 1 < a < 2
+                                boolean upperEq = comp == 0 && other.includeUpper() && main instanceof LessThan;
+                                // a < 2 AND (1 < a < 3) -> 1 < a < 2
+                                boolean upper = comp > 0 || upperEq;
+
+                                if (upper) {
+                                    ranges.remove(i);
+                                    ranges.add(i, new Range(other.location(), other.value(),
+                                            other.lower(), other.includeLower(),
+                                            main.right(), upperEq ? true : other.includeUpper()));
+                                }
+
+                                // found a match
+                                return true;
+                            }
+                        }
+                    }
+
+                    return false;
+                }
+            }
+            return false;
+        }
+    
+        /**
+         * Find commonalities between the given comparison in the given list.
+         * The method can be applied both for conjunctive (AND) or disjunctive purposes (OR).
+         */
+        private static boolean findExistingComparison(BinaryComparison main, List<BinaryComparison> bcs, boolean conjunctive) {
+            Object value = main.right().fold();
+            
+            // NB: the loop modifies the list (hence why the int is used)
+            for (int i = 0; i < bcs.size(); i++) {
+                BinaryComparison other = bcs.get(i);
+                // skip if cannot evaluate
+                if (!other.right().foldable()) {
+                    continue;
+                }
+                // if bc is a higher/lower value or gte vs gt, use it instead
+                if ((other instanceof GreaterThan || other instanceof GreaterThanOrEqual) &&
+                    (main instanceof GreaterThan || main instanceof GreaterThanOrEqual)) {
+                    
+                    if (main.left().semanticEquals(other.left())) {
+                        Integer compare = BinaryComparison.compare(value, other.right().fold());
+                        
+                        if (compare != null) {
+                                 // AND
+                            if ((conjunctive &&
+                                  // a > 3 AND a > 2 -> a > 3
+                                  (compare > 0 ||
+                                  // a > 2 AND a >= 2 -> a > 2
+                                  (compare == 0 && main instanceof GreaterThan && other instanceof GreaterThanOrEqual)))
+                                ||
+                                // OR
+                                (!conjunctive &&
+                                  // a > 2 OR a > 3 -> a > 2
+                                  (compare < 0 ||
+                                  // a >= 2 OR a > 2 -> a >= 2
+                                  (compare == 0 && main instanceof GreaterThanOrEqual && other instanceof GreaterThan)))) {
+                                bcs.remove(i);
+                                bcs.add(i, main);
+                            }
+                            // found a match
+                            return true;
+                        }
+
+                        return false;
+                    }
+                }
+                // if bc is a lower/higher value or lte vs lt, use it instead
+                else if ((other instanceof LessThan || other instanceof LessThanOrEqual) &&
+                        (main instanceof LessThan || main instanceof LessThanOrEqual)) {
+                    
+                    if (main.left().semanticEquals(other.left())) {
+                        Integer compare = BinaryComparison.compare(value, other.right().fold());
+                        
+                        if (compare != null) {
+                                 // AND
+                            if ((conjunctive &&
+                                  // a < 2 AND a < 3 -> a < 2
+                                  (compare < 0 ||
+                                  // a < 2 AND a <= 2 -> a < 2
+                                  (compare == 0 && main instanceof LessThan && other instanceof LessThanOrEqual)))
+                                ||
+                                // OR
+                                (!conjunctive &&
+                                  // a < 2 OR a < 3 -> a < 3
+                                  (compare > 0 ||
+                                  // a <= 2 OR a < 2 -> a <= 2
+                                  (compare == 0 && main instanceof LessThanOrEqual && other instanceof LessThan)))) {
+                                bcs.remove(i);
+                                bcs.add(i, main);
+                                
+                            }
+                            // found a match
+                            return true;
+                        }
+
+                        return false;
+                    }
+                }
+            }
+                
+            return false;
+        }
+    }
 
     static class SkipQueryOnLimitZero extends OptimizerRule<Limit> {
         @Override
@@ -1435,4 +1875,4 @@ public class Optimizer extends RuleExecutor<LogicalPlan> {
     enum TransformDirection {
         UP, DOWN
     };
-}
+}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/tree/Node.java

@@ -5,6 +5,8 @@
  */
 package org.elasticsearch.xpack.sql.tree;
 
+import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
+
 import java.util.ArrayList;
 import java.util.BitSet;
 import java.util.List;
@@ -37,6 +39,9 @@ public abstract class Node<T extends Node<T>> {
 
     public Node(Location location, List<T> children) {
         this.location = (location != null ? location : Location.EMPTY);
+        if (children.contains(null)) {
+            throw new SqlIllegalArgumentException("Null children are not allowed");
+        }
         this.children = children;
     }
 

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerRunTests.java

@@ -0,0 +1,49 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+package org.elasticsearch.xpack.sql.optimizer;
+
+import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.xpack.sql.analysis.analyzer.Analyzer;
+import org.elasticsearch.xpack.sql.analysis.index.EsIndex;
+import org.elasticsearch.xpack.sql.analysis.index.IndexResolution;
+import org.elasticsearch.xpack.sql.expression.function.FunctionRegistry;
+import org.elasticsearch.xpack.sql.parser.SqlParser;
+import org.elasticsearch.xpack.sql.plan.logical.LogicalPlan;
+import org.elasticsearch.xpack.sql.type.EsField;
+import org.elasticsearch.xpack.sql.type.TypesTests;
+
+import java.util.Map;
+import java.util.TimeZone;
+
+public class OptimizerRunTests extends ESTestCase {
+
+    private final SqlParser parser;
+    private final IndexResolution getIndexResult;
+    private final FunctionRegistry functionRegistry;
+    private final Analyzer analyzer;
+    private final Optimizer optimizer;
+
+    public OptimizerRunTests() {
+        parser = new SqlParser();
+        functionRegistry = new FunctionRegistry();
+
+        Map<String, EsField> mapping = TypesTests.loadMapping("mapping-multi-field-variation.json");
+
+        EsIndex test = new EsIndex("test", mapping);
+        getIndexResult = IndexResolution.valid(test);
+        analyzer = new Analyzer(functionRegistry, getIndexResult, TimeZone.getTimeZone("UTC"));
+        optimizer = new Optimizer();
+    }
+
+    private LogicalPlan plan(String sql) {
+        return optimizer.optimize(analyzer.analyze(parser.createStatement(sql)));
+    }
+
+    public void testWhereClause() {
+        LogicalPlan p = plan("SELECT some.string l FROM test WHERE int IS NOT NULL AND int < 10005 ORDER BY int");
+        assertNotNull(p);
+    }
+}

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerTests.java

@@ -9,6 +9,7 @@ import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xpack.sql.expression.Alias;
 import org.elasticsearch.xpack.sql.expression.Expression;
 import org.elasticsearch.xpack.sql.expression.Expressions;
+import org.elasticsearch.xpack.sql.expression.FieldAttribute;
 import org.elasticsearch.xpack.sql.expression.Literal;
 import org.elasticsearch.xpack.sql.expression.NamedExpression;
 import org.elasticsearch.xpack.sql.expression.Order;
@@ -49,8 +50,10 @@ import org.elasticsearch.xpack.sql.expression.regex.RLike;
 import org.elasticsearch.xpack.sql.optimizer.Optimizer.BinaryComparisonSimplification;
 import org.elasticsearch.xpack.sql.optimizer.Optimizer.BooleanLiteralsOnTheRight;
 import org.elasticsearch.xpack.sql.optimizer.Optimizer.BooleanSimplification;
+import org.elasticsearch.xpack.sql.optimizer.Optimizer.CombineBinaryComparisons;
 import org.elasticsearch.xpack.sql.optimizer.Optimizer.CombineProjections;
 import org.elasticsearch.xpack.sql.optimizer.Optimizer.ConstantFolding;
+import org.elasticsearch.xpack.sql.optimizer.Optimizer.PropagateEquals;
 import org.elasticsearch.xpack.sql.optimizer.Optimizer.PruneDuplicateFunctions;
 import org.elasticsearch.xpack.sql.optimizer.Optimizer.PruneSubqueryAliases;
 import org.elasticsearch.xpack.sql.optimizer.Optimizer.ReplaceFoldableAttributes;
@@ -65,7 +68,7 @@ import org.elasticsearch.xpack.sql.session.EmptyExecutable;
 import org.elasticsearch.xpack.sql.tree.Location;
 import org.elasticsearch.xpack.sql.tree.NodeInfo;
 import org.elasticsearch.xpack.sql.type.DataType;
-import org.joda.time.DateTimeZone;
+import org.elasticsearch.xpack.sql.type.EsField;
 
 import java.util.Arrays;
 import java.util.Collections;
@@ -74,6 +77,7 @@ import java.util.TimeZone;
 
 import static java.util.Arrays.asList;
 import static java.util.Collections.emptyList;
+import static java.util.Collections.emptyMap;
 import static java.util.Collections.singletonList;
 import static org.elasticsearch.xpack.sql.tree.Location.EMPTY;
 
@@ -217,6 +221,10 @@ public class OptimizerTests extends ESTestCase {
         assertEquals(10, lt.right().fold());
     }
 
+    //
+    // Constant folding
+    //
+
     public void testConstantFolding() {
         Expression exp = new Add(EMPTY, L(2), L(3));
 
@@ -314,6 +322,10 @@ public class OptimizerTests extends ESTestCase {
         return l.value();
     }
 
+    //
+    // Logical simplifications
+    //
+
     public void testBinaryComparisonSimplification() {
         assertEquals(Literal.TRUE, new BinaryComparisonSimplification().rule(new Equals(EMPTY, L(5), L(5))));
         assertEquals(Literal.TRUE, new BinaryComparisonSimplification().rule(new GreaterThanOrEqual(EMPTY, L(5), L(5))));
@@ -369,4 +381,512 @@ public class OptimizerTests extends ESTestCase {
 
         assertEquals(expected, simplification.rule(actual));
     }
-}
+
+    //
+    // Range optimization
+    //
+
+    // 6 < a <= 5  -> FALSE
+    public void testFoldExcludingRangeToFalse() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+
+        Range r = new Range(EMPTY, fa, L(6), false, L(5), true);
+        assertTrue(r.foldable());
+        assertEquals(Boolean.FALSE, r.fold());
+    }
+
+    // 6 < a <= 5.5 -> FALSE
+    public void testFoldExcludingRangeWithDifferentTypesToFalse() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+
+        Range r = new Range(EMPTY, fa, L(6), false, L(5.5d), true);
+        assertTrue(r.foldable());
+        assertEquals(Boolean.FALSE, r.fold());
+    }
+
+    // Conjunction
+
+    public void testCombineBinaryComparisonsNotComparable() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+        LessThanOrEqual lte = new LessThanOrEqual(EMPTY, fa, L(6));
+        LessThan lt = new LessThan(EMPTY, fa, Literal.FALSE);
+
+        CombineBinaryComparisons rule = new CombineBinaryComparisons();
+        And and = new And(EMPTY, lte, lt);
+        Expression exp = rule.rule(and);
+        assertEquals(exp, and);
+    }
+
+    // a <= 6 AND a < 5  -> a < 5
+    public void testCombineBinaryComparisonsUpper() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+        LessThanOrEqual lte = new LessThanOrEqual(EMPTY, fa, L(6));
+        LessThan lt = new LessThan(EMPTY, fa, L(5));
+
+        CombineBinaryComparisons rule = new CombineBinaryComparisons();
+
+        Expression exp = rule.rule(new And(EMPTY, lte, lt));
+        assertEquals(LessThan.class, exp.getClass());
+        LessThan r = (LessThan) exp;
+        assertEquals(L(5), r.right());
+    }
+
+    // 6 <= a AND 5 < a  -> 6 <= a
+    public void testCombineBinaryComparisonsLower() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+        GreaterThanOrEqual gte = new GreaterThanOrEqual(EMPTY, fa, L(6));
+        GreaterThan gt = new GreaterThan(EMPTY, fa, L(5));
+
+        CombineBinaryComparisons rule = new CombineBinaryComparisons();
+
+        Expression exp = rule.rule(new And(EMPTY, gte, gt));
+        assertEquals(GreaterThanOrEqual.class, exp.getClass());
+        GreaterThanOrEqual r = (GreaterThanOrEqual) exp;
+        assertEquals(L(6), r.right());
+    }
+
+    // 5 <= a AND 5 < a  -> 5 < a
+    public void testCombineBinaryComparisonsInclude() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+        GreaterThanOrEqual gte = new GreaterThanOrEqual(EMPTY, fa, L(5));
+        GreaterThan gt = new GreaterThan(EMPTY, fa, L(5));
+
+        CombineBinaryComparisons rule = new CombineBinaryComparisons();
+
+        Expression exp = rule.rule(new And(EMPTY, gte, gt));
+        assertEquals(GreaterThan.class, exp.getClass());
+        GreaterThan r = (GreaterThan) exp;
+        assertEquals(L(5), r.right());
+    }
+
+    // 3 <= a AND 4 < a AND a <= 7 AND a < 6 -> 4 < a < 6
+    public void testCombineMultipleBinaryComparisons() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+        GreaterThanOrEqual gte = new GreaterThanOrEqual(EMPTY, fa, L(3));
+        GreaterThan gt = new GreaterThan(EMPTY, fa, L(4));
+        LessThanOrEqual lte = new LessThanOrEqual(EMPTY, fa, L(7));
+        LessThan lt = new LessThan(EMPTY, fa, L(6));
+
+        CombineBinaryComparisons rule = new CombineBinaryComparisons();
+
+        Expression exp = rule.rule(new And(EMPTY, gte, new And(EMPTY, gt, new And(EMPTY, lt, lte))));
+        assertEquals(Range.class, exp.getClass());
+        Range r = (Range) exp;
+        assertEquals(L(4), r.lower());
+        assertFalse(r.includeLower());
+        assertEquals(L(6), r.upper());
+        assertFalse(r.includeUpper());
+    }
+
+    // 3 <= a AND TRUE AND 4 < a AND a != 5 AND a <= 7 -> 4 < a <= 7 AND a != 5 AND TRUE
+    public void testCombineMixedMultipleBinaryComparisons() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+        GreaterThanOrEqual gte = new GreaterThanOrEqual(EMPTY, fa, L(3));
+        GreaterThan gt = new GreaterThan(EMPTY, fa, L(4));
+        LessThanOrEqual lte = new LessThanOrEqual(EMPTY, fa, L(7));
+        Expression ne = new Not(EMPTY, new Equals(EMPTY, fa, L(5)));
+
+        CombineBinaryComparisons rule = new CombineBinaryComparisons();
+
+        // TRUE AND a != 5 AND 4 < a <= 7
+        Expression exp = rule.rule(new And(EMPTY, gte, new And(EMPTY, Literal.TRUE, new And(EMPTY, gt, new And(EMPTY, ne, lte)))));
+        assertEquals(And.class, exp.getClass());
+        And and = ((And) exp);
+        assertEquals(Range.class, and.right().getClass());
+        Range r = (Range) and.right();
+        assertEquals(L(4), r.lower());
+        assertFalse(r.includeLower());
+        assertEquals(L(7), r.upper());
+        assertTrue(r.includeUpper());
+    }
+
+    // 1 <= a AND a < 5  -> 1 <= a < 5
+    public void testCombineComparisonsIntoRange() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+        GreaterThanOrEqual gte = new GreaterThanOrEqual(EMPTY, fa, L(1));
+        LessThan lt = new LessThan(EMPTY, fa, L(5));
+
+        CombineBinaryComparisons rule = new CombineBinaryComparisons();
+        Expression exp = rule.rule(new And(EMPTY, gte, lt));
+        assertEquals(Range.class, rule.rule(exp).getClass());
+
+        Range r = (Range) exp;
+        assertEquals(L(1), r.lower());
+        assertTrue(r.includeLower());
+        assertEquals(L(5), r.upper());
+        assertFalse(r.includeUpper());
+    }
+
+    // a != NULL AND a > 1 AND a < 5 AND a == 10  -> (a != NULL AND a == 10) AND 1 <= a < 5
+    public void testCombineUnbalancedComparisonsMixedWithEqualsIntoRange() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+        IsNotNull isn = new IsNotNull(EMPTY, fa);
+        GreaterThanOrEqual gte = new GreaterThanOrEqual(EMPTY, fa, L(1));
+
+        Equals eq = new Equals(EMPTY, fa, L(10));
+        LessThan lt = new LessThan(EMPTY, fa, L(5));
+
+        And and = new And(EMPTY, new And(EMPTY, isn, gte), new And(EMPTY, lt, eq));
+
+        CombineBinaryComparisons rule = new CombineBinaryComparisons();
+        Expression exp = rule.rule(and);
+        assertEquals(And.class, exp.getClass());
+        And a = (And) exp;
+        assertEquals(Range.class, a.right().getClass());
+
+        Range r = (Range) a.right();
+        assertEquals(L(1), r.lower());
+        assertTrue(r.includeLower());
+        assertEquals(L(5), r.upper());
+        assertFalse(r.includeUpper());
+    }
+
+    // (2 < a < 3) AND (1 < a < 4) -> (2 < a < 3)
+    public void testCombineBinaryComparisonsConjunctionOfIncludedRange() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+
+        Range r1 = new Range(EMPTY, fa, L(2), false, L(3), false);
+        Range r2 = new Range(EMPTY, fa, L(1), false, L(4), false);
+
+        And and = new And(EMPTY, r1, r2);
+
+        CombineBinaryComparisons rule = new CombineBinaryComparisons();
+        Expression exp = rule.rule(and);
+        assertEquals(r1, exp);
+    }
+
+    // (2 < a < 3) AND a < 2 -> 2 < a < 2
+    public void testCombineBinaryComparisonsConjunctionOfNonOverlappingBoundaries() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+
+        Range r1 = new Range(EMPTY, fa, L(2), false, L(3), false);
+        Range r2 = new Range(EMPTY, fa, L(1), false, L(2), false);
+
+        And and = new And(EMPTY, r1, r2);
+
+        CombineBinaryComparisons rule = new CombineBinaryComparisons();
+        Expression exp = rule.rule(and);
+        assertEquals(Range.class, exp.getClass());
+        Range r = (Range) exp;
+        assertEquals(L(2), r.lower());
+        assertFalse(r.includeLower());
+        assertEquals(L(2), r.upper());
+        assertFalse(r.includeUpper());
+        assertEquals(Boolean.FALSE, r.fold());
+    }
+
+    // (2 < a < 3) AND (2 < a <= 3) -> 2 < a < 3
+    public void testCombineBinaryComparisonsConjunctionOfUpperEqualsOverlappingBoundaries() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+
+        Range r1 = new Range(EMPTY, fa, L(2), false, L(3), false);
+        Range r2 = new Range(EMPTY, fa, L(2), false, L(3), true);
+
+        And and = new And(EMPTY, r1, r2);
+
+        CombineBinaryComparisons rule = new CombineBinaryComparisons();
+        Expression exp = rule.rule(and);
+        assertEquals(r1, exp);
+    }
+
+    // (2 < a < 3) AND (1 < a < 3) -> 2 < a < 3
+    public void testCombineBinaryComparisonsConjunctionOverlappingUpperBoundary() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+
+        Range r2 = new Range(EMPTY, fa, L(2), false, L(3), false);
+        Range r1 = new Range(EMPTY, fa, L(1), false, L(3), false);
+
+        And and = new And(EMPTY, r1, r2);
+
+        CombineBinaryComparisons rule = new CombineBinaryComparisons();
+        Expression exp = rule.rule(and);
+        assertEquals(r2, exp);
+    }
+
+    // (2 < a <= 3) AND (1 < a < 3) -> 2 < a < 3
+    public void testCombineBinaryComparisonsConjunctionWithDifferentUpperLimitInclusion() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+
+        Range r1 = new Range(EMPTY, fa, L(1), false, L(3), false);
+        Range r2 = new Range(EMPTY, fa, L(2), false, L(3), true);
+
+        And and = new And(EMPTY, r1, r2);
+
+        CombineBinaryComparisons rule = new CombineBinaryComparisons();
+        Expression exp = rule.rule(and);
+        assertEquals(Range.class, exp.getClass());
+        Range r = (Range) exp;
+        assertEquals(L(2), r.lower());
+        assertFalse(r.includeLower());
+        assertEquals(L(3), r.upper());
+        assertFalse(r.includeUpper());
+    }
+
+    // (0 < a <= 1) AND (0 <= a < 2) -> 0 < a <= 1
+    public void testRangesOverlappingConjunctionNoLowerBoundary() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+
+        Range r1 = new Range(EMPTY, fa, L(0), false, L(1), true);
+        Range r2 = new Range(EMPTY, fa, L(0), true, L(2), false);
+
+        And and = new And(EMPTY, r1, r2);
+
+        CombineBinaryComparisons rule = new CombineBinaryComparisons();
+        Expression exp = rule.rule(and);
+        assertEquals(r1, exp);
+    }
+
+    // Disjunction
+
+    public void testCombineBinaryComparisonsDisjunctionNotComparable() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+
+        GreaterThan gt1 = new GreaterThan(EMPTY, fa, L(1));
+        GreaterThan gt2 = new GreaterThan(EMPTY, fa, Literal.FALSE);
+
+        Or or = new Or(EMPTY, gt1, gt2);
+
+        CombineBinaryComparisons rule = new CombineBinaryComparisons();
+        Expression exp = rule.rule(or);
+        assertEquals(exp, or);
+    }
+
+
+    // 2 < a OR 1 < a OR 3 < a -> 1 < a
+    public void testCombineBinaryComparisonsDisjunctionLowerBound() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+
+        GreaterThan gt1 = new GreaterThan(EMPTY, fa, L(1));
+        GreaterThan gt2 = new GreaterThan(EMPTY, fa, L(2));
+        GreaterThan gt3 = new GreaterThan(EMPTY, fa, L(3));
+
+        Or or = new Or(EMPTY, gt1, new Or(EMPTY, gt2, gt3));
+
+        CombineBinaryComparisons rule = new CombineBinaryComparisons();
+        Expression exp = rule.rule(or);
+        assertEquals(GreaterThan.class, exp.getClass());
+
+        GreaterThan gt = (GreaterThan) exp;
+        assertEquals(L(1), gt.right());
+    }
+
+    // 2 < a OR 1 < a OR 3 <= a -> 1 < a
+    public void testCombineBinaryComparisonsDisjunctionIncludeLowerBounds() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+
+        GreaterThan gt1 = new GreaterThan(EMPTY, fa, L(1));
+        GreaterThan gt2 = new GreaterThan(EMPTY, fa, L(2));
+        GreaterThanOrEqual gte3 = new GreaterThanOrEqual(EMPTY, fa, L(3));
+
+        Or or = new Or(EMPTY, new Or(EMPTY, gt1, gt2), gte3);
+
+        CombineBinaryComparisons rule = new CombineBinaryComparisons();
+        Expression exp = rule.rule(or);
+        assertEquals(GreaterThan.class, exp.getClass());
+
+        GreaterThan gt = (GreaterThan) exp;
+        assertEquals(L(1), gt.right());
+    }
+
+    // a < 1 OR a < 2 OR a < 3 ->  a < 3
+    public void testCombineBinaryComparisonsDisjunctionUpperBound() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+
+        LessThan lt1 = new LessThan(EMPTY, fa, L(1));
+        LessThan lt2 = new LessThan(EMPTY, fa, L(2));
+        LessThan lt3 = new LessThan(EMPTY, fa, L(3));
+
+        Or or = new Or(EMPTY, new Or(EMPTY, lt1, lt2), lt3);
+
+        CombineBinaryComparisons rule = new CombineBinaryComparisons();
+        Expression exp = rule.rule(or);
+        assertEquals(LessThan.class, exp.getClass());
+
+        LessThan lt = (LessThan) exp;
+        assertEquals(L(3), lt.right());
+    }
+
+    // a < 2 OR a <= 2 OR a < 1 ->  a <= 2
+    public void testCombineBinaryComparisonsDisjunctionIncludeUpperBounds() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+
+        LessThan lt1 = new LessThan(EMPTY, fa, L(1));
+        LessThan lt2 = new LessThan(EMPTY, fa, L(2));
+        LessThanOrEqual lte2 = new LessThanOrEqual(EMPTY, fa, L(2));
+
+        Or or = new Or(EMPTY, lt2, new Or(EMPTY, lte2, lt1));
+
+        CombineBinaryComparisons rule = new CombineBinaryComparisons();
+        Expression exp = rule.rule(or);
+        assertEquals(LessThanOrEqual.class, exp.getClass());
+
+        LessThanOrEqual lte = (LessThanOrEqual) exp;
+        assertEquals(L(2), lte.right());
+    }
+
+    // a < 2 OR 3 < a OR a < 1 OR 4 < a ->  a < 2 OR 3 < a
+    public void testCombineBinaryComparisonsDisjunctionOfLowerAndUpperBounds() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+
+        LessThan lt1 = new LessThan(EMPTY, fa, L(1));
+        LessThan lt2 = new LessThan(EMPTY, fa, L(2));
+
+        GreaterThan gt3 = new GreaterThan(EMPTY, fa, L(3));
+        GreaterThan gt4 = new GreaterThan(EMPTY, fa, L(4));
+
+        Or or = new Or(EMPTY, new Or(EMPTY, lt2, gt3), new Or(EMPTY, lt1, gt4));
+
+        CombineBinaryComparisons rule = new CombineBinaryComparisons();
+        Expression exp = rule.rule(or);
+        assertEquals(Or.class, exp.getClass());
+
+        Or ro = (Or) exp;
+
+        assertEquals(LessThan.class, ro.left().getClass());
+        LessThan lt = (LessThan) ro.left();
+        assertEquals(L(2), lt.right());
+        assertEquals(GreaterThan.class, ro.right().getClass());
+        GreaterThan gt = (GreaterThan) ro.right();
+        assertEquals(L(3), gt.right());
+    }
+
+    // (2 < a < 3) OR (1 < a < 4) -> (1 < a < 4)
+    public void testCombineBinaryComparisonsDisjunctionOfIncludedRangeNotComparable() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+
+        Range r1 = new Range(EMPTY, fa, L(2), false, L(3), false);
+        Range r2 = new Range(EMPTY, fa, L(1), false, Literal.FALSE, false);
+
+        Or or = new Or(EMPTY, r1, r2);
+
+        CombineBinaryComparisons rule = new CombineBinaryComparisons();
+        Expression exp = rule.rule(or);
+        assertEquals(or, exp);
+    }
+
+
+    // (2 < a < 3) OR (1 < a < 4) -> (1 < a < 4)
+    public void testCombineBinaryComparisonsDisjunctionOfIncludedRange() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+
+        Range r1 = new Range(EMPTY, fa, L(2), false, L(3), false);
+        Range r2 = new Range(EMPTY, fa, L(1), false, L(4), false);
+
+        Or or = new Or(EMPTY, r1, r2);
+
+        CombineBinaryComparisons rule = new CombineBinaryComparisons();
+        Expression exp = rule.rule(or);
+        assertEquals(Range.class, exp.getClass());
+
+        Range r = (Range) exp;
+        assertEquals(L(1), r.lower());
+        assertFalse(r.includeLower());
+        assertEquals(L(4), r.upper());
+        assertFalse(r.includeUpper());
+    }
+
+    // (2 < a < 3) OR (1 < a < 2) -> same
+    public void testCombineBinaryComparisonsDisjunctionOfNonOverlappingBoundaries() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+
+        Range r1 = new Range(EMPTY, fa, L(2), false, L(3), false);
+        Range r2 = new Range(EMPTY, fa, L(1), false, L(2), false);
+
+        Or or = new Or(EMPTY, r1, r2);
+
+        CombineBinaryComparisons rule = new CombineBinaryComparisons();
+        Expression exp = rule.rule(or);
+        assertEquals(or, exp);
+    }
+
+    // (2 < a < 3) OR (2 < a <= 3) -> 2 < a <= 3
+    public void testCombineBinaryComparisonsDisjunctionOfUpperEqualsOverlappingBoundaries() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+
+        Range r1 = new Range(EMPTY, fa, L(2), false, L(3), false);
+        Range r2 = new Range(EMPTY, fa, L(2), false, L(3), true);
+
+        Or or = new Or(EMPTY, r1, r2);
+
+        CombineBinaryComparisons rule = new CombineBinaryComparisons();
+        Expression exp = rule.rule(or);
+        assertEquals(r2, exp);
+    }
+
+    // (2 < a < 3) OR (1 < a < 3) -> 1 < a < 3
+    public void testCombineBinaryComparisonsOverlappingUpperBoundary() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+
+        Range r2 = new Range(EMPTY, fa, L(2), false, L(3), false);
+        Range r1 = new Range(EMPTY, fa, L(1), false, L(3), false);
+
+        Or or = new Or(EMPTY, r1, r2);
+
+        CombineBinaryComparisons rule = new CombineBinaryComparisons();
+        Expression exp = rule.rule(or);
+        assertEquals(r1, exp);
+    }
+
+    // (2 < a <= 3) OR (1 < a < 3) -> same (the <= prevents the ranges from being combined)
+    public void testCombineBinaryComparisonsWithDifferentUpperLimitInclusion() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+
+        Range r1 = new Range(EMPTY, fa, L(1), false, L(3), false);
+        Range r2 = new Range(EMPTY, fa, L(2), false, L(3), true);
+
+        Or or = new Or(EMPTY, r1, r2);
+
+        CombineBinaryComparisons rule = new CombineBinaryComparisons();
+        Expression exp = rule.rule(or);
+        assertEquals(or, exp);
+    }
+
+    // (0 < a <= 1) OR (0 < a < 2) -> 0 < a < 2
+    public void testRangesOverlappingNoLowerBoundary() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+
+        Range r2 = new Range(EMPTY, fa, L(0), false, L(2), false);
+        Range r1 = new Range(EMPTY, fa, L(0), false, L(1), true);
+
+        Or or = new Or(EMPTY, r1, r2);
+
+        CombineBinaryComparisons rule = new CombineBinaryComparisons();
+        Expression exp = rule.rule(or);
+        assertEquals(r2, exp);
+    }
+
+    // Equals
+
+    // a == 1 AND a == 2 -> FALSE
+    public void testDualEqualsConjunction() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+        Equals eq1 = new Equals(EMPTY, fa, L(1));
+        Equals eq2 = new Equals(EMPTY, fa, L(2));
+
+        PropagateEquals rule = new PropagateEquals();
+        Expression exp = rule.rule(new And(EMPTY, eq1, eq2));
+        assertEquals(Literal.FALSE, rule.rule(exp));
+    }
+
+    // 1 <= a < 10 AND a == 1 -> a == 1
+    public void testEliminateRangeByEqualsInInterval() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+        Equals eq1 = new Equals(EMPTY, fa, L(1));
+        Range r = new Range(EMPTY, fa, L(1), true, L(10), false);
+
+        PropagateEquals rule = new PropagateEquals();
+        Expression exp = rule.rule(new And(EMPTY, eq1, r));
+        assertEquals(eq1, rule.rule(exp));
+    }
+
+    // 1 < a < 10 AND a == 10 -> FALSE
+    public void testEliminateRangeByEqualsOutsideInterval() {
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
+        Equals eq1 = new Equals(EMPTY, fa, L(10));
+        Range r = new Range(EMPTY, fa, L(1), false, L(10), false);
+
+        PropagateEquals rule = new PropagateEquals();
+        Expression exp = rule.rule(new And(EMPTY, eq1, r));
+        assertEquals(Literal.FALSE, rule.rule(exp));
+    }
+}

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/MatchQueryTests.java

@@ -8,16 +8,21 @@ package org.elasticsearch.xpack.sql.querydsl.query;
 import org.elasticsearch.index.query.MatchQueryBuilder;
 import org.elasticsearch.index.query.Operator;
 import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.xpack.sql.expression.FieldAttribute;
 import org.elasticsearch.xpack.sql.expression.predicate.fulltext.MatchQueryPredicate;
 import org.elasticsearch.xpack.sql.tree.Location;
 import org.elasticsearch.xpack.sql.tree.LocationTests;
+import org.elasticsearch.xpack.sql.type.DataType;
+import org.elasticsearch.xpack.sql.type.EsField;
 
 import java.util.Arrays;
 import java.util.List;
 import java.util.function.Function;
 
-import static org.hamcrest.Matchers.equalTo;
+import static java.util.Collections.emptyMap;
 import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode;
+import static org.elasticsearch.xpack.sql.tree.Location.EMPTY;
+import static org.hamcrest.Matchers.equalTo;
 
 public class MatchQueryTests extends ESTestCase {
     static MatchQuery randomMatchQuery() {
@@ -62,14 +67,16 @@ public class MatchQueryTests extends ESTestCase {
 
     private static MatchQueryBuilder getBuilder(String options) {
         final Location location = new Location(1, 1);
-        final MatchQueryPredicate mmqp = new MatchQueryPredicate(location, null, "eggplant", options);
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.KEYWORD, emptyMap(), true));
+        final MatchQueryPredicate mmqp = new MatchQueryPredicate(location, fa, "eggplant", options);
         final MatchQuery mmq = new MatchQuery(location, "eggplant", "foo", mmqp);
         return (MatchQueryBuilder) mmq.asBuilder();
     }
 
     public void testToString() {
         final Location location = new Location(1, 1);
-        final MatchQueryPredicate mmqp = new MatchQueryPredicate(location, null, "eggplant", "");
+        FieldAttribute fa = new FieldAttribute(EMPTY, "a", new EsField("af", DataType.KEYWORD, emptyMap(), true));
+        final MatchQueryPredicate mmqp = new MatchQueryPredicate(location, fa, "eggplant", "");
         final MatchQuery mmq = new MatchQuery(location, "eggplant", "foo", mmqp);
         assertEquals("MatchQuery@1:2[eggplant:foo]", mmq.toString());
     }