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

EQL: Fix translation of bool fields (#63694) 

This commit fixes two issues in dealing with bool fields in EQL:
- avoid simplifications of field == true expressions
- adding comparison to clauses on fields missing logic (where bool)

Fix #63693

(cherry picked from commit d10a5d0e842bbd4e0031834de948ceb24da3872b)
(cherry picked from commit 0227da3a275c7f22ff524d99d53e1a79146f9e28)
This commit is contained in:
Costin Leau 2020-10-15 14:22:28 +03:00 committed by Costin Leau
parent 2cc0ccb110
commit 06eae58d40
6 changed files with 116 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -14,6 +14,7 @@ import org.elasticsearch.xpack.eql.plan.physical.LocalRelation;
import org.elasticsearch.xpack.eql.session.Payload.Type;
import org.elasticsearch.xpack.eql.util.MathUtils;
import org.elasticsearch.xpack.ql.expression.Expression;
import org.elasticsearch.xpack.ql.expression.FieldAttribute;
import org.elasticsearch.xpack.ql.expression.Literal;
import org.elasticsearch.xpack.ql.expression.NamedExpression;
import org.elasticsearch.xpack.ql.expression.Order;
@ -21,13 +22,14 @@ import org.elasticsearch.xpack.ql.expression.Order.NullsPosition;
import org.elasticsearch.xpack.ql.expression.Order.OrderDirection;
import org.elasticsearch.xpack.ql.expression.predicate.Predicates;
import org.elasticsearch.xpack.ql.expression.predicate.logical.And;
import org.elasticsearch.xpack.ql.expression.predicate.logical.BinaryLogic;
import org.elasticsearch.xpack.ql.expression.predicate.logical.Or;
import org.elasticsearch.xpack.ql.expression.predicate.nulls.IsNotNull;
import org.elasticsearch.xpack.ql.expression.predicate.nulls.IsNull;
import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.BinaryComparison;
import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.Equals;
import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.NotEquals;
import org.elasticsearch.xpack.ql.optimizer.OptimizerRules.BooleanEqualsSimplification;
import org.elasticsearch.xpack.ql.optimizer.OptimizerRules.BooleanFunctionEqualsElimination;
import org.elasticsearch.xpack.ql.optimizer.OptimizerRules.BooleanLiteralsOnTheRight;
import org.elasticsearch.xpack.ql.optimizer.OptimizerRules.BooleanSimplification;
import org.elasticsearch.xpack.ql.optimizer.OptimizerRules.CombineBinaryComparisons;
@ -49,10 +51,10 @@ import org.elasticsearch.xpack.ql.rule.RuleExecutor;
import org.elasticsearch.xpack.ql.type.DataTypes;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import static java.util.Arrays.asList;
import static java.util.Collections.singletonList;
import static java.util.stream.Collectors.toList;
@ -68,12 +70,15 @@ public class Optimizer extends RuleExecutor<LogicalPlan> {
new ReplaceSurrogateFunction(),
new ReplaceRegexMatch());
Batch syntactic = new Batch("Rewrite Syntactic Sugar", Limiter.ONCE,
new AddMissingEquals());
Batch operators = new Batch("Operator Optimization",
new ConstantFolding(),
// boolean
new BooleanSimplification(),
new BooleanLiteralsOnTheRight(),
new BooleanEqualsSimplification(),
new BooleanFunctionEqualsElimination(),
// needs to occur before BinaryComparison combinations
new ReplaceNullChecks(),
new PropagateEquals(),
@ -99,7 +104,34 @@ public class Optimizer extends RuleExecutor<LogicalPlan> {
Batch label = new Batch("Set as Optimized", Limiter.ONCE,
new SetAsOptimized());
return Arrays.asList(substitutions, operators, constraints, operators, ordering, local, label);
return asList(substitutions, syntactic, operators, constraints, operators, ordering, local, label);
}
private static class AddMissingEquals extends OptimizerRule<Filter> {
@Override
protected LogicalPlan rule(Filter filter) {
// check the condition itself
Expression condition = replaceRawBoolFieldWithEquals(filter.condition());
// otherwise look for binary logic
if (condition == filter.condition()) {
condition = condition.transformUp(b ->
b.replaceChildren(asList(replaceRawBoolFieldWithEquals(b.left()), replaceRawBoolFieldWithEquals(b.right())))
, BinaryLogic.class);
}
if (condition != filter.condition()) {
filter = new Filter(filter.source(), filter.child(), condition);
}
return filter;
}
private Expression replaceRawBoolFieldWithEquals(Expression e) {
if (e instanceof FieldAttribute) {
e = new Equals(e.source(), e, Literal.of(e, Boolean.TRUE));
}
return e;
}
}
private static class ReplaceNullChecks extends OptimizerRule<Filter> {

3
x-pack/plugin/eql/src/test/resources/mapping-default.json
View File

@ -87,6 +87,9 @@
},
"constant_keyword" : {
"type" : "constant_keyword"
},
"bool" : {
"type" : "boolean"
}
}
}

