JiajunBernoulli commented on a change in pull request #2724:
URL: https://github.com/apache/calcite/pull/2724#discussion_r812579698
##########
File path:
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
##########
@@ -3301,6 +3301,85 @@ private void checkHavingAliasSameAsColumn(boolean
upperAlias) {
sql(query).ok(expected);
}
+ @Test void testSetOpLimitShouldRetainParentheses() {
+ // If the subquery parentheses of limit are discarded,
+ // the semantics of parsing will be affected.
+ // Parentheses should be retained, otherwise not required.
+ final String query = "select \"product_id\" from \"product\""
+ + "union all\n"
+ + "(select \"product_id\" from \"product\" limit 10)"
+ + "intersect all\n"
+ + "(select \"product_id\" from \"product\")";
+ final String expected = "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\")";
+ sql(query).ok(expected);
+
+ // Offset is the same as limit.
+ final String unionOffsetQuery = "select \"product_id\" from(\n"
+ + "(select \"product_id\" from \"product\" limit 10)\n"
+ + "union all\n"
+ + "(select \"product_id\" from \"product\" offset 5)\n"
+ + "union all\n"
+ + "(select \"product_id\" from \"product\" order by \"product_id\"
limit 5 offset 5))";
+ final String unionOffsetRes = "SELECT *\n"
+ + "FROM (SELECT *\n"
+ + "FROM ((SELECT \"product_id\"\n"
+ + "FROM \"foodmart\".\"product\"\n"
+ + "FETCH NEXT 10 ROWS ONLY)\n"
+ + "UNION ALL\n"
+ + "(SELECT \"product_id\"\n"
+ + "FROM \"foodmart\".\"product\"\n"
+ + "OFFSET 5 ROWS))\n"
+ + "UNION ALL\n"
+ + "(SELECT \"product_id\"\n"
+ + "FROM \"foodmart\".\"product\"\n"
+ + "ORDER BY \"product_id\"\n"
+ + "OFFSET 5 ROWS\n"
+ + "FETCH NEXT 5 ROWS ONLY)) AS \"t6\"";
+ sql(unionOffsetQuery).ok(unionOffsetRes);
+ }
+
+ @Test void testSetOpOrderMaybeRetainParentheses() {
+ // Order by without limit/offset will be removed, so parentheses are not
required.
Review comment:
Ok, I've deleted this test case.
##########
File path:
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
##########
@@ -3301,6 +3301,85 @@ private void checkHavingAliasSameAsColumn(boolean
upperAlias) {
sql(query).ok(expected);
}
+ @Test void testSetOpLimitShouldRetainParentheses() {
Review comment:
There has been a case of UNION and INTERSECT before, I've added a
combination of set-ops.
##########
File path: core/src/main/java/org/apache/calcite/sql/SqlUtil.java
##########
@@ -417,24 +418,52 @@ public static void unparseSqlIdentifierSyntax(
writer.endList(frame);
}
+ private static void unparseSetOpSyntax(
+ SqlNode sqlNode,
+ SqlWriter writer,
+ int leftPrec,
+ int rightPrec) {
+ SqlSelect sqlSelect = (SqlSelect) sqlNode;
+ if (sqlSelect.getOrderList() != null
+ || sqlSelect.getFetch() != null
+ || sqlSelect.getOffset() != null) {
+ final SqlWriter.Frame operandFrame0 =
writer.startList(FrameTypeEnum.SETOP, "(", ")");
+ sqlSelect.unparse(writer, leftPrec, rightPrec);
+ writer.endList(operandFrame0);
+ } else {
+ sqlSelect.unparse(writer, leftPrec, rightPrec);
+ }
+ }
+
public static void unparseBinarySyntax(
SqlOperator operator,
SqlCall call,
SqlWriter writer,
int leftPrec,
int rightPrec) {
assert call.operandCount() == 2;
+ boolean isSetOp = operator instanceof SqlSetOperator;
final SqlWriter.Frame frame =
writer.startList(
- (operator instanceof SqlSetOperator)
+ isSetOp
? SqlWriter.FrameTypeEnum.SETOP
: SqlWriter.FrameTypeEnum.SIMPLE);
- call.operand(0).unparse(writer, leftPrec, operator.getLeftPrec());
+ SqlNode operand0 = call.operand(0);
+ if (isSetOp && operand0.getKind() == SqlKind.SELECT) {
+ unparseSetOpSyntax(operand0, writer, leftPrec, operator.getLeftPrec());
+ } else {
+ operand0.unparse(writer, leftPrec, operator.getLeftPrec());
+ }
final boolean needsSpace = operator.needsSpace();
writer.setNeedWhitespace(needsSpace);
writer.sep(operator.getName());
writer.setNeedWhitespace(needsSpace);
- call.operand(1).unparse(writer, operator.getRightPrec(), rightPrec);
+ SqlNode operand1 = call.operand(1);
Review comment:
I moved it to `unparseBinaryOperand`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
