Skip to content

Commit

Permalink
SQL: Disallow non-collapsable subselects with ORDER BY (#72991)
Browse files Browse the repository at this point in the history
Ordering an already pre-ordered and limited subselect is not allowed,
as such queries cannot be collapsed and translated into query DSL, but
the require an extra ordering step on top of the results returned
internally by the search/agg query.

Fixes: #71158
  • Loading branch information
matriv committed May 14, 2021
1 parent 891642b commit a5a20ae
Show file tree
Hide file tree
Showing 3 changed files with 154 additions and 0 deletions.
29 changes: 29 additions & 0 deletions x-pack/plugin/sql/qa/server/src/main/resources/select.sql-spec
Original file line number Diff line number Diff line change
Expand Up @@ -126,3 +126,32 @@ selectMathPIFromIndexWithWhereEvaluatingToFalse
SELECT PI() AS pi FROM test_emp WHERE PI()=5;
selectMathPIFromIndexWithWhereEvaluatingToFalseAndWithLimit
SELECT PI() AS pi FROM test_emp WHERE PI()=5 LIMIT 3;


//
// SELECT with collapsable subqueries
//
selectOrderByLimit1
SELECT * FROM (SELECT * FROM test_emp ORDER BY emp_no DESC) LIMIT 5;
selectOrderByLimit2
SELECT * FROM (SELECT * FROM (SELECT * FROM test_emp ORDER BY emp_no DESC)) LIMIT 10;
selectOrderByOrderByLimit1
SELECT * FROM (SELECT * FROM test_emp ORDER BY emp_no DESC) ORDER BY emp_no ASC LIMIT 5;
selectOrderByOrderByLimit2
SELECT * FROM (SELECT * FROM (SELECT * FROM test_emp ORDER BY emp_no DESC) ORDER BY emp_no ASC) LIMIT 5;
selectOrderByOrderByOrderByLimit
SELECT * FROM (SELECT * FROM (SELECT * FROM test_emp ORDER BY emp_no DESC) ORDER BY emp_no ASC) ORDER BY emp_no DESC LIMIT 5;
selectOrderByOrderByOrderByLimitLimit
SELECT * FROM (SELECT * FROM (SELECT * FROM (SELECT * FROM test_emp ORDER BY emp_no DESC) ORDER BY emp_no ASC) ORDER BY emp_no DESC LIMIT 12) LIMIT 6;
selectOrderByLimitSameOrderBy
SELECT * FROM (SELECT * FROM (SELECT * FROM test_emp LIMIT 10) ORDER BY emp_no) ORDER BY emp_no LIMIT 5;
selectGroupByOrderByLimit
SELECT * FROM (SELECT max(salary) AS max, languages FROM test_emp GROUP BY languages) ORDER BY max DESC LIMIT 3;
selectGroupByOrderByLimitNulls
SELECT * FROM (SELECT max(salary) AS max, languages FROM test_emp GROUP BY languages) ORDER BY max DESC NULLS FIRST LIMIT 3;
selectGroupByLimitOrderByLimit
SELECT * FROM (SELECT max(salary) AS max, languages FROM test_emp WHERE languages IS NOT NULL GROUP BY languages LIMIT 5) ORDER BY max DESC LIMIT 3;
selectGroupByOrderByOrderByLimit
SELECT * FROM (SELECT max(salary) AS max, languages FROM test_emp GROUP BY languages ORDER BY max ASC) ORDER BY max DESC NULLS FIRST LIMIT 4;
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;
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
package org.elasticsearch.xpack.sql.planner;

import org.elasticsearch.xpack.ql.common.Failure;
import org.elasticsearch.xpack.ql.expression.Order;
import org.elasticsearch.xpack.ql.util.Holder;
import org.elasticsearch.xpack.sql.plan.physical.LimitExec;
import org.elasticsearch.xpack.sql.plan.physical.OrderExec;
import org.elasticsearch.xpack.sql.plan.physical.PhysicalPlan;
import org.elasticsearch.xpack.sql.plan.physical.Unexecutable;
import org.elasticsearch.xpack.sql.plan.physical.UnplannedExec;
Expand All @@ -32,6 +36,8 @@ static List<Failure> verifyMappingPlan(PhysicalPlan plan) {
});
});

checkForNonCollapsableSubselects(plan, failures);

return failures;
}

Expand All @@ -51,4 +57,22 @@ static List<Failure> verifyExecutingPlan(PhysicalPlan plan) {

return failures;
}