43
x-pack/plugin/eql/src/test/resources/queryfolder_tests.txt
View File

@ -145,6 +145,49 @@ process where endsWith(user_name, "c")
{"wildcard":{"user_name":{"wildcard":"*c","boost":1.0}}}],"adjust_pure_negative":true,"boost":1.0}}
;
fieldNoEquals
process where bool
;
{"bool":{"must":[
{"term":{"bool":{"value":true,
;
fieldNoEqualsInExpression
process where length(file_name) > 0 and bool
;
{"bool":{"must":[
{"term":{"bool":{"value":true,
;
fieldEqualsTrue
process where bool == true
;
{"bool":{"must":[
{"term":{"bool":{"value":true,
;
fieldEqualsFalse
process where bool == false
;
{"bool":{"must":[
{"term":{"bool":{"value":false
;
fieldNotEqualsTrue
process where bool != true
;
{"bool":{"must_not":[
{"term":{"bool":{"value":true
;
fieldNotEqualsFalse
process where bool != false
;
{"bool":{"must_not":[
{"term":{"bool":{"value":false
;
lengthFunctionWithExactSubField
process where length(file_name) > 0
;

5
x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/function/scalar/BinaryScalarFunction.java
View File

@ -31,7 +31,10 @@ public abstract class BinaryScalarFunction extends ScalarFunction {
if (newChildren.size() != 2) {
throw new IllegalArgumentException("expected [2] children but received [" + newChildren.size() + "]");
}
return replaceChildren(newChildren.get(0), newChildren.get(1));
Expression newLeft = newChildren.get(0);
Expression newRight = newChildren.get(1);
return left.equals(newLeft) && right.equals(newRight) ? this : replaceChildren(newLeft, newRight);
}
protected abstract BinaryScalarFunction replaceChildren(Expression newLeft, Expression newRight);

7
x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/optimizer/OptimizerRules.java
View File

@ -9,6 +9,7 @@ import org.elasticsearch.xpack.ql.expression.Expression;
import org.elasticsearch.xpack.ql.expression.Expressions;
import org.elasticsearch.xpack.ql.expression.Literal;
import org.elasticsearch.xpack.ql.expression.Order;
import org.elasticsearch.xpack.ql.expression.function.Function;
import org.elasticsearch.xpack.ql.expression.function.scalar.SurrogateFunction;
import org.elasticsearch.xpack.ql.expression.predicate.BinaryOperator;
import org.elasticsearch.xpack.ql.expression.predicate.BinaryPredicate;
@ -70,15 +71,15 @@ public final class OptimizerRules {
* This rule must always be placed after {@link BooleanLiteralsOnTheRight}, since it looks at TRUE/FALSE literals' existence
* on the right hand-side of the {@link Equals}/{@link NotEquals} expressions.
*/
public static final class BooleanEqualsSimplification extends OptimizerExpressionRule {
public static final class BooleanFunctionEqualsElimination extends OptimizerExpressionRule {
public BooleanEqualsSimplification() {
public BooleanFunctionEqualsElimination() {
super(TransformDirection.UP);
}
@Override
protected Expression rule(Expression e) {
if (e instanceof Equals || e instanceof NotEquals) {
if ((e instanceof Equals || e instanceof NotEquals) && ((BinaryComparison) e).left() instanceof Function) {
// for expression "==" or "!=" TRUE/FALSE, return the expression itself or its negated variant
BinaryComparison bc = (BinaryComparison) e;

36
x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/optimizer/OptimizerRulesTests.java
View File

@ -24,6 +24,7 @@ import org.elasticsearch.xpack.ql.expression.predicate.operator.arithmetic.Div;
import org.elasticsearch.xpack.ql.expression.predicate.operator.arithmetic.Mod;
import org.elasticsearch.xpack.ql.expression.predicate.operator.arithmetic.Mul;
import org.elasticsearch.xpack.ql.expression.predicate.operator.arithmetic.Sub;
import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.BinaryComparison;
import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.Equals;
import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.GreaterThan;
import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.GreaterThanOrEqual;
@ -35,7 +36,7 @@ import org.elasticsearch.xpack.ql.expression.predicate.regex.Like;
import org.elasticsearch.xpack.ql.expression.predicate.regex.LikePattern;
import org.elasticsearch.xpack.ql.expression.predicate.regex.RLike;
import org.elasticsearch.xpack.ql.expression.predicate.regex.RLikePattern;
import org.elasticsearch.xpack.ql.optimizer.OptimizerRules.BooleanEqualsSimplification;
import org.elasticsearch.xpack.ql.optimizer.OptimizerRules.BooleanFunctionEqualsElimination;
import org.elasticsearch.xpack.ql.optimizer.OptimizerRules.BooleanLiteralsOnTheRight;
import org.elasticsearch.xpack.ql.optimizer.OptimizerRules.BooleanSimplification;
import org.elasticsearch.xpack.ql.optimizer.OptimizerRules.CombineBinaryComparisons;
@ -275,20 +276,33 @@ public class OptimizerRulesTests extends ESTestCase {
assertEquals(expected, simplification.rule(actual));
}
public void testBoolEqualsSimplification() {
BooleanEqualsSimplification s = new BooleanEqualsSimplification();
public void testBoolEqualsSimplificationOnExpressions() {
BooleanFunctionEqualsElimination s = new BooleanFunctionEqualsElimination();
Expression exp = new GreaterThan(EMPTY, getFieldAttribute(), L(0), null);
assertEquals(DUMMY_EXPRESSION, s.rule(new Equals(EMPTY, DUMMY_EXPRESSION, TRUE)));
assertEquals(new Not(EMPTY, DUMMY_EXPRESSION), s.rule(new Equals(EMPTY, DUMMY_EXPRESSION, FALSE)));
assertEquals(exp, s.rule(new Equals(EMPTY, exp, TRUE)));
assertEquals(new Not(EMPTY, exp), s.rule(new Equals(EMPTY, exp, FALSE)));
}
assertEquals(new Not(EMPTY, DUMMY_EXPRESSION), s.rule(notEqualsOf(DUMMY_EXPRESSION, TRUE)));
assertEquals(DUMMY_EXPRESSION, s.rule(notEqualsOf(DUMMY_EXPRESSION, FALSE)));
public void testBoolEqualsSimplificationOnFields() {
BooleanFunctionEqualsElimination s = new BooleanFunctionEqualsElimination();
assertEquals(NULL, s.rule(new Equals(EMPTY, NULL, TRUE)));
assertEquals(new Not(EMPTY, NULL), s.rule(new Equals(EMPTY, NULL, FALSE)));
FieldAttribute field = getFieldAttribute();
assertEquals(new Not(EMPTY, NULL), s.rule(notEqualsOf(NULL, TRUE)));
assertEquals(NULL, s.rule(notEqualsOf(NULL, FALSE)));
List<? extends BinaryComparison> comparisons = Arrays.asList(
new Equals(EMPTY, field, TRUE),
new Equals(EMPTY, field, FALSE),
notEqualsOf(field, TRUE),
notEqualsOf(field, FALSE),
new Equals(EMPTY, NULL, TRUE),
new Equals(EMPTY, NULL, FALSE),
notEqualsOf(NULL, TRUE),
notEqualsOf(NULL, FALSE)
);
for (BinaryComparison comparison : comparisons) {
assertEquals(comparison, s.rule(comparison));
}
}
//