Skip to content

Commit

Permalink
SQL: Fix literal projection with condition (#74083) (#74308)
Browse files Browse the repository at this point in the history 
Fixes #64567

Queries with a literal selection and a filter like `SELECT 1 FROM test_emp WHERE gender = 'F'` are currently erroneously optimised to use a local relation. This causes ES to always return a single record, no matter how many records match the filter condition.

This PR makes sure that `SkipQueryIfFoldingProjection` only skips the query if it's an aggregate with only constants (e.g. `SELECT 'foo' FROM test GROUP BY 1`). This optimization seems to lead to another issue #74064 that's not yet addressed in this PR.

Besides this the "skip query" optimization, the `SkipQueryIfFoldingProjection` class also folds constants from `LocalRelation`s and pushes the evaluated values into a new `LocalRelation` (e.g. for queries like `SELECT 1 + 2`).

(cherry-picked from commit 189adf5)
  • Loading branch information
@Luegg
Luegg committed Jun 18, 2021
1 parent 0d48dae commit 6ab6bee
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 37 deletions.
12 changes: 12 additions & 0 deletions x-pack/plugin/sql/qa/server/src/main/resources/select.sql-spec
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,16 @@ sameConstantsWithLimitV2
SELECT 5, 3, 3 FROM "test_emp" LIMIT 5;
sameConstantsWithLimitV3
SELECT 3, 5, 3, 3 FROM "test_emp" LIMIT 5;
constantWithWhere
SELECT 1 FROM "test_emp" WHERE gender = 'F';
constantsWithWhere
SELECT 1, 1, SUBSTRING('test', 1, 1) AS c FROM "test_emp" WHERE gender = 'F';
constantWithWhereConjunction
SELECT 1 FROM "test_emp" WHERE gender = 'F' AND SUBSTRING(first_name, 1, 1) = 'A';
constantWithWhereOrderAndLimit
SELECT 1 FROM "test_emp" WHERE gender = 'F' ORDER BY emp_no LIMIT 5;
constantsWithWhereOrderAndLimit
SELECT 1, SUBSTRING('test', 1, 1) AS c FROM "test_emp" WHERE gender = 'F' AND SUBSTRING(first_name, 1, 1) = 'A' ORDER BY emp_no;
constantAndColumnWithLimit
SELECT 3, first_name, last_name FROM "test_emp" ORDER BY emp_no LIMIT 5;
constantComparisonWithLimit
Expand Down Expand Up @@ -157,3 +167,5 @@ selectGroupByOrderByOrderByLimitNulls
SELECT * FROM (SELECT max(salary) AS max, languages FROM test_emp GROUP BY languages ORDER BY max ASC NULLS LAST) ORDER BY max DESC NULLS FIRST LIMIT 4;
selectGroupByWithAliasedSubQuery
SELECT max, languages FROM (SELECT max(salary) AS max, languages FROM test_emp GROUP BY languages ORDER BY max ASC NULLS LAST) AS subquery;
selectConstantFromSubQuery
SELECT * FROM (SELECT * FROM (SELECT 1));
57 changes: 30 additions & 27 deletions x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/optimizer/Optimizer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1148,29 +1148,40 @@ private static LogicalPlan skipPlan(UnaryPlan plan) {
static class SkipQueryIfFoldingProjection extends OptimizerRule<LogicalPlan> {
@Override
protected LogicalPlan rule(LogicalPlan plan) {
Holder<LocalRelation> optimizedPlan = new Holder<>();
plan.forEachDown(Project.class, p -> {
List<Object> values = extractConstants(p.projections());
if (values.size() == p.projections().size() && (p.child() instanceof EsRelation) == false &&
isNotQueryWithFromClauseAndFilterFoldedToFalse(p)) {
optimizedPlan.set(new LocalRelation(p.source(), new SingletonExecutable(p.output(), values.toArray())));
}
});
List<LogicalPlan> leaves = plan.collectLeaves();

if (optimizedPlan.get() != null) {
return optimizedPlan.get();
}
List<LogicalPlan> projectOrAggregates = plan.collect(p ->
p instanceof Project || p instanceof Aggregate);

if (leaves.size() == 1 && projectOrAggregates.size() == 1) {
LogicalPlan leaf = leaves.get(0);
LogicalPlan projectOrAggregate = projectOrAggregates.get(0);

List<Object> foldedValues = null;

// exclude LocalRelations that have been introduced by earlier optimizations (skipped ESRelations)
boolean isNonSkippedLocalRelation = leaf instanceof LocalRelation
&& ((LocalRelation) leaf).executable() instanceof EmptyExecutable == false;

plan.forEachDown(Aggregate.class, a -> {
List<Object> values = extractConstants(a.aggregates());
if (values.size() == a.aggregates().size() && a.groupings().isEmpty()
&& isNotQueryWithFromClauseAndFilterFoldedToFalse(a)) {
optimizedPlan.set(new LocalRelation(a.source(), new SingletonExecutable(a.output(), values.toArray())));
if (projectOrAggregate instanceof Project && isNonSkippedLocalRelation) {
foldedValues = extractConstants(((Project) projectOrAggregate).projections());
} else if (projectOrAggregate instanceof Aggregate) {
Aggregate a = (Aggregate) projectOrAggregate;
List<Object> folded = extractConstants(a.aggregates());

boolean onlyConstantAggregations = leaf instanceof EsRelation
&& folded.size() == a.aggregates().size()
&& a.groupings().isEmpty();

if (isNonSkippedLocalRelation || onlyConstantAggregations) {
foldedValues = folded;
}
}
});

if (optimizedPlan.get() != null) {
return optimizedPlan.get();
if (foldedValues != null) {
return new LocalRelation(projectOrAggregate.source(),
new SingletonExecutable(projectOrAggregate.output(), foldedValues.toArray()));
}
}

return plan;
Expand Down Expand Up @@ -1198,14 +1209,6 @@ private List<Object> extractConstants(List<? extends NamedExpression> named) {
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) == false || (plan.child() instanceof LocalRelation &&
(((LocalRelation) plan.child()).executable() instanceof EmptyExecutable) == false));
}
}

abstract static class OptimizerBasicRule extends Rule<LogicalPlan, LogicalPlan> {
Expand Down
13 changes: 3 additions & 10 deletions x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/LocalRelation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import org.elasticsearch.action.ActionListener;
import org.elasticsearch.xpack.ql.expression.Attribute;
import org.elasticsearch.xpack.ql.plan.logical.LogicalPlan;
import org.elasticsearch.xpack.ql.plan.logical.LeafPlan;
import org.elasticsearch.xpack.ql.tree.NodeInfo;
import org.elasticsearch.xpack.ql.tree.NodeUtils;
import org.elasticsearch.xpack.ql.tree.Source;
Expand All @@ -19,14 +19,12 @@
import java.util.List;
import java.util.Objects;

import static java.util.Collections.emptyList;

public class LocalRelation extends LogicalPlan implements Executable {
public class LocalRelation extends LeafPlan implements Executable {

private final Executable executable;

public LocalRelation(Source source, Executable executable) {
super(source, emptyList());
super(source);
this.executable = executable;
}

Expand All @@ -35,11 +33,6 @@ protected NodeInfo<LocalRelation> info() {
return NodeInfo.create(this, LocalRelation::new, executable);
}

@Override
public LogicalPlan replaceChildren(List<LogicalPlan> newChildren) {
throw new UnsupportedOperationException("this type of node doesn't have any children to replace");
}

public Executable executable() {
return executable;
}
Expand Down
49 changes: 49 additions & 0 deletions x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import org.elasticsearch.xpack.ql.plan.logical.Aggregate;
import org.elasticsearch.xpack.ql.plan.logical.EsRelation;
import org.elasticsearch.xpack.ql.plan.logical.Filter;
import org.elasticsearch.xpack.ql.plan.logical.LeafPlan;
import org.elasticsearch.xpack.ql.plan.logical.LogicalPlan;
import org.elasticsearch.xpack.ql.plan.logical.OrderBy;
import org.elasticsearch.xpack.ql.plan.logical.Project;
Expand Down Expand Up @@ -117,6 +118,7 @@
import org.elasticsearch.xpack.sql.plan.logical.SubQueryAlias;
import org.elasticsearch.xpack.sql.plan.logical.command.ShowTables;
import org.elasticsearch.xpack.sql.session.EmptyExecutable;
import org.elasticsearch.xpack.sql.session.SingletonExecutable;

import java.lang.reflect.Constructor;
import java.util.Collections;
Expand Down Expand Up @@ -1135,4 +1137,51 @@ public void testSumIsNotReplacedWithStats() {
assertEquals(1, p.aggregates().size());
assertEquals(sumAlias, p.aggregates().get(0));
}

//
// SkipQueryIfFoldingProjection
//

public void testSkipQueryOnLocalRelation() {
// SELECT TRUE as a
Project plan = new Project(EMPTY,
new LocalRelation(EMPTY, new SingletonExecutable(emptyList())),
singletonList(new Alias(EMPTY, "a", TRUE)));

LogicalPlan optimized = new Optimizer.SkipQueryIfFoldingProjection().apply(plan);

assertEquals(LocalRelation.class, optimized.getClass());
assertEquals(plan.output(), ((LocalRelation) optimized).executable().output());
}

public void testSkipQueryOnAggregationOnEsRelationWithOnlyConstants() {
Aggregate plan = new Aggregate(EMPTY,
new EsRelation(EMPTY, new EsIndex("table", emptyMap()), false),
emptyList(),
singletonList(new Alias(EMPTY, "a", TRUE))
);

LogicalPlan optimized = new Optimizer.SkipQueryIfFoldingProjection().apply(plan);

optimized.forEachDown(LeafPlan.class, l -> {
assertEquals(LocalRelation.class, l.getClass());
assertEquals(SingletonExecutable.class, ((LocalRelation) l).executable().getClass());
});
}

public void testDoNotSkipQueryOnEsRelationWithFilter() {
// SELECT TRUE as a FROM table WHERE col IS NULL
Project plan = new Project(EMPTY,
new Filter(EMPTY,
new EsRelation(EMPTY, new EsIndex("table", emptyMap()), false),
new IsNull(EMPTY, getFieldAttribute("col"))),
singletonList(new Alias(EMPTY, "a", TRUE)));

LogicalPlan optimized = new Optimizer.SkipQueryIfFoldingProjection().apply(plan);

optimized.forEachDown(LeafPlan.class, l -> {
assertEquals(EsRelation.class, l.getClass());
});
}

}

0 comments on commit 6ab6bee

Please sign in to comment.