private static void checkForNonCollapsableSubselects(PhysicalPlan plan, List<Failure> failures) {
Holder<Boolean> hasLimit = new Holder<>(Boolean.FALSE);
Holder<List<Order>> orderBy = new Holder<>();
plan.forEachUp(p -> {
if (hasLimit.get() == false && p instanceof LimitExec) {
hasLimit.set(Boolean.TRUE);
return;
}
if (p instanceof OrderExec) {
if (hasLimit.get() && orderBy.get() != null && ((OrderExec) p).order().equals(orderBy.get()) == false) {
failures.add(fail(p, "Cannot use ORDER BY on top of a subquery with ORDER BY and LIMIT"));
} else {
orderBy.set(((OrderExec) p).order());
}
}
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

package org.elasticsearch.xpack.sql.planner;

import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.ql.index.EsIndex;
import org.elasticsearch.xpack.ql.index.IndexResolution;
import org.elasticsearch.xpack.sql.analysis.analyzer.Analyzer;
import org.elasticsearch.xpack.sql.analysis.analyzer.Verifier;
import org.elasticsearch.xpack.sql.expression.function.SqlFunctionRegistry;
import org.elasticsearch.xpack.sql.parser.SqlParser;
import org.elasticsearch.xpack.sql.stats.Metrics;

import static org.elasticsearch.xpack.sql.SqlTestUtils.TEST_CFG;
import static org.elasticsearch.xpack.sql.types.SqlTypesTests.loadMapping;

public class VerifierTests extends ESTestCase {

private final SqlParser parser = new SqlParser();
private final IndexResolution indexResolution = IndexResolution.valid(
new EsIndex("test", loadMapping("mapping-multi-field-with-nested.json"))
);
private final Analyzer analyzer = new Analyzer(
TEST_CFG,
new SqlFunctionRegistry(),
indexResolution,
new Verifier(new Metrics())
);
private final Planner planner = new Planner();

private String error(String sql) {
PlanningException e = expectThrows(
PlanningException.class,
() -> planner.plan(analyzer.analyze(parser.createStatement(sql), true), true)
);
String message = e.getMessage();
assertTrue(message.startsWith("Found "));
String pattern = "\nline ";
int index = message.indexOf(pattern);
return message.substring(index + pattern.length());
}

public void testSubselectWithOrderByOnTopOfOrderByAndLimit() {
assertEquals(
"1:60: Cannot use ORDER BY on top of a subquery with ORDER BY and LIMIT",
error("SELECT * FROM (SELECT * FROM test ORDER BY 1 ASC LIMIT 10) ORDER BY 2")
);
assertEquals(
"1:72: Cannot use ORDER BY on top of a subquery with ORDER BY and LIMIT",
error("SELECT * FROM (SELECT * FROM (SELECT * FROM test LIMIT 10) ORDER BY 1) ORDER BY 2")
);
assertEquals(
"1:75: Cannot use ORDER BY on top of a subquery with ORDER BY and LIMIT",
error("SELECT * FROM (SELECT * FROM (SELECT * FROM test ORDER BY 1 ASC) LIMIT 5) ORDER BY 1 DESC")
);
assertEquals(
"1:152: Cannot use ORDER BY on top of a subquery with ORDER BY and LIMIT",
error("SELECT * FROM (" +
"SELECT * FROM (" +
"SELECT * FROM (" +
"SELECT * FROM test ORDER BY int DESC" +
") ORDER BY int ASC NULLS LAST) " +
"ORDER BY int DESC NULLS LAST LIMIT 12) " +
"ORDER BY int DESC NULLS FIRST")
);
}

public void testSubselectWithOrderByOnTopOfGroupByOrderByAndLimit() {
assertEquals(
"1:96: Cannot use ORDER BY on top of a subquery with ORDER BY and LIMIT",
error(
"SELECT * FROM (SELECT max(int) AS max, bool FROM test GROUP BY bool ORDER BY max ASC LIMIT 10) ORDER BY max DESC"
)
);
assertEquals(
"1:112: Cannot use ORDER BY on top of a subquery with ORDER BY and LIMIT",
error(
"SELECT * FROM ("
+ "SELECT * FROM ("
+ "SELECT max(int) AS max, bool FROM test GROUP BY bool ORDER BY max ASC) "
+ "LIMIT 10) "
+ "ORDER BY max DESC"
)
);
assertEquals(
"1:186: Cannot use ORDER BY on top of a subquery with ORDER BY and LIMIT",
error("SELECT * FROM (" +
"SELECT * FROM (" +
"SELECT * FROM (" +
"SELECT max(int) AS max, bool FROM test GROUP BY bool ORDER BY max DESC" +
") ORDER BY max ASC NULLS LAST) " +
"ORDER BY max DESC NULLS LAST LIMIT 12) " +
"ORDER BY max DESC NULLS FIRST")
);
}
}

0 comments on commit a5a20ae

Please sign in to comment.