From 895246d6b189d9fad73183ba6adde2531441bd08 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Thu, 4 Jan 2018 15:15:09 +0100 Subject: [PATCH] SQL: Fix simplification of boolean expressions. (elastic/x-pack-elasticsearch#3422) There is an error in the optimizer that causes expressions that look like `a OR FALSE` to not be rewritten to `a`. Original commit: elastic/x-pack-elasticsearch@8d19b77b8b7d6b4588e514b81781316e7d9062c5 --- .../xpack/sql/optimizer/Optimizer.java | 4 +- .../optimizer/BooleanSimplificationTests.java | 95 +++++++++++++++++++ 2 files changed, 97 insertions(+), 2 deletions(-) create mode 100644 sql/server/src/test/java/org/elasticsearch/xpack/sql/optimizer/BooleanSimplificationTests.java diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/optimizer/Optimizer.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/optimizer/Optimizer.java index 9c9a54509bb..45ab0b8606b 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/optimizer/Optimizer.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/optimizer/Optimizer.java @@ -1149,10 +1149,10 @@ public class Optimizer extends RuleExecutor { return TRUE; } - if (TRUE.equals(l)) { + if (FALSE.equals(l)) { return r; } - if (TRUE.equals(r)) { + if (FALSE.equals(r)) { return l; } diff --git a/sql/server/src/test/java/org/elasticsearch/xpack/sql/optimizer/BooleanSimplificationTests.java b/sql/server/src/test/java/org/elasticsearch/xpack/sql/optimizer/BooleanSimplificationTests.java new file mode 100644 index 00000000000..561d677c5e3 --- /dev/null +++ b/sql/server/src/test/java/org/elasticsearch/xpack/sql/optimizer/BooleanSimplificationTests.java @@ -0,0 +1,95 @@ +/* + * 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.expression.Expression; +import org.elasticsearch.xpack.sql.expression.Literal; +import org.elasticsearch.xpack.sql.expression.predicate.And; +import org.elasticsearch.xpack.sql.expression.predicate.Or; +import org.elasticsearch.xpack.sql.optimizer.Optimizer.BooleanSimplification; +import org.elasticsearch.xpack.sql.tree.Location; +import org.elasticsearch.xpack.sql.type.DataType; +import org.elasticsearch.xpack.sql.type.DataTypes; + +import java.util.Collections; + +public class BooleanSimplificationTests extends ESTestCase { + + private static final Expression DUMMY_EXPRESSION = new DummyBooleanExpression(Location.EMPTY, 0); + + private static class DummyBooleanExpression extends Expression { + + private final int id; + + public DummyBooleanExpression(Location location, int id) { + super(location, Collections.emptyList()); + this.id = id; + } + + @Override + public boolean nullable() { + return false; + } + + @Override + public DataType dataType() { + return DataTypes.BOOLEAN; + } + + @Override + public int hashCode() { + int h = getClass().hashCode(); + h = 31 * h + id; + return h; + } + + @Override + public boolean equals(Object obj) { + if (obj == null || getClass() != obj.getClass()) { + return false; + } + return id == ((DummyBooleanExpression) obj).id; + } + } + + public void testSimplifyOr() { + BooleanSimplification simplification = new BooleanSimplification(); + + assertEquals(Literal.TRUE, simplification.rule(new Or(Location.EMPTY, Literal.TRUE, Literal.TRUE))); + assertEquals(Literal.TRUE, simplification.rule(new Or(Location.EMPTY, Literal.TRUE, DUMMY_EXPRESSION))); + assertEquals(Literal.TRUE, simplification.rule(new Or(Location.EMPTY, DUMMY_EXPRESSION, Literal.TRUE))); + + assertEquals(Literal.FALSE, simplification.rule(new Or(Location.EMPTY, Literal.FALSE, Literal.FALSE))); + assertEquals(DUMMY_EXPRESSION, simplification.rule(new Or(Location.EMPTY, Literal.FALSE, DUMMY_EXPRESSION))); + assertEquals(DUMMY_EXPRESSION, simplification.rule(new Or(Location.EMPTY, DUMMY_EXPRESSION, Literal.FALSE))); + } + + public void testSimplifyAnd() { + BooleanSimplification simplification = new BooleanSimplification(); + + assertEquals(Literal.TRUE, simplification.rule(new And(Location.EMPTY, Literal.TRUE, Literal.TRUE))); + assertEquals(DUMMY_EXPRESSION, simplification.rule(new And(Location.EMPTY, Literal.TRUE, DUMMY_EXPRESSION))); + assertEquals(DUMMY_EXPRESSION, simplification.rule(new And(Location.EMPTY, DUMMY_EXPRESSION, Literal.TRUE))); + + assertEquals(Literal.FALSE, simplification.rule(new And(Location.EMPTY, Literal.FALSE, Literal.FALSE))); + assertEquals(Literal.FALSE, simplification.rule(new And(Location.EMPTY, Literal.FALSE, DUMMY_EXPRESSION))); + assertEquals(Literal.FALSE, simplification.rule(new And(Location.EMPTY, DUMMY_EXPRESSION, Literal.FALSE))); + } + + public void testCommonFactorExtraction() { + BooleanSimplification simplification = new BooleanSimplification(); + + Expression a = new DummyBooleanExpression(Location.EMPTY, 1); + Expression b = new DummyBooleanExpression(Location.EMPTY, 2); + Expression c = new DummyBooleanExpression(Location.EMPTY, 3); + + Expression actual = new Or(Location.EMPTY, new And(Location.EMPTY, a, b), new And(Location.EMPTY, a, c)); + Expression expected = new And(Location.EMPTY, a, new Or(Location.EMPTY, b, c)); + + assertEquals(expected, simplification.rule(actual)); + } +}