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.

Fix elastic#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.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 {
@@ -34,10 +33,34 @@ public DataType dataType() {
     }
 
     @SuppressWarnings({ "rawtypes", "unchecked" })
-    static Integer compare(Object left, Object right) {
-        if (left instanceof Comparable && right instanceof Comparable) {
-            return Integer.valueOf(((Comparable) left).compareTo(right));
+    public static Integer compare(Object l, Object r) {
+        // typical number comparison
+        if (l instanceof Number && r instanceof Number) {
+            return compare((Number) l, (Number) r);
+        }
+
+        try {
+            if (l instanceof Comparable && r instanceof Comparable) {
+                return Integer.valueOf(((Comparable) l).compareTo(r));
+            }
+        } catch (ClassCastException cce) {
+            // types are not compatible
+            // 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.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 static List<Expression> splitAnd(Expression exp) {
             list.addAll(splitAnd(and.right()));
             return list;
         }
-        return Collections.singletonList(exp);
+        return singletonList(exp);
     }
 
     public static List<Expression> splitOr(Expression exp) {
@@ -33,15 +36,38 @@ public static List<Expression> splitOr(Expression exp) {
             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' - while a bit longer this method creates a balanced tree as oppose to a plain
+     * recursive approach that 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;
+        }
+
+        List<Expression> result = new ArrayList<>(exps);
+
+        while (result.size() > 1) {
+            // combine (in place) expressions in pairs
+            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 +79,7 @@ public static List<Expression> inCommon(List<Expression> l, List<Expression> r)
                 }
             }
         }
-        return common.isEmpty() ? Collections.emptyList() : common;
+        return common.isEmpty() ? emptyList() : common;
     }
 
     public static List<Expression> subtract(List<Expression> from, List<Expression> r) {
@@ -65,7 +91,7 @@ public static List<Expression> subtract(List<Expression> from, List<Expression>
                 }
             }
         }
-        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.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 boolean includeUpper() {
 
     @Override
     public boolean foldable() {
-        return value.foldable() && lower.foldable() && upper.foldable();
+        if (lower.foldable() && upper.foldable()) {
+            return (excludingBoundaries() || value.foldable());
+        }
+
+        return false;
     }
 
     @Override
     public Object fold() {
+        if (excludingBoundaries()) {
+            return Boolean.FALSE;
+        }
+
         Object val = value.fold();
         Integer lowerCompare = BinaryComparison.compare(lower.fold(), val);
         Integer upperCompare = BinaryComparison.compare(val, upper().fold());
@@ -79,6 +86,13 @@ public Object fold() {
         return lowerComparsion && upperComparsion;
     }
 
+    private boolean excludingBoundaries() {
+        // if the boundaries exclude each other, the value doesn't need to be evaluated
+        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();