Skip to content

SQL: Fix issue with optimization on queries with ORDER BY/LIMIT #40256

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 20, 2019
Merged

SQL: Fix issue with optimization on queries with ORDER BY/LIMIT #40256

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ce1b582
SQL: Fix issue with optimization on queries with ORDER BY/LIMIT
matriv Mar 20, 2019
ca916bc
Merge remote-tracking branch 'upstream/master' into mt/fix-40211
matriv Mar 20, 2019
8abdd82
Address comment
matriv Mar 20, 2019
9888f02
Merge remote-tracking branch 'upstream/master' into mt/fix-40211
matriv Mar 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 16 additions & 7 deletions x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/optimizer/Optimizer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1909,7 +1909,7 @@ static class SkipQueryOnLimitZero extends OptimizerRule<Limit> {
@Override
protected LogicalPlan rule(Limit limit) {
if (limit.limit() instanceof Literal) {
if (Integer.valueOf(0).equals((((Literal) limit.limit()).fold()))) {
if (Integer.valueOf(0).equals((limit.limit().fold()))) {
return new LocalRelation(limit.source(), new EmptyExecutable(limit.output()));
}
}
Expand All @@ -1920,21 +1920,30 @@ protected LogicalPlan rule(Limit limit) {
static class SkipQueryIfFoldingProjection extends OptimizerRule<LogicalPlan> {
@Override
protected LogicalPlan rule(LogicalPlan plan) {
if (plan instanceof Project) {
Project p = (Project) plan;
Holder<LocalRelation> optimizedPlan = new Holder<>();
plan.forEachDown(p -> {
List<Object> values = extractConstants(p.projections());
if (values.size() == p.projections().size() && !(p.child() instanceof EsRelation) &&
isNotQueryWithFromClauseAndFilterFoldedToFalse(p)) {
return new LocalRelation(p.source(), new SingletonExecutable(p.output(), values.toArray()));
optimizedPlan.set(new LocalRelation(p.source(), new SingletonExecutable(p.output(), values.toArray())));
}
}, Project.class);

if (optimizedPlan.get() != null) {
return optimizedPlan.get();
}
if (plan instanceof Aggregate) {
Aggregate a = (Aggregate) plan;

plan.forEachDown(a -> {
List<Object> values = extractConstants(a.aggregates());
if (values.size() == a.aggregates().size() && isNotQueryWithFromClauseAndFilterFoldedToFalse(a)) {
return new LocalRelation(a.source(), new SingletonExecutable(a.output(), values.toArray()));
optimizedPlan.set(new LocalRelation(a.source(), new SingletonExecutable(a.output(), values.toArray())));
}
}, Aggregate.class);

if (optimizedPlan.get() != null) {
return optimizedPlan.get();
}

return plan;
}

Expand Down
12 changes: 6 additions & 6 deletions x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1226,7 +1226,7 @@ public void testTranslateMinToFirst() {
List<Order> order = ((OrderBy) result).order();
assertEquals(2, order.size());
assertEquals(First.class, order.get(0).child().getClass());
assertEquals(min2, order.get(1).child());;
assertEquals(min2, order.get(1).child());
First first = (First) order.get(0).child();

assertTrue(((OrderBy) result).child() instanceof Aggregate);
Expand All @@ -1249,7 +1249,7 @@ public void testTranslateMaxToLast() {
assertTrue(result instanceof OrderBy);
List<Order> order = ((OrderBy) result).order();
assertEquals(Last.class, order.get(0).child().getClass());
assertEquals(max2, order.get(1).child());;
assertEquals(max2, order.get(1).child());
Last last = (Last) order.get(0).child();

assertTrue(((OrderBy) result).child() instanceof Aggregate);
Expand Down Expand Up @@ -1288,8 +1288,8 @@ public void testSortAggregateOnOrderByWithTwoFields() {
assertEquals(2, groupings.size());
assertTrue(groupings.get(0) instanceof FieldAttribute);
assertTrue(groupings.get(1) instanceof FieldAttribute);
assertEquals(firstField, ((FieldAttribute) groupings.get(0)));
assertEquals(secondField, ((FieldAttribute) groupings.get(1)));
assertEquals(firstField, groupings.get(0));
assertEquals(secondField, groupings.get(1));
}

public void testSortAggregateOnOrderByOnlyAliases() {
Expand Down Expand Up @@ -1320,7 +1320,7 @@ public void testSortAggregateOnOrderByOnlyAliases() {
assertEquals(2, groupings.size());
assertTrue(groupings.get(0) instanceof Alias);
assertTrue(groupings.get(1) instanceof Alias);
assertEquals(firstAlias, ((Alias) groupings.get(0)));
assertEquals(secondAlias, ((Alias) groupings.get(1)));
assertEquals(firstAlias, groupings.get(0));
assertEquals(secondAlias, groupings.get(1));
}
}
60 changes: 60 additions & 0 deletions x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryFolderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,36 @@ public void testFoldingToLocalExecWithProject() {
assertThat(ee.output().get(0).toString(), startsWith("keyword{f}#"));
}

public void testFoldingToLocalExecWithProjectAndLimit() {
PhysicalPlan p = plan("SELECT keyword FROM test WHERE 1 = 2 LIMIT 10");
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("keyword{f}#"));
}

public void testFoldingToLocalExecWithProjectAndOrderBy() {
PhysicalPlan p = plan("SELECT keyword FROM test WHERE 1 = 2 ORDER 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("keyword{f}#"));
}

public void testFoldingToLocalExecWithProjectAndOrderByAndLimit() {
PhysicalPlan p = plan("SELECT keyword FROM test WHERE 1 = 2 ORDER BY 1 LIMIT 10");
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("keyword{f}#"));
}

public void testLocalExecWithPrunedFilterWithFunction() {
PhysicalPlan p = plan("SELECT E() FROM test WHERE PI() = 5");
assertEquals(LocalExec.class, p.getClass());
Expand All @@ -90,6 +120,36 @@ public void testLocalExecWithPrunedFilterWithFunctionAndAggregation() {
assertThat(ee.output().get(0).toString(), startsWith("E(){c}#"));
}

public void testFoldingToLocalExecWithAggregationAndLimit() {
PhysicalPlan p = plan("SELECT 'foo' FROM test GROUP BY 1 LIMIT 10");
assertEquals(LocalExec.class, p.getClass());
LocalExec le = (LocalExec) p;
assertEquals(SingletonExecutable.class, le.executable().getClass());
SingletonExecutable ee = (SingletonExecutable) le.executable();
assertEquals(1, ee.output().size());
assertThat(ee.output().get(0).toString(), startsWith("'foo'{c}#"));
}

public void testFoldingToLocalExecWithAggregationAndOrderBy() {
PhysicalPlan p = plan("SELECT 'foo' FROM test GROUP BY 1 ORDER BY 1");
assertEquals(LocalExec.class, p.getClass());
LocalExec le = (LocalExec) p;
assertEquals(SingletonExecutable.class, le.executable().getClass());
SingletonExecutable ee = (SingletonExecutable) le.executable();
assertEquals(1, ee.output().size());
assertThat(ee.output().get(0).toString(), startsWith("'foo'{c}#"));
}

public void testFoldingToLocalExecWithAggregationAndOrderByAndLimit() {
PhysicalPlan p = plan("SELECT 'foo' FROM test GROUP BY 1 ORDER BY 1 LIMIT 10");
assertEquals(LocalExec.class, p.getClass());
LocalExec le = (LocalExec) p;
assertEquals(SingletonExecutable.class, le.executable().getClass());
SingletonExecutable ee = (SingletonExecutable) le.executable();
assertEquals(1, ee.output().size());
assertThat(ee.output().get(0).toString(), startsWith("'foo'{c}#"));
}

public void testLocalExecWithoutFromClause() {
PhysicalPlan p = plan("SELECT E(), 'foo', abs(10)");
assertEquals(LocalExec.class, p.getClass());
Expand Down