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
This commit is contained in:
Marios Trivyzas 2018-12-20 09:56:07 +02:00 committed by GitHub
parent a7f344cc7f
commit 6221d6be49
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 63 additions and 4 deletions

View File

@ -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<LogicalPlan> {
if (plan instanceof Project) {
Project p = (Project) plan;
List<Object> 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<Object> 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<LogicalPlan> {
}
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)));
}
}

View File

@ -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());
}

View File

@ -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<Attribute> output;
private final Object[] values;
public SingletonExecutable() {
this(Collections.emptyList());
}
public SingletonExecutable(List<Attribute> output, Object... values) {
Check.isTrue(output.size() == values.length, "Attributes {} and values {} are out of sync", output, values);
this.output = output;

View File

@ -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());