[CALCITE-6063] If ARRAY subquery has ORDER BY (without LIMIT), rows a…

…re not sorted
chucheng92 · Jan 25, 2024 · 8c978fc · 8c978fc
1 parent aca7f02
commit 8c978fc
Show file tree

Hide file tree

Showing 5 changed files with 160 additions and 14 deletions.
diff --git a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
@@ -5538,7 +5538,9 @@ ImmutableList<RelNode> retrieveCursors() {
         case ARRAY_QUERY_CONSTRUCTOR:
           call = (SqlCall) expr;
           query = Iterables.getOnlyElement(call.getOperandList());
-          root = convertQueryRecursive(query, false, null);
+          // let top=true to make the query be top-level query,
+          // then ORDER BY will be reserved.
+          root = convertQueryRecursive(query, true, null);
           return RexSubQuery.array(root.rel);
 
         case MAP_QUERY_CONSTRUCTOR:

diff --git a/core/src/test/resources/sql/sub-query.iq b/core/src/test/resources/sql/sub-query.iq
@@ -3726,4 +3726,52 @@ SELECT map(SELECT empno, deptno from emp where false);
 
 !ok
 
+# [CALCITE-6063] ARRAY subquery with OrderBy loses Sort
+# normal behavior
+SELECT array(SELECT empno FROM emp);
++--------------------------------------------------------------------------------------+
+| EXPR$0                                                                               |
++--------------------------------------------------------------------------------------+
+| [7369, 7499, 7521, 7566, 7654, 7698, 7782, 7788, 7839, 7844, 7876, 7900, 7902, 7934] |
++--------------------------------------------------------------------------------------+
+(1 row)
+
+!ok
+
+# [CALCITE-6063] ARRAY subquery with OrderBy loses Sort
+# with filter
+SELECT array(SELECT empno FROM emp WHERE empno > 7800);
++--------------------------------------+
+| EXPR$0                               |
++--------------------------------------+
+| [7839, 7844, 7876, 7900, 7902, 7934] |
++--------------------------------------+
+(1 row)
+
+!ok
+
+# [CALCITE-6063] ARRAY subquery with OrderBy loses Sort
+# with filter and order by
+SELECT array(SELECT empno FROM emp WHERE empno > 7800 ORDER BY empno DESC);
++--------------------------------------+
+| EXPR$0                               |
++--------------------------------------+
+| [7934, 7902, 7900, 7876, 7844, 7839] |
++--------------------------------------+
+(1 row)
+
+!ok
+
+# [CALCITE-6063] ARRAY subquery with OrderBy loses Sort
+# with filter and order by and limit
+SELECT array(SELECT empno FROM emp WHERE empno > 7800 ORDER BY empno DESC LIMIT 2);
++--------------+
+| EXPR$0       |
++--------------+
+| [7934, 7902] |
++--------------+
+(1 row)
+
+!ok
+
 # End sub-query.iq
diff --git a/site/_docs/reference.md b/site/_docs/reference.md
@@ -1643,6 +1643,14 @@ Implicit type coercion of following cases are ignored:
 | ARRAY '[' value [, value ]* ']' | Creates an array from a list of values.
 | MAP '[' key, value [, key, value ]* ']' | Creates a map from a list of key-value pairs.
 
+### Value constructors by query
+
+| Operator syntax                    | Description |
+|:-----------------------------------|:------------|
+| ARRAY (sub-query)                  | Creates an array from the result of a sub-query. Example: `ARRAY(SELECT empno FROM emp)` |
+| MAP (sub-query)                    | Creates a map from the result of a key-value pair sub-query. Example: `MAP(SELECT empno, deptno FROM emp)` |
+| MULTISET (sub-query)               | Creates a multiset from the result of a sub-query. Example: `MULTISET(SELECT empno FROM emp)` |
+
 ### Collection functions
 
 | Operator syntax | Description

diff --git a/testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java b/testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java
@@ -6033,6 +6033,18 @@ private static Matcher<SqlNode> isCharLiteral(String s) {
             + "FROM `T`)))");
   }
 
