From de9a0fab78ec49d696862294265972bcf689daa5 Mon Sep 17 00:00:00 2001 From: Ran Tao Date: Wed, 24 Jan 2024 16:39:59 +0800 Subject: [PATCH] [CALCITE-6063] If ARRAY subquery has ORDER BY (without LIMIT), rows are not sorted --- .../calcite/sql2rel/SqlToRelConverter.java | 4 +- core/src/test/resources/sql/sub-query.iq | 48 ++++++++++ site/_docs/reference.md | 8 ++ .../calcite/sql/parser/SqlParserTest.java | 20 ++++ .../apache/calcite/test/SqlOperatorTest.java | 92 ++++++++++++++++--- 5 files changed, 159 insertions(+), 13 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 index e5afc62468f..0998c054035 100644 --- 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 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 index 48508dfab72..4d76b3fb6b9 100644 --- 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 index 1ba228d312c..bf604b6a396 100644 --- 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 ORDER BY empno)` | +| 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 index ac15c0c16e3..a0a23f4804a 100644 --- 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 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 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 index 24bf9945919..5d3df2e8580 100644 --- 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 - * [CALCITE-4999] - * ARRAY, MULTISET functions should return an collection of scalars - * if a sub-query returns 1 column. - */ @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 - * [CALCITE-4999] - * ARRAY, MULTISET functions should return an collection of scalars - * if a sub-query returns 1 column. - */ @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() { @@ -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() {