You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by li...@apache.org on 2022/10/25 11:22:14 UTC
[calcite] branch main updated: [CALCITE-5299] JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH body
This is an automated email from the ASF dual-hosted git repository.
libenchao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new defea67b80 [CALCITE-5299] JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH body
defea67b80 is described below
commit defea67b80fe432c5a8f776718169a3595b222a8
Author: wumou.wm <wu...@outlook.com>
AuthorDate: Sat Oct 15 02:20:57 2022 +0800
[CALCITE-5299] JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH body
Remove SET_QUERY from SqlKind#EXPRESSION.
This closes #2938
---
.../main/java/org/apache/calcite/sql/SqlKind.java | 2 +-
.../main/java/org/apache/calcite/sql/SqlWith.java | 5 +-
.../java/org/apache/calcite/sql/SqlWriter.java | 5 +
.../apache/calcite/sql/pretty/SqlPrettyWriter.java | 2 +-
.../apache/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 e6ef04d619..9dff2716d8 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 06171be8c2..091b65d8ff 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 @@ public class SqlWith extends SqlCall {
}
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 4eae8a165c..9f84632302 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 @@ public interface SqlWriter {
*/
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 13f43e8b50..1471565a8f 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 class SqlPrettyWriter implements SqlWriter {
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 c14c291734..520cb55b06 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 @@ public class SqlParserTest {
+ "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 @@ public class SqlParserTest {
+ "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 @@ public class SqlParserTest {
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 @@ public class SqlParserTest {
sql(sql).fails("(?s)Encountered \"with\" at .*");
}
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-5299">[CALCITE-5299]
+ * JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH body</a>. */
+ @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
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-5299">[CALCITE-5299]
+ * JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH body</a>. */
+ @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 @@ public class SqlParserTest {
+ " 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 @@ public class SqlParserTest {
+ "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 @@ public class SqlParserTest {
.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 @@ public class SqlParserTest {
// 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 @@ public class SqlParserTest {
+ "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 @@ public class SqlParserTest {
@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 @@ public class SqlParserTest {
+ "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 @@ public class SqlParserTest {
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 @@ public class SqlParserTest {
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 @@ public class SqlParserTest {
@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 @@ public class SqlParserTest {
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 @@ public class SqlParserTest {
@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 @@ public class SqlParserTest {
@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 @@ public class SqlParserTest {
+ "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 @@ public class SqlParserTest {
+ "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 @@ public class SqlParserTest {
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 @@ public class SqlParserTest {
@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 @@ public class SqlParserTest {
+ " 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);
}