[GitHub] [calcite] l4wei commented on a diff in pull request #2952: [CALCITE-5252] JDBC adapter sometimes miss parentheses around SELECT in WITH_ITEM body

[GitBox <gi...@apache.org>]

l4wei commented on code in PR #2952:
URL: https://github.com/apache/calcite/pull/2952#discussion_r1010014089


##########
core/src/main/java/org/apache/calcite/sql/SqlWithItem.java:
##########
@@ -97,7 +97,7 @@ private static class SqlWithItemOperator extends SqlSpecialOperator {
         withItem.columnList.unparse(writer, getLeftPrec(), getRightPrec());
       }
       writer.keyword("AS");
-      withItem.query.unparse(writer, 10, 10);
+      withItem.query.unparse(writer, 100, 100);

Review Comment:
   IMHO, in essence, parentheses should be provided by subquery like `SqlSelect` or `SqlCall` to indicate the order of execution. Return to the calcite repo, I noticed current implements have followed the principle I mentioned just now, so the approach in current PR has done it in the same style. On the other hand, redundant  parentheses around `SqlSelect` will be produced in many test cases if using the approach mentioned by you, e.g. testWith2, testWithOrderBy, testWith, testSingleQuotedAlias and so on.



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org