From b16df019ed9fc7dba7392be9b758358c5a4e927b Mon Sep 17 00:00:00 2001 From: "wumou.wm" Date: Sat, 17 Sep 2022 17:19:44 +0800 Subject: [PATCH] [CALCITE-5265] JDBC adapter sometimes adds unnecessary parentheses around SELECT in INSERT This closes #2910 --- .../calcite/sql/pretty/SqlPrettyWriter.java | 1 + .../rel/rel2sql/RelToSqlConverterTest.java | 46 ++++++++++++++++-- .../apache/calcite/test/JdbcAdapterTest.java | 4 +- .../calcite/sql/parser/SqlParserTest.java | 48 +++++++++---------- 4 files changed, 69 insertions(+), 30 deletions(-) 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 e39e5ffa7bc6..57cbf5f75092 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 @@ -396,6 +396,7 @@ public void setWindowDeclListNewline(boolean windowDeclListNewline) { @Override public boolean inQuery() { return (frame == null) + || (frame.frameType == FrameTypeEnum.SELECT) || (frame.frameType == FrameTypeEnum.ORDER_BY) || (frame.frameType == FrameTypeEnum.WITH) || (frame.frameType == FrameTypeEnum.SETOP); diff --git a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java index 0db4ea246639..e21ac0ede0ce 100644 --- a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java +++ b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java @@ -6160,7 +6160,7 @@ private void checkLiteral2(String expression, String expected) { final String expected = "INSERT INTO \"foodmart\".\"account\" (" + "\"account_id\", \"account_parent\", \"account_description\", " + "\"account_type\", \"account_rollup\", \"Custom_Members\")\n" - + "(SELECT \"EXPR$0\" AS \"account_id\"," + + "SELECT \"EXPR$0\" AS \"account_id\"," + " \"EXPR$1\" AS \"account_parent\"," + " CAST(NULL AS VARCHAR(30) CHARACTER SET \"ISO-8859-1\") " + "AS \"account_description\"," @@ -6169,7 +6169,7 @@ private void checkLiteral2(String expression, String expected) { + " CAST(NULL AS VARCHAR(255) CHARACTER SET \"ISO-8859-1\") " + "AS \"Custom_Members\"\n" + "FROM (VALUES (1, NULL, '123', '123')) " - + "AS \"t\" (\"EXPR$0\", \"EXPR$1\", \"EXPR$2\", \"EXPR$3\"))"; + + "AS \"t\" (\"EXPR$0\", \"EXPR$1\", \"EXPR$2\", \"EXPR$3\")"; sql(query).ok(expected); // validate sql(expected).exec(); @@ -6189,7 +6189,7 @@ private void checkLiteral2(String expression, String expected) { final String expected = "INSERT INTO \"foodmart\".\"account\" " + "(\"account_id\", \"account_parent\", \"account_description\", " + "\"account_type\", \"account_rollup\", \"Custom_Members\")\n" - + "(SELECT \"product\".\"product_id\" AS \"account_id\", " + + "SELECT \"product\".\"product_id\" AS \"account_id\", " + "CAST(NULL AS INTEGER) AS \"account_parent\", CAST(NULL AS VARCHAR" + "(30) CHARACTER SET \"ISO-8859-1\") AS \"account_description\", " + "CAST(\"product\".\"product_id\" AS VARCHAR CHARACTER SET " @@ -6199,7 +6199,7 @@ private void checkLiteral2(String expression, String expected) { + "CAST(NULL AS VARCHAR(255) CHARACTER SET \"ISO-8859-1\") AS \"Custom_Members\"\n" + "FROM \"foodmart\".\"product\"\n" + "INNER JOIN \"foodmart\".\"sales_fact_1997\" " - + "ON \"product\".\"product_id\" = \"sales_fact_1997\".\"product_id\")"; + + "ON \"product\".\"product_id\" = \"sales_fact_1997\".\"product_id\""; sql(query).ok(expected); // validate sql(expected).exec(); @@ -6367,6 +6367,44 @@ private void checkLiteral2(String expression, String expected) { .withCalcite().ok(expectedCalciteX); } + /** Test case for + * [CALCITE-5265] + * JDBC adapter sometimes adds unnecessary parentheses around SELECT in INSERT. */ + @Test void testInsertSelect() { + final String sql = "insert into \"DEPT\" select * from \"DEPT\""; + final String expected = "" + + "INSERT INTO \"SCOTT\".\"DEPT\" (\"DEPTNO\", \"DNAME\", \"LOC\")\n" + + "SELECT *\n" + + "FROM \"SCOTT\".\"DEPT\""; + sql(sql) + .schema(CalciteAssert.SchemaSpec.JDBC_SCOTT) + .ok(expected); + } + + /** Test case for + * [CALCITE-5265] + * JDBC adapter sometimes adds unnecessary parentheses around SELECT in INSERT. */ + @Test void testInsertUnionThenIntersect() { + final String sql = "" + + "insert into \"DEPT\"\n" + + "(select * from \"DEPT\" union select * from \"DEPT\")\n" + + "intersect select * from \"DEPT\""; + final String expected = "" + + "INSERT INTO \"SCOTT\".\"DEPT\" (\"DEPTNO\", \"DNAME\", \"LOC\")\n" + + "SELECT *\n" + + "FROM (SELECT *\n" + + "FROM \"SCOTT\".\"DEPT\"\n" + + "UNION\n" + + "SELECT *\n" + + "FROM \"SCOTT\".\"DEPT\")\n" + + "INTERSECT\n" + + "SELECT *\n" + + "FROM \"SCOTT\".\"DEPT\""; + sql(sql) + .schema(CalciteAssert.SchemaSpec.JDBC_SCOTT) + .ok(expected); + } + @Test void testInsertValuesWithDynamicParams() { final String sql = "insert into \"DEPT\" values (?,?,?), (?,?,?)"; final String expected = "" diff --git a/core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java b/core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java index 501c2485e434..564fe6dab9ba 100644 --- a/core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java +++ b/core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java @@ -982,11 +982,11 @@ private LockWrapper exclusiveCleanDb(Connection c) throws SQLException { final String jdbcSql = "INSERT INTO \"foodmart\".\"expense_fact\"" + " (\"store_id\", \"account_id\", \"exp_date\", \"time_id\"," + " \"category_id\", \"currency_id\", \"amount\")\n" - + "(SELECT \"store_id\", \"account_id\", \"exp_date\"," + + "SELECT \"store_id\", \"account_id\", \"exp_date\"," + " \"time_id\" + 1 AS \"time_id\", \"category_id\"," + " \"currency_id\", \"amount\"\n" + "FROM \"foodmart\".\"expense_fact\"\n" - + "WHERE \"store_id\" = 666)"; + + "WHERE \"store_id\" = 666"; that.query(sql) .explainContains(explain) .planUpdateHasSql(jdbcSql, 1); 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 198518f0db2b..4a9a01803dd2 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 @@ -4410,8 +4410,8 @@ void checkPeriodPredicate(Checker checker) { @Test void testInsertSelect() { final String expected = "INSERT INTO `EMPS`\n" - + "(SELECT *\n" - + "FROM `EMPS`)"; + + "SELECT *\n" + + "FROM `EMPS`"; sql("insert into emps select * from emps") .ok(expected) .node(not(isDdl())); @@ -4456,29 +4456,29 @@ void checkPeriodPredicate(Checker checker) { @Test void testInsertColumnList() { final String expected = "INSERT INTO `EMPS` (`X`, `Y`)\n" - + "(SELECT *\n" - + "FROM `EMPS`)"; + + "SELECT *\n" + + "FROM `EMPS`"; sql("insert into emps(x,y) select * from emps") .ok(expected); } @Test void testInsertCaseSensitiveColumnList() { final String expected = "INSERT INTO `emps` (`x`, `y`)\n" - + "(SELECT *\n" - + "FROM `EMPS`)"; + + "SELECT *\n" + + "FROM `EMPS`"; sql("insert into \"emps\"(\"x\",\"y\") select * from emps") .ok(expected); } @Test void testInsertExtendedColumnList() { String expected = "INSERT INTO `EMPS` EXTEND (`Z` BOOLEAN) (`X`, `Y`)\n" - + "(SELECT *\n" - + "FROM `EMPS`)"; + + "SELECT *\n" + + "FROM `EMPS`"; sql("insert into emps(z boolean)(x,y) select * from emps") .ok(expected); expected = "INSERT INTO `EMPS` EXTEND (`Z` BOOLEAN) (`X`, `Y`, `Z`)\n" - + "(SELECT *\n" - + "FROM `EMPS`)"; + + "SELECT *\n" + + "FROM `EMPS`"; sql("insert into emps(x, y, z boolean) select * from emps") .withConformance(SqlConformanceEnum.LENIENT) .ok(expected); @@ -4515,13 +4515,13 @@ void checkPeriodPredicate(Checker checker) { @Test void testInsertCaseSensitiveExtendedColumnList() { String expected = "INSERT INTO `emps` EXTEND (`z` BOOLEAN) (`x`, `y`)\n" - + "(SELECT *\n" - + "FROM `EMPS`)"; + + "SELECT *\n" + + "FROM `EMPS`"; sql("insert into \"emps\"(\"z\" boolean)(\"x\",\"y\") select * from emps") .ok(expected); expected = "INSERT INTO `emps` EXTEND (`z` BOOLEAN) (`x`, `y`, `z`)\n" - + "(SELECT *\n" - + "FROM `EMPS`)"; + + "SELECT *\n" + + "FROM `EMPS`"; sql("insert into \"emps\"(\"x\", \"y\", \"z\" boolean) select * from emps") .withConformance(SqlConformanceEnum.LENIENT) .ok(expected); @@ -4531,8 +4531,8 @@ void checkPeriodPredicate(Checker checker) { final String expected = "EXPLAIN PLAN INCLUDING ATTRIBUTES" + " WITH IMPLEMENTATION FOR\n" + "INSERT INTO `EMPS1`\n" - + "(SELECT *\n" - + "FROM `EMPS2`)"; + + "SELECT *\n" + + "FROM `EMPS2`"; sql("explain plan for insert into emps1 select * from emps2") .ok(expected) .node(not(isDdl())); @@ -4552,8 +4552,8 @@ void checkPeriodPredicate(Checker checker) { @Test void testUpsertSelect() { final String sql = "upsert into emps select * from emp as e"; final String expected = "UPSERT INTO `EMPS`\n" - + "(SELECT *\n" - + "FROM `EMP` AS `E`)"; + + "SELECT *\n" + + "FROM `EMP` AS `E`"; if (isReserved("UPSERT")) { sql(sql).ok(expected); } @@ -6672,8 +6672,8 @@ public void subTestIntervalDayFailsValidation() { } }); final String str0 = "INSERT INTO `EMPS`\n" - + "(SELECT *\n" - + "FROM `EMPS`)"; + + "SELECT *\n" + + "FROM `EMPS`"; assertThat(str0, is(toLinux(sqlNodeVisited0.toString()))); final String sql1 = "insert into emps select empno from emps"; @@ -6685,8 +6685,8 @@ public void subTestIntervalDayFailsValidation() { } }); final String str1 = "INSERT INTO `EMPS`\n" - + "(SELECT `EMPNO`\n" - + "FROM `EMPS`)"; + + "SELECT `EMPNO`\n" + + "FROM `EMPS`"; assertThat(str1, is(toLinux(sqlNodeVisited1.toString()))); } @@ -10177,8 +10177,8 @@ protected void checkTimeUnitCodes(Map timeUnitCodes) { + "select * from emps"; final String expected = "INSERT INTO `EMPS`\n" + "/*+ `PROPERTIES`(`K1` = 'v1', `K2` = 'v2'), `INDEX`(`IDX0`, `IDX1`) */\n" - + "(SELECT *\n" - + "FROM `EMPS`)"; + + "SELECT *\n" + + "FROM `EMPS`"; sql(sql).ok(expected); }