+  @Test void testMultisetQueryConstructor() {
+    sql("SELECT multiset(SELECT x FROM (VALUES(1)) x)")
+        .ok("SELECT (MULTISET ((SELECT `X`\n"
+            + "FROM (VALUES (ROW(1))) AS `X`)))");
+    sql("SELECT multiset(SELECT x FROM (VALUES(1)) x ^ORDER^ BY x)")
+        .fails("(?s)Encountered \"ORDER\" at.*");
+    sql("SELECT multiset(SELECT x FROM (VALUES(1)) x, ^SELECT^ x FROM (VALUES(1)) x)")
+        .fails("(?s)Incorrect syntax near the keyword 'SELECT' at.*");
+    sql("SELECT multiset(^1^, SELECT x FROM (VALUES(1)) x)")
+        .fails("(?s)Non-query expression encountered in illegal context");
+  }
+
   @Test void testMultisetUnion() {
     expr("a multiset union b")
         .ok("(`A` MULTISET UNION ALL `B`)");
@@ -6244,6 +6256,14 @@ private static Matcher<SqlNode> isCharLiteral(String s) {
     sql("SELECT map(SELECT T.x, T.y FROM (VALUES(1, 2)) AS T(x, y))")
         .ok("SELECT (MAP ((SELECT `T`.`X`, `T`.`Y`\n"
             + "FROM (VALUES (ROW(1, 2))) AS `T` (`X`, `Y`))))");
+    // with order by
+    // note: map subquery is not sql standard, parser allows order by,
+    // but has no sorting effect in runtime (sort will be removed)
+    sql("SELECT map(SELECT T.x, T.y FROM (VALUES(1, 2) ORDER BY T.x) AS T(x, y))")
+        .ok("SELECT (MAP ((SELECT `T`.`X`, `T`.`Y`\n"
+            + "FROM (VALUES (ROW(1, 2))\n"
+            + "ORDER BY `T`.`X`) AS `T` (`X`, `Y`))))");
+
     sql("SELECT map(1, ^SELECT^ x FROM (VALUES(1)) x)")
         .fails("(?s)Incorrect syntax near the keyword 'SELECT'.*");
     sql("SELECT map(SELECT x FROM (VALUES(1)) x, ^SELECT^ x FROM (VALUES(1)) x)")

diff --git a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
@@ -10697,35 +10697,69 @@ private static void checkArrayConcatAggFuncFails(SqlOperatorFixture t) {
         "[1         , 2         , Hi        , null]", "CHAR(10) ARRAY NOT NULL");
   }
 
-  /**
-   * Test case for
-   * <a href="https://issues.apache.org/jira/browse/CALCITE-4999">[CALCITE-4999]
-   * ARRAY, MULTISET functions should return an collection of scalars
-   * if a sub-query returns 1 column</a>.
-   */
   @Test void testArrayQueryConstructor() {
     final SqlOperatorFixture f = fixture();
     f.setFor(SqlStdOperatorTable.ARRAY_QUERY, SqlOperatorFixture.VmName.EXPAND);
+
+    // Test case for [CALCITE-4999] ARRAY, MULTISET functions should
+    // return a collection of scalars if a sub-query returns 1 column
     f.checkScalar("array(select 1)", "[1]",
         "INTEGER NOT NULL ARRAY NOT NULL");
     f.check("select array(select ROW(1,2))",
         "RecordType(INTEGER NOT NULL EXPR$0, INTEGER NOT NULL EXPR$1) NOT NULL ARRAY NOT NULL",
         "[{1, 2}]");
+
+    // single sub-clause test
+    f.check("select array(select x from unnest(array[1,2,3,4]) as t(x))",
+        "INTEGER NOT NULL ARRAY NOT NULL", "[1, 2, 3, 4]");
+    f.check("select array(select x from unnest(array[1,2,3,4]) as t(x) where x > 1)",
+        "INTEGER NOT NULL ARRAY NOT NULL", "[2, 3, 4]");
+    f.check("select array(select x from unnest(array[1,2,3,4]) as t(x) limit 2)",
+        "INTEGER NOT NULL ARRAY NOT NULL", "[1, 2]");
+    f.check("select array(select x from unnest(array[1,2,3,4]) as t(x) where x > 1 limit 2)",
+        "INTEGER NOT NULL ARRAY NOT NULL", "[2, 3]");
+
+    // combined tests
+    f.check("select array(select x from unnest(array[1,2,3,4]) as t(x) "
+        + "order by x asc)", "INTEGER NOT NULL ARRAY NOT NULL", "[1, 2, 3, 4]");
+    f.check("select array(select x from unnest(array[1,2,3,4]) as t(x) "
+        + "where x > 1 order by x asc)", "INTEGER NOT NULL ARRAY NOT NULL", "[2, 3, 4]");
+    f.check("select array(select x from unnest(array[1,2,3,4]) as t(x) "
+        + "where x > 1 order by x asc limit 2)", "INTEGER NOT NULL ARRAY NOT NULL", "[2, 3]");
+    f.check("select array(select x from unnest(array[1,2,3,4]) as t(x) "
+        + "order by x desc)", "INTEGER NOT NULL ARRAY NOT NULL", "[4, 3, 2, 1]");
+    f.check("select array(select x from unnest(array[1,2,3,4]) as t(x) "
+        + "where x > 1 order by x desc)", "INTEGER NOT NULL ARRAY NOT NULL", "[4, 3, 2]");
+    f.check("select array(select x from unnest(array[1,2,3,4]) as t(x) "
+        + "where x > 1 order by x desc limit 2)", "INTEGER NOT NULL ARRAY NOT NULL", "[4, 3]");
   }
 
-  /**
-   * Test case for
-   * <a href="https://issues.apache.org/jira/browse/CALCITE-4999">[CALCITE-4999]
-   * ARRAY, MULTISET functions should return an collection of scalars
-   * if a sub-query returns 1 column</a>.
-   */
   @Test void testMultisetQueryConstructor() {
     final SqlOperatorFixture f = fixture();
+
+    // Test case for [CALCITE-4999] ARRAY, MULTISET functions should
+    // return an collection of scalars if a sub-query returns 1 column
     f.setFor(SqlStdOperatorTable.MULTISET_QUERY, SqlOperatorFixture.VmName.EXPAND);
     f.checkScalar("multiset(select 1)", "[1]", "INTEGER NOT NULL MULTISET NOT NULL");
     f.check("select multiset(select ROW(1,2))",
         "RecordType(INTEGER NOT NULL EXPR$0, INTEGER NOT NULL EXPR$1) NOT NULL MULTISET NOT NULL",
         "[{1, 2}]");
+
+    // test filter, orderby, limit
+    // multiset subquery not support orderby and limit sub-clause
+    f.check("select multiset(select x from unnest(array[1,2,3,4]) as t(x))",
+        "INTEGER NOT NULL MULTISET NOT NULL", "[1, 2, 3, 4]");
+    f.check("select multiset(select x from unnest(array[1,2,3,4]) as t(x) where x > 1)",
+        "INTEGER NOT NULL MULTISET NOT NULL", "[2, 3, 4]");
+
+    f.checkFails("select multiset(select x from unnest(array[1,2,3,4]) as t(x) ^order^ by x)",
+        "(?s).*Encountered \"order\" at .*", false);
+    f.checkFails("select multiset(select x from unnest(array[1,2,3,4]) as t(x) ^limit^ 2)",
+        "(?s).*Encountered \"limit\" at .*", false);
+    f.checkFails("select multiset(select x from unnest(array[1,2,3,4]) as t(x) "
+        + "^order^ by x limit 2)", "(?s).*Encountered \"order\" at .*", false);
+    f.checkFails("select multiset(select x from unnest(array[1,2,3,4]) as t(x) where x > 1 "
+        + "^order^ by x limit 2)", "(?s).*Encountered \"order\" at .*", false);
   }
 
   @Test void testItemOp() {
@@ -10964,7 +10998,7 @@ private static void checkArrayConcatAggFuncFails(SqlOperatorFixture t) {
   }
 
   @Test void testMapQueryConstructor() {
-    final SqlOperatorFixture f = fixture();
+    final SqlOperatorFixture f = Fixtures.forOperators(true);
     f.setFor(SqlStdOperatorTable.MAP_QUERY, VmName.EXPAND);
     // must be 2 fields
     f.checkFails("map(select 1)", "MAP requires exactly two fields, got 1; "
@@ -10995,6 +11029,40 @@ private static void checkArrayConcatAggFuncFails(SqlOperatorFixture t) {
         "(NULL, INTEGER NOT NULL) MAP NOT NULL");
     f.checkScalar("map(select null, null)", "{null=null}",
         "(NULL, NULL) MAP NOT NULL");
+
+    // single sub-clause test for filter/limit/orderby
+    f.check("select map(select x,y from (values(1,2),(3,4)) as t(x,y))",
+        "(INTEGER NOT NULL, INTEGER NOT NULL) MAP NOT NULL", "{1=2, 3=4}");
+    f.check("select map(select x,y from (values(1,2),(3,4)) as t(x,y) where x > 1)",
+        "(INTEGER NOT NULL, INTEGER NOT NULL) MAP NOT NULL", "{3=4}");
+    f.check("select map(select x,y from (values(1,2),(3,4),(5,6)) as t(x,y) limit 1)",
+        "(INTEGER NOT NULL, INTEGER NOT NULL) MAP NOT NULL", "{1=2}");
+    f.check("select map(select x,y from (values(1,2),(3,4),(5,6)) as t(x,y) where x > 1 limit 1)",
+        "(INTEGER NOT NULL, INTEGER NOT NULL) MAP NOT NULL", "{3=4}");
+
+    // combined tests for filter/limit/orderby
+    // note: map subquery is not sql standard, it has limitations below:
+    // case-1: use order by without limit
+    // it has no sorting effect in runtime (sort will be removed)
+    // case-2: use order by and limit
+    // the order by will take effect (sort will be reserved),
+    // but we do not guarantee the order of the final map result
+    f.check("select map(select x,y from (values(1,2),(3,4)) as t(x,y) order by x asc)",
+        "(INTEGER NOT NULL, INTEGER NOT NULL) MAP NOT NULL", "{1=2, 3=4}");
+    f.check("select map(select x,y from (values(1,2),(3,4)) as t(x,y) "
+            + "where x > 1 order by x asc)",
+        "(INTEGER NOT NULL, INTEGER NOT NULL) MAP NOT NULL", "{3=4}");
+    f.check("select map(select x,y from (values(1,2),(3,4),(5,6)) as t(x,y) "
+        + "where x > 1 order by x asc limit 1)",
+        "(INTEGER NOT NULL, INTEGER NOT NULL) MAP NOT NULL", "{3=4}");
+    f.check("select map(select x,y from (values(1,2),(3,4)) as t(x,y) order by x desc)",
+        "(INTEGER NOT NULL, INTEGER NOT NULL) MAP NOT NULL", "{1=2, 3=4}");
+    f.check("select map(select x,y from (values(1,2),(3,4)) as t(x,y) "
+            + "where x > 1 order by x desc)",
+        "(INTEGER NOT NULL, INTEGER NOT NULL) MAP NOT NULL", "{3=4}");
+    f.check("select map(select x,y from (values(1,2),(3,4),(5,6)) as t(x,y) "
+            + "where x > 1 order by x desc limit 1)",
+        "(INTEGER NOT NULL, INTEGER NOT NULL) MAP NOT NULL", "{5=6}");
   }
 
   @Test void testCeilFunc() {