You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jb...@apache.org on 2022/06/23 13:48:29 UTC
[calcite] branch main updated: [CALCITE-5013] Unparse SqlSetOperator should be retained parentheses when generating SQL for UNION ... LIMIT
This is an automated email from the ASF dual-hosted git repository.
jbalint 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 b2fb233426 [CALCITE-5013] Unparse SqlSetOperator should be retained parentheses when generating SQL for UNION ... LIMIT
b2fb233426 is described below
commit b2fb2334260fc8c8bd6ab147da24b10f6c772ea8
Author: xiejiajun <ji...@foxmail.com>
AuthorDate: Sat Feb 19 15:12:04 2022 +0800
[CALCITE-5013] Unparse SqlSetOperator should be retained parentheses when generating SQL for UNION ... LIMIT
Co-authored-by: Julian Hyde <jh...@apache.org>
---
.../java/org/apache/calcite/sql/SqlSelect.java | 12 ++-
.../calcite/sql/fun/SqlInternalOperators.java | 32 ++++++++
.../calcite/rel/rel2sql/RelToSqlConverterTest.java | 78 ++++++++++++++++++
.../apache/calcite/sql/parser/SqlParserTest.java | 95 ++++++++++++++++++++++
4 files changed, 216 insertions(+), 1 deletion(-)
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlSelect.java b/core/src/main/java/org/apache/calcite/sql/SqlSelect.java
index 4ce27f0668..6f51cb3f4d 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlSelect.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlSelect.java
@@ -16,6 +16,7 @@
*/
package org.apache.calcite.sql;
+import org.apache.calcite.sql.fun.SqlInternalOperators;
import org.apache.calcite.sql.parser.SqlParserPos;
import org.apache.calcite.sql.validate.SqlValidator;
import org.apache.calcite.sql.validate.SqlValidatorScope;
@@ -249,7 +250,16 @@ public class SqlSelect extends SqlCall {
// Override SqlCall, to introduce a sub-query frame.
@Override public void unparse(SqlWriter writer, int leftPrec, int rightPrec) {
- if (!writer.inQuery()) {
+ if (!writer.inQuery()
+ || getFetch() != null
+ && (leftPrec > SqlInternalOperators.FETCH.getLeftPrec()
+ || rightPrec > SqlInternalOperators.FETCH.getLeftPrec())
+ || getOffset() != null
+ && (leftPrec > SqlInternalOperators.OFFSET.getLeftPrec()
+ || rightPrec > SqlInternalOperators.OFFSET.getLeftPrec())
+ || getOrderList() != null
+ && (leftPrec > SqlOrderBy.OPERATOR.getLeftPrec()
+ || rightPrec > SqlOrderBy.OPERATOR.getRightPrec())) {
// If this SELECT is the topmost item in a sub-query, introduce a new
// frame. (The topmost item in the sub-query might be a UNION or
// ORDER. In this case, we don't need a wrapper frame.)
diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlInternalOperators.java b/core/src/main/java/org/apache/calcite/sql/fun/SqlInternalOperators.java
index c3f12806f9..a6cfc789e9 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlInternalOperators.java
+++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlInternalOperators.java
@@ -24,6 +24,7 @@ import org.apache.calcite.sql.SqlNode;
import org.apache.calcite.sql.SqlNodeList;
import org.apache.calcite.sql.SqlOperator;
import org.apache.calcite.sql.SqlOperatorTable;
+import org.apache.calcite.sql.SqlSyntax;
import org.apache.calcite.sql.SqlWriter;
import org.apache.calcite.sql.type.InferTypes;
import org.apache.calcite.sql.type.OperandTypes;
@@ -119,4 +120,35 @@ public abstract class SqlInternalOperators {
public static final SqlInternalOperator GROUP_BY_DISTINCT =
new SqlRollupOperator("GROUP BY DISTINCT", SqlKind.GROUP_BY_DISTINCT);
+ /** Fetch operator is ONLY used for its precedence during unparsing. */
+ public static final SqlOperator FETCH =
+ SqlBasicOperator.create("FETCH")
+ .withPrecedence(SqlStdOperatorTable.UNION.getLeftPrec() - 2, true);
+
+ /** Offset operator is ONLY used for its precedence during unparsing. */
+ public static final SqlOperator OFFSET =
+ SqlBasicOperator.create("OFFSET")
+ .withPrecedence(SqlStdOperatorTable.UNION.getLeftPrec() - 2, true);
+
+ /** Subject to change. */
+ private static class SqlBasicOperator extends SqlOperator {
+ @Override public SqlSyntax getSyntax() {
+ return SqlSyntax.SPECIAL;
+ }
+
+ /** Private constructor. Use {@link #create}. */
+ private SqlBasicOperator(String name, int leftPrecedence, int rightPrecedence) {
+ super(name, SqlKind.OTHER, leftPrecedence, rightPrecedence,
+ ReturnTypes.BOOLEAN, InferTypes.RETURN_TYPE, OperandTypes.ANY);
+ }
+
+ static SqlBasicOperator create(String name) {
+ return new SqlBasicOperator(name, 0, 0);
+ }
+
+ SqlBasicOperator withPrecedence(int prec, boolean leftAssoc) {
+ return new SqlBasicOperator(getName(), leftPrec(prec, leftAssoc),
+ rightPrec(prec, leftAssoc));
+ }
+ }
}
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 b077c37cdb..2ea4231930 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
@@ -3374,6 +3374,84 @@ class RelToSqlConverterTest {
sql(query).ok(expected);
}
+
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-5013">[CALCITE-5013]
+ * Unparse SqlSetOperator should be retained parentheses
+ * when the operand has limit or offset</a>. */
+ @Test void testSetOpRetainParentheses() {
+ // Parentheses will be discarded, because semantics not be affected.
+ final String discardedParenthesesQuery = "SELECT \"product_id\" FROM \"product\""
+ + "UNION ALL\n"
+ + "(SELECT \"product_id\" FROM \"product\" WHERE \"product_id\" > 10)\n"
+ + "INTERSECT ALL\n"
+ + "(SELECT \"product_id\" FROM \"product\" )";
+ final String discardedParenthesesRes = "SELECT \"product_id\"\n"
+ + "FROM \"foodmart\".\"product\"\n"
+ + "UNION ALL\n"
+ + "SELECT *\n"
+ + "FROM (SELECT \"product_id\"\n"
+ + "FROM \"foodmart\".\"product\"\n"
+ + "WHERE \"product_id\" > 10\n"
+ + "INTERSECT ALL\n"
+ + "SELECT \"product_id\"\n"
+ + "FROM \"foodmart\".\"product\")";
+ sql(discardedParenthesesQuery).ok(discardedParenthesesRes);
+
+ // Parentheses will be retained because sub-query has LIMIT or OFFSET.
+ // If parentheses are discarded the semantics of parsing will be affected.
+ final String allSetOpQuery = "SELECT \"product_id\" FROM \"product\""
+ + "UNION ALL\n"
+ + "(SELECT \"product_id\" FROM \"product\" LIMIT 10)\n"
+ + "INTERSECT ALL\n"
+ + "(SELECT \"product_id\" FROM \"product\" OFFSET 10)\n"
+ + "EXCEPT ALL\n"
+ + "(SELECT \"product_id\" FROM \"product\" LIMIT 5 OFFSET 5)";
+ final String allSetOpRes = "SELECT *\n"
+ + "FROM (SELECT \"product_id\"\n"
+ + "FROM \"foodmart\".\"product\"\n"
+ + "UNION ALL\n"
+ + "SELECT *\n"
+ + "FROM ((SELECT \"product_id\"\n"
+ + "FROM \"foodmart\".\"product\"\n"
+ + "FETCH NEXT 10 ROWS ONLY)\n"
+ + "INTERSECT ALL\n"
+ + "(SELECT \"product_id\"\n"
+ + "FROM \"foodmart\".\"product\"\n"
+ + "OFFSET 10 ROWS)))\n"
+ + "EXCEPT ALL\n"
+ + "(SELECT \"product_id\"\n"
+ + "FROM \"foodmart\".\"product\"\n"
+ + "OFFSET 5 ROWS\n"
+ + "FETCH NEXT 5 ROWS ONLY)";
+ sql(allSetOpQuery).ok(allSetOpRes);
+
+ // After the config is enabled, order by will be retained, so parentheses are required.
+ final String retainOrderQuery = "SELECT \"product_id\" FROM \"product\""
+ + "UNION ALL\n"
+ + "(SELECT \"product_id\" FROM \"product\" ORDER BY \"product_id\")";
+ final String retainOrderResult = "SELECT \"product_id\"\n"
+ + "FROM \"foodmart\".\"product\"\n"
+ + "UNION ALL\n"
+ + "(SELECT \"product_id\"\n"
+ + "FROM \"foodmart\".\"product\"\n"
+ + "ORDER BY \"product_id\")";
+ sql(retainOrderQuery).withConfig(c -> c.withRemoveSortInSubQuery(false)).ok(retainOrderResult);
+
+ // Parentheses are required to keep ORDER and LIMIT on the sub-query.
+ final String retainLimitQuery = "SELECT \"product_id\" FROM \"product\""
+ + "UNION ALL\n"
+ + "(SELECT \"product_id\" FROM \"product\" ORDER BY \"product_id\" LIMIT 2)";
+ final String retainLimitResult = "SELECT \"product_id\"\n"
+ + "FROM \"foodmart\".\"product\"\n"
+ + "UNION ALL\n"
+ + "(SELECT \"product_id\"\n"
+ + "FROM \"foodmart\".\"product\"\n"
+ + "ORDER BY \"product_id\"\n"
+ + "FETCH NEXT 2 ROWS ONLY)";
+ sql(retainLimitQuery).ok(retainLimitResult);
+ }
+
/** Test case for
* <a href="https://issues.apache.org/jira/browse/CALCITE-4674">[CALCITE-4674]
* Excess quotes in generated SQL when STAR is a column alias</a>. */
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 f34ea90263..e6657ee8d0 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
@@ -2617,6 +2617,101 @@ public class SqlParserTest {
.fails("(?s).*Encountered \"union\" at .*");
}
+ @Test void testLimitUnion2() {
+ // LIMIT is allowed in a parenthesized sub-query inside UNION;
+ // the result probably has more parentheses than strictly necessary.
+ 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"
+ + "FROM `T`\n"
+ + "FETCH NEXT 10 ROWS ONLY)\n"
+ + "UNION ALL\n"
+ + "(SELECT `B`\n"
+ + "FROM `T`\n"
+ + "OFFSET 20 ROWS))";
+ sql(sql).ok(expected);
+ }
+
+ @Test void testUnionOffset() {
+ // Note that the second sub-query has parentheses, to ensure that ORDER BY,
+ // OFFSET, FETCH are associated with just that sub-query, not the UNION.
+ 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"
+ + "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))";
+ 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"
+ + "FROM `T`\n"
+ + "UNION ALL\n"
+ + "(SELECT `B`\n"
+ + "FROM `T`\n"
+ + "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"
+ + "FROM `T`\n"
+ + "UNION ALL\n"
+ + "(SELECT `B`\n"
+ + "FROM `T`\n"
+ + "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"
+ + "FROM `T`\n"
+ + "UNION ALL\n"
+ + "(SELECT `B`\n"
+ + "FROM `T`\n"
+ + "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"
+ + "FROM `T`\n"
+ + "UNION ALL\n"
+ + "(SELECT `B`\n"
+ + "FROM `T`\n"
+ + "OFFSET 3 ROWS\n"
+ + "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"
+ + "FROM `T`\n"
+ + "INTERSECT\n"
+ + "(SELECT `B`\n"
+ + "FROM `T`\n"
+ + "OFFSET 3 ROWS\n"
+ + "FETCH NEXT 5 ROWS ONLY))";
+ sql(sql6).ok(expected6);
+ }
+
@Test void testUnionOfNonQueryFails() {
sql("select 1 from emp union ^2^ + 5")
.fails("Non-query expression encountered in illegal context");