Browse Source

SQL: Disallow non-collapsable subselects with ORDER BY (#72991)

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
Marios Trivyzas 4 years ago
parent
commit
a5a20ae510

+ 29 - 0
x-pack/plugin/sql/qa/server/src/main/resources/select.sql-spec

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

+ 24 - 0
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/Verifier.java

@@ -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;
@@ -32,6 +36,8 @@ abstract class Verifier {
             });
         });
 
+        checkForNonCollapsableSubselects(plan, failures);
+
         return failures;
     }
 
@@ -51,4 +57,22 @@ abstract class Verifier {
 
         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());
+                }
+            }
+        });
+    }
 }

+ 101 - 0
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/VerifierTests.java

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