Skip to content

Commit

Permalink
SQL: Optimisation fixes for conjunction merges (elastic#50703)
Browse files Browse the repository at this point in the history
* SQL: Optimisation fixes for conjunction merges

This commit fixes the following issues around the way comparisions are
merged with ranges in conjunctions:
* the decision to include the equality of the lower limit is corrected;
* the selection of the upper limit is corrected to use the upper bound
of the range;
* the list of terms in the conjunction is sorted to have the ranges at
the bottom; this allows subsequent binary comarisions to find compatible
ranges and potentially be merged away. The end guarantee being that the
optimisation takes place irrespective of the order of the conjunction
terms in the statement.

Some comments are also corrected.

* adress review observation on anon. comparator

Replace anonymous comparator of split AND Expressions with a lambda.
  • Loading branch information
bpintea authored and SivagurunathanV committed Jan 21, 2020
1 parent 720ec8f commit e6fba66
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
* In a SQL statement, an Expression is whatever a user specifies inside an
* action, so for instance:
*
* {@code SELECT a, b, MAX(c, d) FROM i}
* {@code SELECT a, b, ABS(c) FROM i}
*
* a, b, ABS(c), and i are all Expressions, with ABS(c) being a Function
* (which is a type of expression) with a single child, c.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ protected LogicalPlan rule(OrderBy ob) {
return ob;
}
}

static class PruneOrderByForImplicitGrouping extends OptimizerRule<OrderBy> {

@Override
Expand Down Expand Up @@ -1090,7 +1090,18 @@ private Expression combine(And and) {

boolean changed = false;

for (Expression ex : Predicates.splitAnd(and)) {
List<Expression> andExps = Predicates.splitAnd(and);
// Ranges need to show up before BinaryComparisons in list, to allow the latter be optimized away into a Range, if possible
andExps.sort((o1, o2) -> {
if (o1 instanceof Range && o2 instanceof Range) {
return 0; // keep ranges' order
} else if (o1 instanceof Range || o2 instanceof Range) {
return o2 instanceof Range ? 1 : -1;
} else {
return 0; // keep non-ranges' order
}
});
for (Expression ex : andExps) {
if (ex instanceof Range) {
Range r = (Range) ex;
if (findExistingRange(r, ranges, true)) {
Expand Down Expand Up @@ -1215,9 +1226,9 @@ private static boolean findExistingRange(Range main, List<Range> ranges, boolean
lowerEq = comp == 0 && main.includeLower() == other.includeLower();
// AND
if (conjunctive) {
// (2 < a < 3) AND (1 < a < 3) -> (1 < a < 3)
// (2 < a < 3) AND (1 < a < 3) -> (2 < a < 3)
lower = comp > 0 ||
// (2 < a < 3) AND (2 < a <= 3) -> (2 < a < 3)
// (2 < a < 3) AND (2 <= a < 3) -> (2 < a < 3)
(comp == 0 && !main.includeLower() && other.includeLower());
}
// OR
Expand Down Expand Up @@ -1316,7 +1327,7 @@ private boolean findConjunctiveComparisonInRange(BinaryComparison main, List<Ran
ranges.remove(i);
ranges.add(i,
new Range(other.source(), other.value(),
main.right(), lowerEq ? true : other.includeLower(),
main.right(), lowerEq ? false : main instanceof GreaterThanOrEqual,
other.upper(), other.includeUpper()));
}

Expand All @@ -1325,19 +1336,19 @@ private boolean findConjunctiveComparisonInRange(BinaryComparison main, List<Ran
}
}
} else if (main instanceof LessThan || main instanceof LessThanOrEqual) {
if (other.lower().foldable()) {
Integer comp = BinaryComparison.compare(value, other.lower().fold());
if (other.upper().foldable()) {
Integer comp = BinaryComparison.compare(value, other.upper().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;
boolean upper = comp < 0 || upperEq;

if (upper) {
ranges.remove(i);
ranges.add(i, new Range(other.source(), other.value(),
other.lower(), other.includeLower(),
main.right(), upperEq ? true : other.includeUpper()));
main.right(), upperEq ? false : main instanceof LessThanOrEqual));
}

// found a match
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1012,6 +1012,57 @@ public void testCombineBinaryComparisonsInclude() {
assertEquals(FIVE, r.right());
}

// 2 < a AND (2 <= a < 3) -> 2 < a < 3
public void testCombineBinaryComparisonsAndRangeLower() {
FieldAttribute fa = getFieldAttribute();

GreaterThan gt = new GreaterThan(EMPTY, fa, TWO);
Range range = new Range(EMPTY, fa, TWO, true, THREE, false);

CombineBinaryComparisons rule = new CombineBinaryComparisons();
Expression exp = rule.rule(new And(EMPTY, gt, range));
assertEquals(Range.class, exp.getClass());
Range r = (Range)exp;
assertEquals(TWO, r.lower());
assertFalse(r.includeLower());
assertEquals(THREE, r.upper());
assertFalse(r.includeUpper());
}

// a < 4 AND (1 < a < 3) -> 1 < a < 3
public void testCombineBinaryComparisonsAndRangeUpper() {
FieldAttribute fa = getFieldAttribute();

LessThan lt = new LessThan(EMPTY, fa, FOUR);
Range range = new Range(EMPTY, fa, ONE, false, THREE, false);

CombineBinaryComparisons rule = new CombineBinaryComparisons();
Expression exp = rule.rule(new And(EMPTY, range, lt));
assertEquals(Range.class, exp.getClass());
Range r = (Range)exp;
assertEquals(ONE, r.lower());
assertFalse(r.includeLower());
assertEquals(THREE, r.upper());
assertFalse(r.includeUpper());
}

// a <= 2 AND (1 < a < 3) -> 1 < a <= 2
public void testCombineBinaryComparisonsAndRangeUpperEqual() {
FieldAttribute fa = getFieldAttribute();

LessThanOrEqual lte = new LessThanOrEqual(EMPTY, fa, TWO);
Range range = new Range(EMPTY, fa, ONE, false, THREE, false);

CombineBinaryComparisons rule = new CombineBinaryComparisons();
Expression exp = rule.rule(new And(EMPTY, lte, range));
assertEquals(Range.class, exp.getClass());
Range r = (Range)exp;
assertEquals(ONE, r.lower());
assertFalse(r.includeLower());
assertEquals(TWO, r.upper());
assertTrue(r.includeUpper());
}

// 3 <= a AND 4 < a AND a <= 7 AND a < 6 -> 4 < a < 6
public void testCombineMultipleBinaryComparisons() {
FieldAttribute fa = getFieldAttribute();
Expand Down

0 comments on commit e6fba66

Please sign in to comment.