From 6221d6be499ee7d2eb23810733897038ad265623 Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Thu, 20 Dec 2018 09:56:07 +0200 Subject: [PATCH] SQL: Fix issue with always false filter involving functions (#36830) When a filter is evaluated to false then it becomes a LocalRelation with an EmptyExecutable. The LocalRelation in turn, becomes a LocalExec and the the SkipQueryIfFoldingProjection was wrongly converting it to a SingletonExecutable. Moreover made a change, so that the queries without FROM clause, which are supposed to return a single row, to become a LocalRelation with a SingletonExecutable instead of EmptyExecutable to avoid mixing up with the ones operating on a table but with a filter that evaluates to false. Fixes: #35980 --- .../xpack/sql/optimizer/Optimizer.java | 15 ++++++- .../xpack/sql/parser/LogicalPlanBuilder.java | 4 +- .../sql/session/SingletonExecutable.java | 5 +++ .../xpack/sql/planner/QueryFolderTests.java | 43 +++++++++++++++++++ 4 files changed, 63 insertions(+), 4 deletions(-) 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 ffd12ea6fb9..3dc894bda3f 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 @@ -68,6 +68,7 @@ import org.elasticsearch.xpack.sql.plan.logical.LogicalPlan; import org.elasticsearch.xpack.sql.plan.logical.OrderBy; import org.elasticsearch.xpack.sql.plan.logical.Project; import org.elasticsearch.xpack.sql.plan.logical.SubQueryAlias; +import org.elasticsearch.xpack.sql.plan.logical.UnaryPlan; import org.elasticsearch.xpack.sql.rule.Rule; import org.elasticsearch.xpack.sql.rule.RuleExecutor; import org.elasticsearch.xpack.sql.session.EmptyExecutable; @@ -1849,14 +1850,15 @@ public class Optimizer extends RuleExecutor { if (plan instanceof Project) { Project p = (Project) plan; List values = extractConstants(p.projections()); - if (values.size() == p.projections().size() && !(p.child() instanceof EsRelation)) { + if (values.size() == p.projections().size() && !(p.child() instanceof EsRelation) && + isNotQueryWithFromClauseAndFilterFoldedToFalse(p)) { return new LocalRelation(p.location(), new SingletonExecutable(p.output(), values.toArray())); } } if (plan instanceof Aggregate) { Aggregate a = (Aggregate) plan; List values = extractConstants(a.aggregates()); - if (values.size() == a.aggregates().size()) { + if (values.size() == a.aggregates().size() && isNotQueryWithFromClauseAndFilterFoldedToFalse(a)) { return new LocalRelation(a.location(), new SingletonExecutable(a.output(), values.toArray())); } } @@ -1875,6 +1877,15 @@ public class Optimizer extends RuleExecutor { } return values; } + + /** + * Check if the plan doesn't model a query with FROM clause on a table + * that its filter (WHERE clause) is folded to FALSE. + */ + private static boolean isNotQueryWithFromClauseAndFilterFoldedToFalse(UnaryPlan plan) { + return (!(plan.child() instanceof LocalRelation) || (plan.child() instanceof LocalRelation && + !(((LocalRelation) plan.child()).executable() instanceof EmptyExecutable))); + } } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/LogicalPlanBuilder.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/LogicalPlanBuilder.java index 23d2c20d305..14654f7e50d 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/LogicalPlanBuilder.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/LogicalPlanBuilder.java @@ -41,7 +41,7 @@ import org.elasticsearch.xpack.sql.plan.logical.SubQueryAlias; import org.elasticsearch.xpack.sql.plan.logical.UnresolvedRelation; import org.elasticsearch.xpack.sql.plan.logical.With; import org.elasticsearch.xpack.sql.proto.SqlTypedParamValue; -import org.elasticsearch.xpack.sql.session.EmptyExecutable; +import org.elasticsearch.xpack.sql.session.SingletonExecutable; import org.elasticsearch.xpack.sql.type.DataType; import java.util.LinkedHashMap; @@ -104,7 +104,7 @@ abstract class LogicalPlanBuilder extends ExpressionBuilder { public LogicalPlan visitQuerySpecification(QuerySpecificationContext ctx) { LogicalPlan query; if (ctx.fromClause() == null) { - query = new LocalRelation(source(ctx), new EmptyExecutable(emptyList())); + query = new LocalRelation(source(ctx), new SingletonExecutable()); } else { query = plan(ctx.fromClause()); } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/session/SingletonExecutable.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/session/SingletonExecutable.java index 129c30a0df3..8a526fac6df 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/session/SingletonExecutable.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/session/SingletonExecutable.java @@ -9,6 +9,7 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.xpack.sql.expression.Attribute; import org.elasticsearch.xpack.sql.util.Check; +import java.util.Collections; import java.util.List; public class SingletonExecutable implements Executable { @@ -16,6 +17,10 @@ public class SingletonExecutable implements Executable { private final List output; private final Object[] values; + public SingletonExecutable() { + this(Collections.emptyList()); + } + public SingletonExecutable(List output, Object... values) { Check.isTrue(output.size() == values.length, "Attributes {} and values {} are out of sync", output, values); this.output = output; diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryFolderTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryFolderTests.java index bb85921369a..c20f4e9d632 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryFolderTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryFolderTests.java @@ -18,6 +18,7 @@ import org.elasticsearch.xpack.sql.plan.physical.EsQueryExec; import org.elasticsearch.xpack.sql.plan.physical.LocalExec; import org.elasticsearch.xpack.sql.plan.physical.PhysicalPlan; import org.elasticsearch.xpack.sql.session.EmptyExecutable; +import org.elasticsearch.xpack.sql.session.SingletonExecutable; import org.elasticsearch.xpack.sql.stats.Metrics; import org.elasticsearch.xpack.sql.type.EsField; import org.elasticsearch.xpack.sql.type.TypesTests; @@ -68,6 +69,48 @@ public class QueryFolderTests extends ESTestCase { assertThat(ee.output().get(0).toString(), startsWith("keyword{f}#")); } + public void testLocalExecWithPrunedFilterWithFunction() { + PhysicalPlan p = plan("SELECT E() FROM test WHERE PI() = 5"); + assertEquals(LocalExec.class, p.getClass()); + LocalExec le = (LocalExec) p; + assertEquals(EmptyExecutable.class, le.executable().getClass()); + EmptyExecutable ee = (EmptyExecutable) le.executable(); + assertEquals(1, ee.output().size()); + assertThat(ee.output().get(0).toString(), startsWith("E{c}#")); + } + + public void testLocalExecWithPrunedFilterWithFunctionAndAggregation() { + PhysicalPlan p = plan("SELECT E() FROM test WHERE PI() = 5 GROUP BY 1"); + assertEquals(LocalExec.class, p.getClass()); + LocalExec le = (LocalExec) p; + assertEquals(EmptyExecutable.class, le.executable().getClass()); + EmptyExecutable ee = (EmptyExecutable) le.executable(); + assertEquals(1, ee.output().size()); + assertThat(ee.output().get(0).toString(), startsWith("E{c}#")); + } + + public void testLocalExecWithoutFromClause() { + PhysicalPlan p = plan("SELECT E(), 'foo', abs(10)"); + assertEquals(LocalExec.class, p.getClass()); + LocalExec le = (LocalExec) p; + assertEquals(SingletonExecutable.class, le.executable().getClass()); + SingletonExecutable ee = (SingletonExecutable) le.executable(); + assertEquals(3, ee.output().size()); + assertThat(ee.output().get(0).toString(), startsWith("E{c}#")); + assertThat(ee.output().get(1).toString(), startsWith("foo{c}#")); + assertThat(ee.output().get(2).toString(), startsWith("ABS(10){c}#")); + } + + public void testLocalExecWithoutFromClauseWithPrunedFilter() { + PhysicalPlan p = plan("SELECT E() WHERE PI() = 5"); + assertEquals(LocalExec.class, p.getClass()); + LocalExec le = (LocalExec) p; + assertEquals(EmptyExecutable.class, le.executable().getClass()); + EmptyExecutable ee = (EmptyExecutable) le.executable(); + assertEquals(1, ee.output().size()); + assertThat(ee.output().get(0).toString(), startsWith("E{c}#")); + } + public void testFoldingOfIsNull() { PhysicalPlan p = plan("SELECT keyword FROM test WHERE (keyword IS NOT NULL) IS NULL"); assertEquals(LocalExec.class, p.getClass());