From defea67b80fe432c5a8f776718169a3595b222a8 Mon Sep 17 00:00:00 2001 From: "wumou.wm" Date: Sat, 15 Oct 2022 02:20:57 +0800 Subject: [PATCH] [CALCITE-5299] JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH body Remove SET_QUERY from SqlKind#EXPRESSION. This closes #2938 --- .../java/org/apache/calcite/sql/SqlKind.java | 2 +- .../java/org/apache/calcite/sql/SqlWith.java | 5 +- .../org/apache/calcite/sql/SqlWriter.java | 5 + .../calcite/sql/pretty/SqlPrettyWriter.java | 2 +- .../calcite/sql/parser/SqlParserTest.java | 184 +++++++++++------- 5 files changed, 122 insertions(+), 76 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/sql/SqlKind.java b/core/src/main/java/org/apache/calcite/sql/SqlKind.java index e6ef04d61959..9dff2716d8c0 100644 --- a/core/src/main/java/org/apache/calcite/sql/SqlKind.java +++ b/core/src/main/java/org/apache/calcite/sql/SqlKind.java @@ -1199,7 +1199,7 @@ public enum SqlKind { NULLS_FIRST, NULLS_LAST, COLLECTION_TABLE, TABLESAMPLE, VALUES, WITH, WITH_ITEM, ITEM, SKIP_TO_FIRST, SKIP_TO_LAST, JSON_VALUE_EXPRESSION, UNNEST), - AGGREGATE, DML, DDL)); + SET_QUERY, AGGREGATE, DML, DDL)); /** * Category of all SQL statement types. diff --git a/core/src/main/java/org/apache/calcite/sql/SqlWith.java b/core/src/main/java/org/apache/calcite/sql/SqlWith.java index 06171be8c2cc..091b65d8ff2f 100644 --- a/core/src/main/java/org/apache/calcite/sql/SqlWith.java +++ b/core/src/main/java/org/apache/calcite/sql/SqlWith.java @@ -103,8 +103,9 @@ private SqlWithOperator() { } writer.endList(frame1); final SqlWriter.Frame frame2 = - writer.startList(SqlWriter.FrameTypeEnum.SIMPLE); - with.body.unparse(writer, 100, 100); + writer.startList(SqlWriter.FrameTypeEnum.WITH_BODY); + with.body.unparse(writer, + SqlWithOperator.INSTANCE.getLeftPrec(), SqlWithOperator.INSTANCE.getRightPrec()); writer.endList(frame2); writer.endList(frame); } diff --git a/core/src/main/java/org/apache/calcite/sql/SqlWriter.java b/core/src/main/java/org/apache/calcite/sql/SqlWriter.java index 4eae8a165c27..9f8463230260 100644 --- a/core/src/main/java/org/apache/calcite/sql/SqlWriter.java +++ b/core/src/main/java/org/apache/calcite/sql/SqlWriter.java @@ -149,6 +149,11 @@ enum FrameTypeEnum implements FrameType { */ WITH, + /** + * The body query of WITH. + */ + WITH_BODY, + /** * OFFSET clause. * diff --git a/core/src/main/java/org/apache/calcite/sql/pretty/SqlPrettyWriter.java b/core/src/main/java/org/apache/calcite/sql/pretty/SqlPrettyWriter.java index 13f43e8b50f5..1471565a8f07 100644 --- a/core/src/main/java/org/apache/calcite/sql/pretty/SqlPrettyWriter.java +++ b/core/src/main/java/org/apache/calcite/sql/pretty/SqlPrettyWriter.java @@ -398,7 +398,7 @@ public void setWindowDeclListNewline(boolean windowDeclListNewline) { return (frame == null) || (frame.frameType == FrameTypeEnum.SELECT) || (frame.frameType == FrameTypeEnum.ORDER_BY) - || (frame.frameType == FrameTypeEnum.WITH) + || (frame.frameType == FrameTypeEnum.WITH_BODY) || (frame.frameType == FrameTypeEnum.SETOP); } 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 c14c291734ae..520cb55b06ad 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 @@ -2267,8 +2267,8 @@ void checkPeriodPredicate(Checker checker) { + "select deptno from femaleEmps"; final String expected = "WITH `FEMALEEMPS` AS (SELECT *\n" + "FROM `EMPS`\n" - + "WHERE (`GENDER` = 'F')) (SELECT `DEPTNO`\n" - + "FROM `FEMALEEMPS`)"; + + "WHERE (`GENDER` = 'F')) SELECT `DEPTNO`\n" + + "FROM `FEMALEEMPS`"; sql(sql).ok(expected); } @@ -2280,8 +2280,8 @@ void checkPeriodPredicate(Checker checker) { + "FROM `EMPS`\n" + "WHERE (`GENDER` = 'F')), `MARRIEDFEMALEEMPS` (`X`, `Y`) AS (SELECT *\n" + "FROM `FEMALEEMPS`\n" - + "WHERE (`MARITASTATUS` = 'M')) (SELECT `DEPTNO`\n" - + "FROM `FEMALEEMPS`)"; + + "WHERE (`MARITASTATUS` = 'M')) SELECT `DEPTNO`\n" + + "FROM `FEMALEEMPS`"; sql(sql).ok(expected); } @@ -2296,8 +2296,8 @@ void checkPeriodPredicate(Checker checker) { final String sql = "with v(i,c) as (values (1, 'a'), (2, 'bb'))\n" + "select c, i from v"; final String expected = "WITH `V` (`I`, `C`) AS (VALUES (ROW(1, 'a')),\n" - + "(ROW(2, 'bb'))) (SELECT `C`, `I`\n" - + "FROM `V`)"; + + "(ROW(2, 'bb'))) SELECT `C`, `I`\n" + + "FROM `V`"; sql(sql).ok(expected); } @@ -2309,6 +2309,31 @@ void checkPeriodPredicate(Checker checker) { sql(sql).fails("(?s)Encountered \"with\" at .*"); } + /** Test case for + * [CALCITE-5299] + * JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH body. */ + @Test void testWithSelect() { + final String sql = "with emp2 as (select * from emp)\n" + + "select * from emp2\n"; + final String expected = "WITH `EMP2` AS (SELECT *\n" + + "FROM `EMP`) SELECT *\n" + + "FROM `EMP2`"; + sql(sql).ok(expected); + } + + /** Test case for + * [CALCITE-5299] + * JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH body. */ + @Test void testWithOrderBy() { + final String sql = "with emp2 as (select * from emp)\n" + + "select * from emp2 order by deptno\n"; + final String expected = "WITH `EMP2` AS (SELECT *\n" + + "FROM `EMP`) SELECT *\n" + + "FROM `EMP2`\n" + + "ORDER BY `DEPTNO`"; + sql(sql).ok(expected); + } + @Test void testWithNestedInSubQuery() { // SQL standard does not allow sub-query to contain WITH but we do final String sql = "with emp2 as (select * from emp)\n" @@ -2317,8 +2342,8 @@ void checkPeriodPredicate(Checker checker) { + " select 1 as uno from empDept)"; final String expected = "WITH `EMP2` AS (SELECT *\n" + "FROM `EMP`) (WITH `DEPT2` AS (SELECT *\n" - + "FROM `DEPT`) (SELECT 1 AS `UNO`\n" - + "FROM `EMPDEPT`))"; + + "FROM `DEPT`) SELECT 1 AS `UNO`\n" + + "FROM `EMPDEPT`)"; sql(sql).ok(expected); } @@ -2329,11 +2354,11 @@ void checkPeriodPredicate(Checker checker) { + "union\n" + "select * from emp2\n"; final String expected = "WITH `EMP2` AS (SELECT *\n" - + "FROM `EMP`) (SELECT *\n" + + "FROM `EMP`) SELECT *\n" + "FROM `EMP2`\n" + "UNION\n" + "SELECT *\n" - + "FROM `EMP2`)"; + + "FROM `EMP2`"; sql(sql).ok(expected); } @@ -2475,8 +2500,8 @@ void checkPeriodPredicate(Checker checker) { .withConfig(c -> c.withQuoting(Quoting.BRACKET) .withConformance(SqlConformanceEnum.DEFAULT)); f2.fails(expectingAlias); - final String sql2b = "WITH `T` AS (SELECT 1 AS `x'y`) (SELECT `x'y`\n" - + "FROM `T` AS `u`)"; + final String sql2b = "WITH `T` AS (SELECT 1 AS `x'y`) SELECT `x'y`\n" + + "FROM `T` AS `u`"; f2.withConformance(SqlConformanceEnum.MYSQL_5) .ok(sql2b); f2.withConformance(SqlConformanceEnum.BIG_QUERY) @@ -2486,8 +2511,8 @@ void checkPeriodPredicate(Checker checker) { // also valid on MSSQL final String sql3 = "with [t] as (select 1 as [x]) select [x] from [t]"; - final String sql3b = "WITH `t` AS (SELECT 1 AS `x`) (SELECT `x`\n" - + "FROM `t`)"; + final String sql3b = "WITH `t` AS (SELECT 1 AS `x`) SELECT `x`\n" + + "FROM `t`"; final SqlParserFixture f3 = sql(sql3) .withConfig(c -> c.withQuoting(Quoting.BRACKET) .withConformance(SqlConformanceEnum.DEFAULT)); @@ -2573,11 +2598,11 @@ void checkPeriodPredicate(Checker checker) { + "select * from dept) and false") .ok("SELECT *\n" + "FROM `EMP`\n" - + "WHERE ((`DEPTNO` IN ((SELECT `DEPTNO`\n" + + "WHERE ((`DEPTNO` IN (SELECT `DEPTNO`\n" + "FROM `DEPT`\n" + "UNION\n" + "SELECT *\n" - + "FROM `DEPT`)\n" + + "FROM `DEPT`\n" + "EXCEPT\n" + "SELECT *\n" + "FROM `DEPT`)) AND FALSE)"); @@ -2637,23 +2662,23 @@ void checkPeriodPredicate(Checker checker) { @Test void testUnion() { sql("select * from a union select * from a") - .ok("(SELECT *\n" + .ok("SELECT *\n" + "FROM `A`\n" + "UNION\n" + "SELECT *\n" - + "FROM `A`)"); + + "FROM `A`"); sql("select * from a union all select * from a") - .ok("(SELECT *\n" + .ok("SELECT *\n" + "FROM `A`\n" + "UNION ALL\n" + "SELECT *\n" - + "FROM `A`)"); + + "FROM `A`"); sql("select * from a union distinct select * from a") - .ok("(SELECT *\n" + .ok("SELECT *\n" + "FROM `A`\n" + "UNION\n" + "SELECT *\n" - + "FROM `A`)"); + + "FROM `A`"); } @Test void testUnionOrder() { @@ -2661,11 +2686,11 @@ void checkPeriodPredicate(Checker checker) { + "union all " + "select x, y from u " + "order by 1 asc, 2 desc") - .ok("(SELECT `A`, `B`\n" + .ok("SELECT `A`, `B`\n" + "FROM `T`\n" + "UNION ALL\n" + "SELECT `X`, `Y`\n" - + "FROM `U`)\n" + + "FROM `U`\n" + "ORDER BY 1, 2 DESC"); } @@ -2691,13 +2716,13 @@ void checkPeriodPredicate(Checker checker) { final String sql = "(select a from t limit 10)\n" + "union all\n" + "(select b from t offset 20)"; - final String expected = "((SELECT `A`\n" + final String expected = "(SELECT `A`\n" + "FROM `T`\n" + "FETCH NEXT 10 ROWS ONLY)\n" + "UNION ALL\n" + "(SELECT `B`\n" + "FROM `T`\n" - + "OFFSET 20 ROWS))"; + + "OFFSET 20 ROWS)"; sql(sql).ok(expected); } @@ -2707,79 +2732,94 @@ void checkPeriodPredicate(Checker checker) { final String sql = "select a from t\n" + "union all\n" + "(select b from t order by b offset 3 fetch next 5 rows only)"; - final String expected = "(SELECT `A`\n" + final String expected = "SELECT `A`\n" + "FROM `T`\n" + "UNION ALL\n" + "(SELECT `B`\n" + "FROM `T`\n" + "ORDER BY `B`\n" + "OFFSET 3 ROWS\n" - + "FETCH NEXT 5 ROWS ONLY))"; + + "FETCH NEXT 5 ROWS ONLY)"; sql(sql).ok(expected); // as above, just ORDER BY final String sql2 = "select a from t\n" + "union all\n" + "(select b from t order by b)"; - final String expected2 = "(SELECT `A`\n" + final String expected2 = "SELECT `A`\n" + "FROM `T`\n" + "UNION ALL\n" + "(SELECT `B`\n" + "FROM `T`\n" - + "ORDER BY `B`))"; + + "ORDER BY `B`)"; sql(sql2).ok(expected2); // as above, just OFFSET final String sql3 = "select a from t\n" + "union all\n" + "(select b from t offset 3)"; - final String expected3 = "(SELECT `A`\n" + final String expected3 = "SELECT `A`\n" + "FROM `T`\n" + "UNION ALL\n" + "(SELECT `B`\n" + "FROM `T`\n" - + "OFFSET 3 ROWS))"; + + "OFFSET 3 ROWS)"; sql(sql3).ok(expected3); // as above, just FETCH final String sql4 = "select a from t\n" + "union all\n" + "(select b from t fetch next 5 rows only)"; - final String expected4 = "(SELECT `A`\n" + final String expected4 = "SELECT `A`\n" + "FROM `T`\n" + "UNION ALL\n" + "(SELECT `B`\n" + "FROM `T`\n" - + "FETCH NEXT 5 ROWS ONLY))"; + + "FETCH NEXT 5 ROWS ONLY)"; sql(sql4).ok(expected4); // as above, just FETCH and OFFSET final String sql5 = "select a from t\n" + "union all\n" + "(select b from t offset 3 fetch next 5 rows only)"; - final String expected5 = "(SELECT `A`\n" + final String expected5 = "SELECT `A`\n" + "FROM `T`\n" + "UNION ALL\n" + "(SELECT `B`\n" + "FROM `T`\n" + "OFFSET 3 ROWS\n" - + "FETCH NEXT 5 ROWS ONLY))"; + + "FETCH NEXT 5 ROWS ONLY)"; sql(sql5).ok(expected5); // as previous, INTERSECT final String sql6 = "select a from t\n" + "intersect\n" + "(select b from t offset 3 fetch next 5 rows only)"; - final String expected6 = "(SELECT `A`\n" + final String expected6 = "SELECT `A`\n" + "FROM `T`\n" + "INTERSECT\n" + "(SELECT `B`\n" + "FROM `T`\n" + "OFFSET 3 ROWS\n" - + "FETCH NEXT 5 ROWS ONLY))"; + + "FETCH NEXT 5 ROWS ONLY)"; sql(sql6).ok(expected6); } + @Test void testUnionIntersect() { + // Note that the union sub-query has parentheses. + final String sql = "(select * from a union select * from b)\n" + + "intersect select * from c"; + final String expected = "(SELECT *\n" + + "FROM `A`\n" + + "UNION\n" + + "SELECT *\n" + + "FROM `B`)\n" + + "INTERSECT\n" + + "SELECT *\n" + + "FROM `C`"; + sql(sql).ok(expected); + } + @Test void testUnionOfNonQueryFails() { sql("select 1 from emp union ^2^ + 5") .fails("Non-query expression encountered in illegal context"); @@ -2798,23 +2838,23 @@ void checkPeriodPredicate(Checker checker) { @Test void testExcept() { sql("select * from a except select * from a") - .ok("(SELECT *\n" + .ok("SELECT *\n" + "FROM `A`\n" + "EXCEPT\n" + "SELECT *\n" - + "FROM `A`)"); + + "FROM `A`"); sql("select * from a except all select * from a") - .ok("(SELECT *\n" + .ok("SELECT *\n" + "FROM `A`\n" + "EXCEPT ALL\n" + "SELECT *\n" - + "FROM `A`)"); + + "FROM `A`"); sql("select * from a except distinct select * from a") - .ok("(SELECT *\n" + .ok("SELECT *\n" + "FROM `A`\n" + "EXCEPT\n" + "SELECT *\n" - + "FROM `A`)"); + + "FROM `A`"); } /** Tests MINUS, which is equivalent to EXCEPT but only supported in some @@ -2825,22 +2865,22 @@ void checkPeriodPredicate(Checker checker) { final String sql = "select col1 from table1 ^MINUS^ select col1 from table2"; sql(sql).fails(pattern); - final String expected = "(SELECT `COL1`\n" + final String expected = "SELECT `COL1`\n" + "FROM `TABLE1`\n" + "EXCEPT\n" + "SELECT `COL1`\n" - + "FROM `TABLE2`)"; + + "FROM `TABLE2`"; sql(sql) .withConformance(SqlConformanceEnum.ORACLE_10) .ok(expected); final String sql2 = "select col1 from table1 MINUS ALL select col1 from table2"; - final String expected2 = "(SELECT `COL1`\n" + final String expected2 = "SELECT `COL1`\n" + "FROM `TABLE1`\n" + "EXCEPT ALL\n" + "SELECT `COL1`\n" - + "FROM `TABLE2`)"; + + "FROM `TABLE2`"; sql(sql2) .withConformance(SqlConformanceEnum.ORACLE_10) .ok(expected2); @@ -2861,23 +2901,23 @@ void checkPeriodPredicate(Checker checker) { @Test void testIntersect() { sql("select * from a intersect select * from a") - .ok("(SELECT *\n" + .ok("SELECT *\n" + "FROM `A`\n" + "INTERSECT\n" + "SELECT *\n" - + "FROM `A`)"); + + "FROM `A`"); sql("select * from a intersect all select * from a") - .ok("(SELECT *\n" + .ok("SELECT *\n" + "FROM `A`\n" + "INTERSECT ALL\n" + "SELECT *\n" - + "FROM `A`)"); + + "FROM `A`"); sql("select * from a intersect distinct select * from a") - .ok("(SELECT *\n" + .ok("SELECT *\n" + "FROM `A`\n" + "INTERSECT\n" + "SELECT *\n" - + "FROM `A`)"); + + "FROM `A`"); } @Test void testJoinCross() { @@ -3342,12 +3382,12 @@ void checkPeriodPredicate(Checker checker) { @Test void testOrderInternal() { sql("(select * from emp order by empno) union select * from emp") - .ok("((SELECT *\n" + .ok("(SELECT *\n" + "FROM `EMP`\n" + "ORDER BY `EMPNO`)\n" + "UNION\n" + "SELECT *\n" - + "FROM `EMP`)"); + + "FROM `EMP`"); sql("select * from (select * from t order by x, y) where a = b") .ok("SELECT *\n" @@ -3497,11 +3537,11 @@ void checkPeriodPredicate(Checker checker) { + "union\n" + "select b from baz\n" + "limit 3"; - final String expected5 = "(SELECT A\n" + final String expected5 = "SELECT A\n" + "FROM FOO\n" + "UNION\n" + "SELECT B\n" - + "FROM BAZ)\n" + + "FROM BAZ\n" + "LIMIT 3"; sql(sql5).withDialect(SparkSqlDialect.DEFAULT).ok(expected5); } @@ -3817,26 +3857,26 @@ void checkPeriodPredicate(Checker checker) { + "select * from e except " + "select * from f union " + "select * from g"; - final String expected = "((((SELECT *\n" + final String expected = "SELECT *\n" + "FROM `A`\n" + "UNION\n" - + "((SELECT *\n" + + "SELECT *\n" + "FROM `B`\n" + "INTERSECT\n" + "SELECT *\n" - + "FROM `C`)\n" + + "FROM `C`\n" + "INTERSECT\n" + "SELECT *\n" - + "FROM `D`))\n" + + "FROM `D`\n" + "EXCEPT\n" + "SELECT *\n" - + "FROM `E`)\n" + + "FROM `E`\n" + "EXCEPT\n" + "SELECT *\n" - + "FROM `F`)\n" + + "FROM `F`\n" + "UNION\n" + "SELECT *\n" - + "FROM `G`)"; + + "FROM `G`"; sql(sql).ok(expected); } @@ -4453,11 +4493,11 @@ void checkPeriodPredicate(Checker checker) { sql("describe statement (select * from emps)").ok(expected0); final String expected2 = "" + "EXPLAIN PLAN INCLUDING ATTRIBUTES WITH IMPLEMENTATION FOR\n" - + "(SELECT `DEPTNO`\n" + + "SELECT `DEPTNO`\n" + "FROM `EMPS`\n" + "UNION\n" + "SELECT `DEPTNO`\n" - + "FROM `DEPTS`)"; + + "FROM `DEPTS`"; sql("describe select deptno from emps union select deptno from depts").ok(expected2); final String expected3 = "" + "EXPLAIN PLAN INCLUDING ATTRIBUTES WITH IMPLEMENTATION FOR\n" @@ -4487,11 +4527,11 @@ void checkPeriodPredicate(Checker checker) { @Test void testInsertUnion() { final String expected = "INSERT INTO `EMPS`\n" - + "(SELECT *\n" + + "SELECT *\n" + "FROM `EMPS1`\n" + "UNION\n" + "SELECT *\n" - + "FROM `EMPS2`)"; + + "FROM `EMPS2`"; sql("insert into emps select * from emps1 union select * from emps2") .ok(expected); } @@ -8458,14 +8498,14 @@ protected void checkTimeUnitCodes(Map timeUnitCodes) { + " select y from b)\n" + "except\n" + "(select z from c)"; - final String expected = "((SELECT `X`\n" + final String expected = "SELECT `X`\n" + "FROM `A`\n" + "UNION\n" + "SELECT `Y`\n" - + "FROM `B`)\n" + + "FROM `B`\n" + "EXCEPT\n" + "SELECT `Z`\n" - + "FROM `C`)"; + + "FROM `C`"; sql(sql).ok(expected); }