[GitHub] [flink] godfreyhe commented on a change in pull request #12431: [FLINK-18055] [sql-client] Fix catalog/database does not exist in sql client
godfreyhe commented on a change in pull request #12431:
URL: https://github.com/apache/flink/pull/12431#discussion_r434322529
##
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/SqlCommandParser.java
##
@@ -132,11 +132,10 @@ private SqlCommandParser() {
cmd = SqlCommand.DROP_CATALOG;
} else if (operation instanceof UseCatalogOperation) {
cmd = SqlCommand.USE_CATALOG;
- operands = new String[] { String.format("`%s`",
((UseCatalogOperation) operation).getCatalogName()) };
+ operands = new String[] { ((UseCatalogOperation)
operation).getCatalogName() };
} else if (operation instanceof UseDatabaseOperation) {
cmd = SqlCommand.USE;
- UseDatabaseOperation op = ((UseDatabaseOperation)
operation);
- operands = new String[] { String.format("`%s`.`%s`",
op.getCatalogName(), op.getDatabaseName()) };
+ operands = new String[] { ((UseDatabaseOperation)
operation).getDatabaseName() };
Review comment:
catalog name is unnecessary, only database is needed.
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:
us...@infra.apache.org
[GitHub] [flink] godfreyhe commented on a change in pull request #12431: [FLINK-18055] [sql-client] Fix catalog/database does not exist in sql client
godfreyhe commented on a change in pull request #12431:
URL: https://github.com/apache/flink/pull/12431#discussion_r434321402
##
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/SqlCommandParser.java
##
@@ -132,11 +132,10 @@ private SqlCommandParser() {
cmd = SqlCommand.DROP_CATALOG;
} else if (operation instanceof UseCatalogOperation) {
cmd = SqlCommand.USE_CATALOG;
- operands = new String[] { String.format("`%s`",
((UseCatalogOperation) operation).getCatalogName()) };
+ operands = new String[] { ((UseCatalogOperation)
operation).getCatalogName() };
Review comment:
The quoting string ("\`") is skipped by sql parser, so we need not to
handle this. see the test (TestItem.validSql("use \`default\`;",
SqlCommand.USE, "default")) in `SqlCommandParserTest#testCommands`
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:
us...@infra.apache.org
[GitHub] [flink] godfreyhe commented on a change in pull request #12431: [FLINK-18055] [sql-client] Fix catalog/database does not exist in sql client
godfreyhe commented on a change in pull request #12431:
URL: https://github.com/apache/flink/pull/12431#discussion_r434321402
##
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/SqlCommandParser.java
##
@@ -132,11 +132,10 @@ private SqlCommandParser() {
cmd = SqlCommand.DROP_CATALOG;
} else if (operation instanceof UseCatalogOperation) {
cmd = SqlCommand.USE_CATALOG;
- operands = new String[] { String.format("`%s`",
((UseCatalogOperation) operation).getCatalogName()) };
+ operands = new String[] { ((UseCatalogOperation)
operation).getCatalogName() };
Review comment:
The quoting string \` is skipped by sql parser, so we need not to
handle this. see the test (`TestItem.validSql("use `default`;", SqlCommand.USE,
"default")`) in `SqlCommandParserTest#testCommands`
##
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/SqlCommandParser.java
##
@@ -132,11 +132,10 @@ private SqlCommandParser() {
cmd = SqlCommand.DROP_CATALOG;
} else if (operation instanceof UseCatalogOperation) {
cmd = SqlCommand.USE_CATALOG;
- operands = new String[] { String.format("`%s`",
((UseCatalogOperation) operation).getCatalogName()) };
+ operands = new String[] { ((UseCatalogOperation)
operation).getCatalogName() };
Review comment:
The quoting string ("\`") is skipped by sql parser, so we need not to
handle this. see the test (`TestItem.validSql("use `default`;", SqlCommand.USE,
"default")`) in `SqlCommandParserTest#testCommands`
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:
us...@infra.apache.org
[GitHub] [flink] godfreyhe commented on a change in pull request #12431: [FLINK-18055] [sql-client] Fix catalog/database does not exist in sql client
godfreyhe commented on a change in pull request #12431:
URL: https://github.com/apache/flink/pull/12431#discussion_r434321402
##
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/SqlCommandParser.java
##
@@ -132,11 +132,10 @@ private SqlCommandParser() {
cmd = SqlCommand.DROP_CATALOG;
} else if (operation instanceof UseCatalogOperation) {
cmd = SqlCommand.USE_CATALOG;
- operands = new String[] { String.format("`%s`",
((UseCatalogOperation) operation).getCatalogName()) };
+ operands = new String[] { ((UseCatalogOperation)
operation).getCatalogName() };
Review comment:
The quoting string ("\`") is skipped by sql parser, so we need not to
handle this. see the test (`TestItem.validSql("use \`default\`;",
SqlCommand.USE, "default")`) in `SqlCommandParserTest#testCommands`
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:
us...@infra.apache.org
[GitHub] [flink] godfreyhe commented on a change in pull request #12431: [FLINK-18055] [sql-client] Fix catalog/database does not exist in sql client
godfreyhe commented on a change in pull request #12431:
URL: https://github.com/apache/flink/pull/12431#discussion_r434321402
##
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/SqlCommandParser.java
##
@@ -132,11 +132,10 @@ private SqlCommandParser() {
cmd = SqlCommand.DROP_CATALOG;
} else if (operation instanceof UseCatalogOperation) {
cmd = SqlCommand.USE_CATALOG;
- operands = new String[] { String.format("`%s`",
((UseCatalogOperation) operation).getCatalogName()) };
+ operands = new String[] { ((UseCatalogOperation)
operation).getCatalogName() };
Review comment:
The quoting string ` is skipped by sql parser, so we need not to handle
this. see the test (`TestItem.validSql("use `default`;", SqlCommand.USE,
"default")`) in `SqlCommandParserTest#testCommands`
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:
us...@infra.apache.org
[GitHub] [flink] godfreyhe commented on a change in pull request #12431: [FLINK-18055] [sql-client] Fix catalog/database does not exist in sql client
godfreyhe commented on a change in pull request #12431:
URL: https://github.com/apache/flink/pull/12431#discussion_r434321402
##
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/SqlCommandParser.java
##
@@ -132,11 +132,10 @@ private SqlCommandParser() {
cmd = SqlCommand.DROP_CATALOG;
} else if (operation instanceof UseCatalogOperation) {
cmd = SqlCommand.USE_CATALOG;
- operands = new String[] { String.format("`%s`",
((UseCatalogOperation) operation).getCatalogName()) };
+ operands = new String[] { ((UseCatalogOperation)
operation).getCatalogName() };
Review comment:
The quoting string ` \` ` is skipped by sql parser, so we need not to
handle this. see the test (`TestItem.validSql("use `default`;", SqlCommand.USE,
"default")`) in `SqlCommandParserTest#testCommands`
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:
us...@infra.apache.org
[GitHub] [flink] godfreyhe commented on a change in pull request #12431: [FLINK-18055] [sql-client] Fix catalog/database does not exist in sql client
godfreyhe commented on a change in pull request #12431:
URL: https://github.com/apache/flink/pull/12431#discussion_r434321402
##
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/SqlCommandParser.java
##
@@ -132,11 +132,10 @@ private SqlCommandParser() {
cmd = SqlCommand.DROP_CATALOG;
} else if (operation instanceof UseCatalogOperation) {
cmd = SqlCommand.USE_CATALOG;
- operands = new String[] { String.format("`%s`",
((UseCatalogOperation) operation).getCatalogName()) };
+ operands = new String[] { ((UseCatalogOperation)
operation).getCatalogName() };
Review comment:
The quoting string ` "`" ` is skipped by sql parser, so we need not to
handle this. see the test (`TestItem.validSql("use `default`;", SqlCommand.USE,
"default")`) in `SqlCommandParserTest#testCommands`
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:
us...@infra.apache.org
[GitHub] [flink] godfreyhe commented on a change in pull request #12431: [FLINK-18055] [sql-client] Fix catalog/database does not exist in sql client
godfreyhe commented on a change in pull request #12431:
URL: https://github.com/apache/flink/pull/12431#discussion_r434321402
##
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/SqlCommandParser.java
##
@@ -132,11 +132,10 @@ private SqlCommandParser() {
cmd = SqlCommand.DROP_CATALOG;
} else if (operation instanceof UseCatalogOperation) {
cmd = SqlCommand.USE_CATALOG;
- operands = new String[] { String.format("`%s`",
((UseCatalogOperation) operation).getCatalogName()) };
+ operands = new String[] { ((UseCatalogOperation)
operation).getCatalogName() };
Review comment:
The quoting string ``` is skipped by sql parser, so we need not to
handle this. see the test (`TestItem.validSql("use `default`;", SqlCommand.USE,
"default")`) in `SqlCommandParserTest#testCommands`
##
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/SqlCommandParser.java
##
@@ -132,11 +132,10 @@ private SqlCommandParser() {
cmd = SqlCommand.DROP_CATALOG;
} else if (operation instanceof UseCatalogOperation) {
cmd = SqlCommand.USE_CATALOG;
- operands = new String[] { String.format("`%s`",
((UseCatalogOperation) operation).getCatalogName()) };
+ operands = new String[] { ((UseCatalogOperation)
operation).getCatalogName() };
Review comment:
The quoting string ` ` ` is skipped by sql parser, so we need not to
handle this. see the test (`TestItem.validSql("use `default`;", SqlCommand.USE,
"default")`) in `SqlCommandParserTest#testCommands`
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:
us...@infra.apache.org
[GitHub] [flink] godfreyhe commented on a change in pull request #12431: [FLINK-18055] [sql-client] Fix catalog/database does not exist in sql client
godfreyhe commented on a change in pull request #12431:
URL: https://github.com/apache/flink/pull/12431#discussion_r434317689
##
File path:
flink-table/flink-sql-client/src/test/java/org/apache/flink/table/client/cli/CliClientTest.java
##
@@ -166,6 +167,64 @@ public void write(int b) throws IOException {
}
}
+ @Test
+ public void testUseCatalog() throws Exception {
+ TestingExecutor executor = new TestingExecutorBuilder()
+ .setUseCatalogConsumer((ignored1, catalogName)
-> {
+ if (!catalogName.equals("cat")) {
+ throw new
SqlExecutionException("unexpected catalog name: " + catalogName);
+ }
+ })
+ .build();
+
+ InputStream inputStream = new ByteArrayInputStream("use catalog
cat;\n".getBytes());
+ ByteArrayOutputStream outputStream = new
ByteArrayOutputStream(256);
+ CliClient cliClient = null;
+ SessionContext sessionContext = new
SessionContext("test-session", new Environment());
+ String sessionId = executor.openSession(sessionContext);
+
+ try (Terminal terminal = new DumbTerminal(inputStream,
outputStream)) {
Review comment:
`CliClient` does not inherit from `AutoCloseable`, so we can put
`CliClient` into `try-with-resource` block
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:
us...@infra.apache.org
