[GitHub] [calcite] libenchao commented on a diff in pull request #2938: [CALCITE-5299] JDBC adapter sometimes adds unnecessary parentheses around SELECT in WITH body

[GitBox]

libenchao commented on code in PR #2938:
URL: https://github.com/apache/calcite/pull/2938#discussion_r1003932604


##########
core/src/main/java/org/apache/calcite/sql/pretty/SqlPrettyWriter.java:
##########
@@ -398,10 +398,14 @@ public void setWindowDeclListNewline(boolean 
windowDeclListNewline) {
     return (frame == null)
         || (frame.frameType == FrameTypeEnum.SELECT)
         || (frame.frameType == FrameTypeEnum.ORDER_BY)
-        || (frame.frameType == FrameTypeEnum.WITH)
+        || (frame.frameType == FrameTypeEnum.WITH_BODY)

Review Comment:
   Per the changes in this PR, you are right that the semantic for `inQuery` 
does not change too much, I'm fine to leave it is for now.  
   The concern I raised is that `FrameTypeEnum#SELECT` is used vaguely to my 
understanding, e.g., we both use `SELECT` frame in `SqlInsert` and 
`SqlSelectOperator`, but they are different in my opinion: `SqlInsert` use 
`SELECT` frame to indicate that we'll go to a `SELECT QUERY`, and 
`SqlSelectOperator` use `SELECT` frame to indicate that we are in a `SELECT 
QUERY`.



-- 
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-unsubscr...@calcite.apache.org

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