You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by GitBox <gi...@apache.org> on 2020/06/09 16:14:27 UTC
[GitHub] [calcite] xy2953396112 opened a new pull request #2014: [CALCITE-4022] Support evaluate SqlInsert SqlBasicCall.toString()
xy2953396112 opened a new pull request #2014:
URL: https://github.com/apache/calcite/pull/2014
https://issues.apache.org/jira/browse/CALCITE-4022
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] yanlin-Lynn commented on a change in pull request #2014: [CALCITE-4022] Support evaluate SqlInsert SqlBasicCall.toString()
Posted by GitBox <gi...@apache.org>.
yanlin-Lynn commented on a change in pull request #2014:
URL: https://github.com/apache/calcite/pull/2014#discussion_r453260138
##########
File path: core/src/main/java/org/apache/calcite/sql/SqlUtil.java
##########
@@ -261,6 +261,32 @@ public static boolean isLiteralChain(SqlNode node) {
}
}
+ /**
+ * Unparses a call to an operator which has special syntax.
+ *
+ * @param operator The operator
+ * @param writer Writer
+ * @param call List of 0 or more operands
+ */
+ public static void unparseSpecialSyntax(SqlOperator operator,
+ SqlWriter writer,
+ SqlCall call) {
+ final List<SqlNode> operands = call.getOperandList();
+ switch (operator.getKind()) {
+ case INSERT:
+ assert operands.size() == 3 || operands.size() == 4;
+ final SqlInsert insert = new SqlInsert(call.getParserPosition(),
+ (SqlNodeList) operands.get(0), operands.get(1), operands.get(2),
+ operands.size() == 3 ? null : (SqlNodeList) call.getOperandList().get(3));
Review comment:
The size of operands is always 4, right?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] xy2953396112 commented on pull request #2014: [CALCITE-4022] Support unparse special syntax when operator is INSERT
Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on pull request #2014:
URL: https://github.com/apache/calcite/pull/2014#issuecomment-660100233
> The commit log is confusing, can you make it more clearer, like "Support unparse special syntax when operator is INSERT"
Thanks, update it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] xy2953396112 commented on a change in pull request #2014: [CALCITE-4022] Support evaluate SqlInsert SqlBasicCall.toString()
Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on a change in pull request #2014:
URL: https://github.com/apache/calcite/pull/2014#discussion_r454962707
##########
File path: core/src/main/java/org/apache/calcite/sql/SqlUtil.java
##########
@@ -261,6 +261,32 @@ public static boolean isLiteralChain(SqlNode node) {
}
}
+ /**
+ * Unparses a call to an operator which has special syntax.
+ *
+ * @param operator The operator
+ * @param writer Writer
+ * @param call List of 0 or more operands
+ */
+ public static void unparseSpecialSyntax(SqlOperator operator,
Review comment:
`SqlUtil#unparseSpecialSyntax` is used for unparse `SqlSyntax.SPECIAL`. other processing logic also exists in SqlUtil.
##########
File path: core/src/main/java/org/apache/calcite/sql/SqlUtil.java
##########
@@ -261,6 +261,32 @@ public static boolean isLiteralChain(SqlNode node) {
}
}
+ /**
+ * Unparses a call to an operator which has special syntax.
+ *
+ * @param operator The operator
+ * @param writer Writer
+ * @param call List of 0 or more operands
+ */
+ public static void unparseSpecialSyntax(SqlOperator operator,
+ SqlWriter writer,
+ SqlCall call) {
+ final List<SqlNode> operands = call.getOperandList();
+ switch (operator.getKind()) {
+ case INSERT:
+ assert operands.size() == 3 || operands.size() == 4;
+ final SqlInsert insert = new SqlInsert(call.getParserPosition(),
+ (SqlNodeList) operands.get(0), operands.get(1), operands.get(2),
+ operands.size() == 3 ? null : (SqlNodeList) call.getOperandList().get(3));
Review comment:
@yanlin-Lynn @zinking
Thanks.The size of operands is always 4, update the code.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zinking commented on a change in pull request #2014: [CALCITE-4022] Support evaluate SqlInsert SqlBasicCall.toString()
Posted by GitBox <gi...@apache.org>.
zinking commented on a change in pull request #2014:
URL: https://github.com/apache/calcite/pull/2014#discussion_r453618841
##########
File path: core/src/main/java/org/apache/calcite/sql/SqlUtil.java
##########
@@ -261,6 +261,32 @@ public static boolean isLiteralChain(SqlNode node) {
}
}
+ /**
+ * Unparses a call to an operator which has special syntax.
+ *
+ * @param operator The operator
+ * @param writer Writer
+ * @param call List of 0 or more operands
+ */
+ public static void unparseSpecialSyntax(SqlOperator operator,
+ SqlWriter writer,
+ SqlCall call) {
+ final List<SqlNode> operands = call.getOperandList();
+ switch (operator.getKind()) {
+ case INSERT:
+ assert operands.size() == 3 || operands.size() == 4;
+ final SqlInsert insert = new SqlInsert(call.getParserPosition(),
+ (SqlNodeList) operands.get(0), operands.get(1), operands.get(2),
+ operands.size() == 3 ? null : (SqlNodeList) call.getOperandList().get(3));
Review comment:
I mean:
1. why `null`, is there a better value to pass
2. call.getOperandList() => operands
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] xy2953396112 commented on pull request #2014: [CALCITE-4022] Support evaluate SqlInsert SqlBasicCall.toString()
Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on pull request #2014:
URL: https://github.com/apache/calcite/pull/2014#issuecomment-652405333
> I guess you need to clarify the issue first in JIRA.
> implementation looks weird before that.
Thanks for review. If a sql `insert into emps select * from emps` visited by SqlShuttle, and then `System.out.println(sqlNodeVisited.toString());` it will throw a UnsupportedOperationException,because it has not yet been implemented.We can see the [SqlSyntax#SPECIAL](https://github.com/apache/calcite/blob/bd1fbfe3c062af0879f36ad0e8b53cb0aaec3842/core/src/main/java/org/apache/calcite/sql/SqlSyntax.java#L113) here. There is a case that can be reproduced in [JIRA](https://issues.apache.org/jira/browse/CALCITE-4022), I hope it can be explained clearly.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] xy2953396112 commented on a change in pull request #2014: [CALCITE-4022] Support evaluate SqlInsert SqlBasicCall.toString()
Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on a change in pull request #2014:
URL: https://github.com/apache/calcite/pull/2014#discussion_r455069059
##########
File path: core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java
##########
@@ -5818,6 +5818,21 @@ public void subTestIntervalDayFailsValidation() {
assertTrue(sqlNodeVisited.getKind() == SqlKind.INSERT);
}
+ @Test void testSqlInsertSqlBasicCallToString() throws Exception {
+ final String sql = "insert into emps select * from emps";
+ final SqlNode sqlNode = getSqlParser(sql).parseStmt();
+ final SqlNode sqlNodeVisited = sqlNode.accept(new SqlShuttle() {
+ @Override public SqlNode visit(SqlIdentifier identifier) {
+ return new SqlIdentifier(identifier.names,
+ identifier.getParserPosition());
+ }
+ });
+ final String str = "INSERT INTO `EMPS`\n"
Review comment:
Thanks, update the code.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] yanlin-Lynn merged pull request #2014: [CALCITE-4022] Support unparse special syntax when operator is INSERT
Posted by GitBox <gi...@apache.org>.
yanlin-Lynn merged pull request #2014:
URL: https://github.com/apache/calcite/pull/2014
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] yanlin-Lynn commented on a change in pull request #2014: [CALCITE-4022] Support evaluate SqlInsert SqlBasicCall.toString()
Posted by GitBox <gi...@apache.org>.
yanlin-Lynn commented on a change in pull request #2014:
URL: https://github.com/apache/calcite/pull/2014#discussion_r455463446
##########
File path: core/src/main/java/org/apache/calcite/sql/SqlUtil.java
##########
@@ -261,6 +261,32 @@ public static boolean isLiteralChain(SqlNode node) {
}
}
+ /**
+ * Unparses a call to an operator which has special syntax.
+ *
+ * @param operator The operator
+ * @param writer Writer
+ * @param call List of 0 or more operands
+ */
+ public static void unparseSpecialSyntax(SqlOperator operator,
+ SqlWriter writer,
+ SqlCall call) {
+ final List<SqlNode> operands = call.getOperandList();
+ switch (operator.getKind()) {
+ case INSERT:
+ assert operands.size() == 4;
+ final SqlInsert insert = new SqlInsert(call.getParserPosition(),
+ (SqlNodeList) operands.get(0), operands.get(1), operands.get(2),
+ operands.get(3) == null ? null : (SqlNodeList) operands.get(3));
Review comment:
Just use `(SqlNodeList) operands.get(3)` will be fine
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zinking commented on a change in pull request #2014: [CALCITE-4022] Support evaluate SqlInsert SqlBasicCall.toString()
Posted by GitBox <gi...@apache.org>.
zinking commented on a change in pull request #2014:
URL: https://github.com/apache/calcite/pull/2014#discussion_r448358283
##########
File path: core/src/main/java/org/apache/calcite/sql/SqlUtil.java
##########
@@ -261,6 +261,32 @@ public static boolean isLiteralChain(SqlNode node) {
}
}
+ /**
+ * Unparses a call to an operator which has special syntax.
+ *
+ * @param operator The operator
+ * @param writer Writer
+ * @param call List of 0 or more operands
+ */
+ public static void unparseSpecialSyntax(SqlOperator operator,
Review comment:
this name is really weird.
could you find a way to `inline` your implementation in the original place instead of here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zinking commented on a change in pull request #2014: [CALCITE-4022] Support evaluate SqlInsert SqlBasicCall.toString()
Posted by GitBox <gi...@apache.org>.
zinking commented on a change in pull request #2014:
URL: https://github.com/apache/calcite/pull/2014#discussion_r448360495
##########
File path: core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java
##########
@@ -5818,6 +5818,21 @@ public void subTestIntervalDayFailsValidation() {
assertTrue(sqlNodeVisited.getKind() == SqlKind.INSERT);
}
+ @Test void testSqlInsertSqlBasicCallToString() throws Exception {
+ final String sql = "insert into emps select * from emps";
+ final SqlNode sqlNode = getSqlParser(sql).parseStmt();
+ final SqlNode sqlNodeVisited = sqlNode.accept(new SqlShuttle() {
+ @Override public SqlNode visit(SqlIdentifier identifier) {
+ return new SqlIdentifier(identifier.names,
+ identifier.getParserPosition());
+ }
+ });
+ final String str = "INSERT INTO `EMPS`\n"
Review comment:
what about one more test regarding the actual execution?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zinking commented on pull request #2014: [CALCITE-4022] Support evaluate SqlInsert SqlBasicCall.toString()
Posted by GitBox <gi...@apache.org>.
zinking commented on pull request #2014:
URL: https://github.com/apache/calcite/pull/2014#issuecomment-652388128
I guess you need to clarify the issue first in JIRA.
implementation looks weird before that.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] yanlin-Lynn commented on pull request #2014: [CALCITE-4022] Support evaluate SqlInsert SqlBasicCall.toString()
Posted by GitBox <gi...@apache.org>.
yanlin-Lynn commented on pull request #2014:
URL: https://github.com/apache/calcite/pull/2014#issuecomment-659988542
The commit log is confusing, can you make it more clearer, like "Support unparse special syntax when operator is INSERT"
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zinking commented on a change in pull request #2014: [CALCITE-4022] Support evaluate SqlInsert SqlBasicCall.toString()
Posted by GitBox <gi...@apache.org>.
zinking commented on a change in pull request #2014:
URL: https://github.com/apache/calcite/pull/2014#discussion_r448358925
##########
File path: core/src/main/java/org/apache/calcite/sql/SqlUtil.java
##########
@@ -261,6 +261,32 @@ public static boolean isLiteralChain(SqlNode node) {
}
}
+ /**
+ * Unparses a call to an operator which has special syntax.
+ *
+ * @param operator The operator
+ * @param writer Writer
+ * @param call List of 0 or more operands
+ */
+ public static void unparseSpecialSyntax(SqlOperator operator,
+ SqlWriter writer,
+ SqlCall call) {
+ final List<SqlNode> operands = call.getOperandList();
+ switch (operator.getKind()) {
+ case INSERT:
+ assert operands.size() == 3 || operands.size() == 4;
+ final SqlInsert insert = new SqlInsert(call.getParserPosition(),
+ (SqlNodeList) operands.get(0), operands.get(1), operands.get(2),
+ operands.size() == 3 ? null : (SqlNodeList) call.getOperandList().get(3));
Review comment:
why `null` and why not reusing `operands` in this line ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] xy2953396112 commented on pull request #2014: [CALCITE-4022] Support unparse special syntax when operator is INSERT
Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on pull request #2014:
URL: https://github.com/apache/calcite/pull/2014#issuecomment-661558313
> +1
Thanks.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] yanlin-Lynn commented on a change in pull request #2014: [CALCITE-4022] Support evaluate SqlInsert SqlBasicCall.toString()
Posted by GitBox <gi...@apache.org>.
yanlin-Lynn commented on a change in pull request #2014:
URL: https://github.com/apache/calcite/pull/2014#discussion_r453259063
##########
File path: core/src/main/java/org/apache/calcite/sql/SqlUtil.java
##########
@@ -261,6 +261,32 @@ public static boolean isLiteralChain(SqlNode node) {
}
}
+ /**
+ * Unparses a call to an operator which has special syntax.
+ *
+ * @param operator The operator
+ * @param writer Writer
+ * @param call List of 0 or more operands
+ */
+ public static void unparseSpecialSyntax(SqlOperator operator,
+ SqlWriter writer,
+ SqlCall call) {
+ final List<SqlNode> operands = call.getOperandList();
+ switch (operator.getKind()) {
+ case INSERT:
+ assert operands.size() == 3 || operands.size() == 4;
+ final SqlInsert insert = new SqlInsert(call.getParserPosition(),
+ (SqlNodeList) operands.get(0), operands.get(1), operands.get(2),
+ operands.size() == 3 ? null : (SqlNodeList) call.getOperandList().get(3));
Review comment:
what's the cases for `operands.size() == 3` and `operands.size() == 4`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] xy2953396112 commented on a change in pull request #2014: [CALCITE-4022] Support evaluate SqlInsert SqlBasicCall.toString()
Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on a change in pull request #2014:
URL: https://github.com/apache/calcite/pull/2014#discussion_r455528677
##########
File path: core/src/main/java/org/apache/calcite/sql/SqlUtil.java
##########
@@ -261,6 +261,32 @@ public static boolean isLiteralChain(SqlNode node) {
}
}
+ /**
+ * Unparses a call to an operator which has special syntax.
+ *
+ * @param operator The operator
+ * @param writer Writer
+ * @param call List of 0 or more operands
+ */
+ public static void unparseSpecialSyntax(SqlOperator operator,
+ SqlWriter writer,
+ SqlCall call) {
+ final List<SqlNode> operands = call.getOperandList();
+ switch (operator.getKind()) {
+ case INSERT:
+ assert operands.size() == 4;
+ final SqlInsert insert = new SqlInsert(call.getParserPosition(),
+ (SqlNodeList) operands.get(0), operands.get(1), operands.get(2),
+ operands.get(3) == null ? null : (SqlNodeList) operands.get(3));
Review comment:
ok, Thanks a lot.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org