honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

SQL: Predicate diff takes into account all values (#41346) 

Fix bug in predicate subtraction that caused the evaluation to be
skipped on the first mismatch instead of evaluating the whole list. In
some cases this caused not only an incorrect result but one that kept on
growing causing the engine to bail

Fix #40835

(cherry picked from commit bd2b33d6eaca616a5acd846204e2d12f905854d4)
This commit is contained in:
Costin Leau 2019-04-18 22:44:16 +03:00 committed by Costin Leau
parent cfed5d65be
commit 5f497d4274
5 changed files with 38 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/capabilities/UnresolvedException.java
View File

@ -7,16 +7,12 @@ package org.elasticsearch.xpack.sql.capabilities;
import org.elasticsearch.xpack.sql.ServerSqlException;
import java.util.Locale;
import static java.lang.String.format;
/**
* Thrown when we accidentally attempt to resolve something on on an unresolved entity. Throwing this
* is always a bug.
*/
public class UnresolvedException extends ServerSqlException {
public UnresolvedException(String action, Object target) {
super(format(Locale.ROOT, "Invalid call to %s on an unresolved object %s", action, target));
super("Invalid call to {} on an unresolved object {}", action, target);
}
}

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

@ -97,14 +97,19 @@ public abstract class Predicates {
return common.isEmpty() ? emptyList() : common;
}
public static List<Expression> subtract(List<Expression> from, List<Expression> r) {
List<Expression> diff = new ArrayList<>(Math.min(from.size(), r.size()));
for (Expression lExp : from) {
for (Expression rExp : r) {
if (!lExp.semanticEquals(rExp)) {
diff.add(lExp);
public static List<Expression> subtract(List<Expression> from, List<Expression> list) {
List<Expression> diff = new ArrayList<>(Math.min(from.size(), list.size()));
for (Expression f : from) {
boolean found = false;
for (Expression l : list) {
if (f.semanticEquals(l)) {
found = true;
break;
}
}
if (found == false) {
diff.add(f);
}
}
return diff.isEmpty() ? emptyList() : diff;
}

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

@ -1236,7 +1236,7 @@ public class Optimizer extends RuleExecutor<LogicalPlan> {
@Override
protected Expression rule(Expression e) {
if (e instanceof BinaryPredicate) {
if (e instanceof And || e instanceof Or) {
return simplifyAndOr((BinaryPredicate<?, ?, ?, ?>) e);
}
if (e instanceof Not) {

4
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/rule/RuleExecutor.java
View File

@ -38,7 +38,7 @@ public abstract class RuleExecutor<TreeType extends Node<TreeType>> {
boolean reached(int runs) {
if (runs >= this.runs) {
throw new RuleExecutionException("Rule execution limit %d reached", runs);
throw new RuleExecutionException("Rule execution limit [{}] reached", runs);
}
return false;
}
@ -139,7 +139,7 @@ public abstract class RuleExecutor<TreeType extends Node<TreeType>> {
for (Batch batch : batches) {
int batchRuns = 0;
List<Transformation> tfs = new ArrayList<Transformation>();
List<Transformation> tfs = new ArrayList<>();
transformations.put(batch, tfs);
boolean hasChanged = false;

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

@ -181,9 +181,12 @@ public class OptimizerTests extends ESTestCase {
}
private static FieldAttribute getFieldAttribute() {
return new FieldAttribute(EMPTY, "a", new EsField("af", DataType.INTEGER, emptyMap(), true));
return getFieldAttribute("a");
}
private static FieldAttribute getFieldAttribute(String name) {
return new FieldAttribute(EMPTY, name, new EsField(name + "f", DataType.INTEGER, emptyMap(), true));
}
public void testPruneSubqueryAliases() {
ShowTables s = new ShowTables(EMPTY, null, null);
@ -1145,6 +1148,23 @@ public class OptimizerTests extends ESTestCase {
assertEquals(or, exp);
}
// (a = 1 AND b = 3 AND c = 4) OR (a = 2 AND b = 3 AND c = 4) -> (b = 3 AND c = 4) AND (a = 1 OR a = 2)
public void testBooleanSimplificationCommonExpressionSubstraction() {
FieldAttribute fa = getFieldAttribute("a");
FieldAttribute fb = getFieldAttribute("b");
FieldAttribute fc = getFieldAttribute("c");
Expression a1 = new Equals(EMPTY, fa, ONE);
Expression a2 = new Equals(EMPTY, fa, TWO);
And common = new And(EMPTY, new Equals(EMPTY, fb, THREE), new Equals(EMPTY, fc, FOUR));
And left = new And(EMPTY, a1, common);
And right = new And(EMPTY, a2, common);
Or or = new Or(EMPTY, left, right);
Expression exp = new BooleanSimplification().rule(or);
assertEquals(new And(EMPTY, common, new Or(EMPTY, a1, a2)), exp);
}
// (0 < a <= 1) OR (0 < a < 2) -> 0 < a < 2
public void testRangesOverlappingNoLowerBoundary() {
FieldAttribute fa = getFieldAttribute();
@ -1289,7 +1309,7 @@ public class OptimizerTests extends ESTestCase {
Order firstOrderBy = new Order(EMPTY, firstField, OrderDirection.ASC, Order.NullsPosition.LAST);
Order secondOrderBy = new Order(EMPTY, secondField, OrderDirection.ASC, Order.NullsPosition.LAST);
OrderBy orderByPlan = new OrderBy(EMPTY,
OrderBy orderByPlan = new OrderBy(EMPTY,
new Aggregate(EMPTY, FROM(), Arrays.asList(secondField, firstField), Arrays.asList(secondAlias, firstAlias)),
Arrays.asList(firstOrderBy, secondOrderBy));
LogicalPlan result = new Optimizer.SortAggregateOnOrderBy().apply(orderByPlan);
@ -1321,7 +1341,7 @@ public class OptimizerTests extends ESTestCase {
Order firstOrderBy = new Order(EMPTY, firstAlias, OrderDirection.ASC, Order.NullsPosition.LAST);
Order secondOrderBy = new Order(EMPTY, secondAlias, OrderDirection.ASC, Order.NullsPosition.LAST);
OrderBy orderByPlan = new OrderBy(EMPTY,
OrderBy orderByPlan = new OrderBy(EMPTY,
new Aggregate(EMPTY, FROM(), Arrays.asList(secondAlias, firstAlias), Arrays.asList(secondAlias, firstAlias)),
Arrays.asList(firstOrderBy, secondOrderBy));
LogicalPlan result = new Optimizer.SortAggregateOnOrderBy().apply(orderByPlan);