From 7790cb5fa93561e2e0ea1250134bef89bd798630 Mon Sep 17 00:00:00 2001 From: Costin Leau Date: Wed, 2 May 2018 19:35:01 +0300 Subject: [PATCH] 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 --- .../predicate/BinaryComparison.java | 38 +- .../sql/expression/predicate/Predicates.java | 53 +- .../xpack/sql/expression/predicate/Range.java | 23 +- .../xpack/sql/optimizer/Optimizer.java | 498 ++++++++++++++++- .../elasticsearch/xpack/sql/tree/Node.java | 5 + .../sql/optimizer/OptimizerRunTests.java | 49 ++ .../xpack/sql/optimizer/OptimizerTests.java | 524 +++++++++++++++++- .../sql/querydsl/query/MatchQueryTests.java | 13 +- 8 files changed, 1155 insertions(+), 48 deletions(-) create mode 100644 x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerRunTests.java diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/BinaryComparison.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/BinaryComparison.java index 0fe94feba16..db1ba1d3cdf 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/BinaryComparison.java +++ b/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) { - 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); } + + 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())); + } } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/Predicates.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/Predicates.java index 7439f6def14..6fb26cb6dcc 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/Predicates.java +++ b/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 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 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 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: + * 
+     *       AND
+     *      /   \
+     *   AND     AND
+     *  /   \   /   \
+     * A     B C     D
+     *
+ * + * 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 exps, BiFunction combiner) { + if (exps.isEmpty()) { + return null; + } + + // clone the list (to modify it) + List 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 inCommon(List l, List r) { @@ -53,7 +92,7 @@ public abstract class Predicates { } } } - return common.isEmpty() ? Collections.emptyList() : common; + return common.isEmpty() ? emptyList() : common; } public static List subtract(List from, List r) { @@ -65,7 +104,7 @@ public abstract class Predicates { } } } - return diff.isEmpty() ? Collections.emptyList() : diff; + return diff.isEmpty() ? emptyList() : diff; } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/Range.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/Range.java index 96e427e90f1..54d541ab406 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/Range.java +++ b/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(); } -} +} \ No newline at end of file diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/optimizer/Optimizer.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/optimizer/Optimizer.java index a56be01c955..de4c0644b79 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/optimizer/Optimizer.java +++ b/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 { 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 { static class BinaryComparisonSimplification extends OptimizerExpressionRule { BinaryComparisonSimplification() { - super(TransformDirection.UP); + super(TransformDirection.DOWN); } @Override @@ -1277,47 +1281,483 @@ public class Optimizer extends RuleExecutor { } } - 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() { - super(TransformDirection.UP); + PropagateEquals() { + 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) { - Expression l = and.left(); - Expression r = and.right(); + // combine conjunction + private Expression propagate(And and) { + List ranges = new ArrayList<>(); + List equals = new ArrayList<>(); + List exps = new ArrayList<>(); - if (l instanceof BinaryComparison && r instanceof BinaryComparison) { - // if the same operator is used - BinaryComparison lb = (BinaryComparison) l; - BinaryComparison rb = (BinaryComparison) r; + boolean changed = false; - - if (lb.left().equals(((BinaryComparison) r).left()) && lb.right() instanceof Literal && rb.right() instanceof Literal) { - // >/>= AND />= - else if ((r instanceof GreaterThan || r instanceof GreaterThanOrEqual) - && (l instanceof LessThan || l instanceof LessThanOrEqual)) { - return new Range(and.location(), rb.left(), rb.right(), r instanceof GreaterThanOrEqual, lb.right(), - l instanceof LessThanOrEqual); + for (Expression ex : Predicates.splitAnd(and)) { + if (ex instanceof Range) { + ranges.add((Range) ex); + } else if (ex instanceof Equals) { + Equals otherEq = (Equals) ex; + // equals on different values evaluate to FALSE + if (otherEq.right().foldable()) { + for (Equals eq : equals) { + // cannot evaluate equals so skip it + if (!eq.right().foldable()) { + continue; + } + 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 ranges = new ArrayList<>(); + List bcs = new ArrayList<>(); + List 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 />= + 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 bcs = new ArrayList<>(); + List ranges = new ArrayList<>(); + List 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 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 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 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 { @Override @@ -1435,4 +1875,4 @@ public class Optimizer extends RuleExecutor { enum TransformDirection { UP, DOWN }; -} +} \ No newline at end of file diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/tree/Node.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/tree/Node.java index c0d885c6dcc..13b17791475 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/tree/Node.java +++ b/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> { public Node(Location location, List children) { this.location = (location != null ? location : Location.EMPTY); + if (children.contains(null)) { + throw new SqlIllegalArgumentException("Null children are not allowed"); + } this.children = children; } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerRunTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerRunTests.java new file mode 100644 index 00000000000..9e2d139460e --- /dev/null +++ b/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 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); + } +} \ No newline at end of file diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerTests.java index a49538c8d53..9f95712b5d4 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerTests.java +++ b/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)); + } +} \ No newline at end of file diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/MatchQueryTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/MatchQueryTests.java index 431a6a146ae..b8b4b2fb32b 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/MatchQueryTests.java +++ b/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()); }