KurtYoung commented on a change in pull request #15437:
URL: https://github.com/apache/flink/pull/15437#discussion_r604728266
##########
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/SqlClient.java
##########
@@ -123,13 +123,22 @@ private void openCli(String sessionId, Executor executor)
{
CliOptionsParser.OPTION_FILE.getOpt()));
}
- boolean isInteractiveMode = !hasSqlFile && !hasUpdateStatement;
-
try (CliClient cli = new CliClient(sessionId, executor,
historyFilePath)) {
- if (isInteractiveMode) {
- cli.open();
+ if (options.getInitFile() != null) {
+ boolean success =
cli.executeInitialization(readFromURL(options.getInitFile()));
+ if (!success) {
+ System.out.println(
+ "Fail to execute init file... Please refer to the
LOG for detailed error messages.");
+ return;
+ } else {
+ System.out.println("Succeed to execute init file. Enter
execution phase...");
Review comment:
rephrase to "Successfully initialized from sql script: " +
options.getInitFile()
##########
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/CliClient.java
##########
@@ -120,8 +125,8 @@
* using {@link #close()}.
*/
@VisibleForTesting
- public CliClient(
- Terminal terminal,
+ CliClient(
+ @Nullable Terminal terminal,
Review comment:
unnecessary to pass a nullable terminal here.
The class field `this.terminal` will naturally be NULL
##########
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/SqlClient.java
##########
@@ -123,13 +123,22 @@ private void openCli(String sessionId, Executor executor)
{
CliOptionsParser.OPTION_FILE.getOpt()));
}
- boolean isInteractiveMode = !hasSqlFile && !hasUpdateStatement;
-
try (CliClient cli = new CliClient(sessionId, executor,
historyFilePath)) {
- if (isInteractiveMode) {
- cli.open();
+ if (options.getInitFile() != null) {
+ boolean success =
cli.executeInitialization(readFromURL(options.getInitFile()));
+ if (!success) {
+ System.out.println(
+ "Fail to execute init file... Please refer to the
LOG for detailed error messages.");
Review comment:
"Failed to initialize from sql script: " + options.getInitFile()
Please refer to the LOG for detailed error messages.
##########
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/CliClient.java
##########
@@ -252,48 +279,30 @@ public void open() {
continue;
}
- executeStatement(line, ExecutionMode.INTERACTIVE);
- }
- }
-
- /** Closes the CLI instance. */
- public void close() {
- try {
- terminal.close();
- } catch (IOException e) {
- throw new SqlClientException("Unable to close terminal.", e);
+ executeStatement(line, ExecutionMode.INTERACTIVE_EXECUTION);
}
}
/**
- * Submits content from Sql file and prints status information and/or
errors on the terminal.
+ * Execute content from Sql file and prints status information and/or
errors on the terminal.
*
* @param content SQL file content
*/
- public void executeSqlFile(String content) {
+ private boolean executeFile(String content, ExecutionMode mode) {
terminal.writer().println(CliStrings.messageInfo(CliStrings.MESSAGE_WILL_EXECUTE).toAnsi());
Review comment:
modify the content of MESSAGE_WILL_EXECUTE to "Executing SQL from file"
##########
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/CliClient.java
##########
@@ -306,17 +315,38 @@ private boolean executeStatement(String statement,
ExecutionMode executionMode)
}
private void validate(Operation operation, ExecutionMode executionMode) {
- ResultMode mode =
executor.getSessionConfig(sessionId).get(EXECUTION_RESULT_MODE);
- if (operation instanceof QueryOperation
- && executionMode.equals(ExecutionMode.NON_INTERACTIVE)
- && !mode.equals(TABLEAU)) {
- throw new IllegalArgumentException(
- String.format(
- "In non-interactive mode, it only supports to use
%s as value of %s when execute query. Please add 'SET %s=%s;' in the sql file.",
- TABLEAU,
- EXECUTION_RESULT_MODE.key(),
- EXECUTION_RESULT_MODE.key(),
- TABLEAU));
+ if (executionMode.equals(ExecutionMode.INITIALIZATION)) {
+ if (!(operation instanceof SetOperation)
+ && !(operation instanceof ResetOperation)
+ && !(operation instanceof CreateOperation)
+ && !(operation instanceof DropOperation)
+ && !(operation instanceof UseOperation)
+ && !(operation instanceof AlterOperation)) {
+ throw new SqlExecutionException(
+ "In init sql file, it only supports to use DDL
operation and SET/RESET operation. "
Review comment:
"Unsupported operation in sql init file: " + operation.asSummaryString()
##########
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/CliClient.java
##########
@@ -97,9 +102,9 @@
private final String sessionId;
- private final Terminal terminal;
+ private @Nullable Terminal terminal;
- private final LineReader lineReader;
+ private Path historyFilePath;
Review comment:
final
##########
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/CliClient.java
##########
@@ -97,9 +102,9 @@
private final String sessionId;
- private final Terminal terminal;
+ private @Nullable Terminal terminal;
Review comment:
unnecessary Nullable
you can also re-organize the class fields, move this one below to all the
final ones
##########
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/CliClient.java
##########
@@ -306,17 +315,38 @@ private boolean executeStatement(String statement,
ExecutionMode executionMode)
}
private void validate(Operation operation, ExecutionMode executionMode) {
- ResultMode mode =
executor.getSessionConfig(sessionId).get(EXECUTION_RESULT_MODE);
- if (operation instanceof QueryOperation
- && executionMode.equals(ExecutionMode.NON_INTERACTIVE)
- && !mode.equals(TABLEAU)) {
- throw new IllegalArgumentException(
- String.format(
- "In non-interactive mode, it only supports to use
%s as value of %s when execute query. Please add 'SET %s=%s;' in the sql file.",
- TABLEAU,
- EXECUTION_RESULT_MODE.key(),
- EXECUTION_RESULT_MODE.key(),
- TABLEAU));
+ if (executionMode.equals(ExecutionMode.INITIALIZATION)) {
+ if (!(operation instanceof SetOperation)
+ && !(operation instanceof ResetOperation)
+ && !(operation instanceof CreateOperation)
+ && !(operation instanceof DropOperation)
+ && !(operation instanceof UseOperation)
+ && !(operation instanceof AlterOperation)) {
Review comment:
also `LoadModuleOperation` and `UnloadModuleOperation`
##########
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/CliClient.java
##########
@@ -252,48 +279,30 @@ public void open() {
continue;
}
- executeStatement(line, ExecutionMode.INTERACTIVE);
- }
- }
-
- /** Closes the CLI instance. */
- public void close() {
- try {
- terminal.close();
- } catch (IOException e) {
- throw new SqlClientException("Unable to close terminal.", e);
+ executeStatement(line, ExecutionMode.INTERACTIVE_EXECUTION);
}
}
/**
- * Submits content from Sql file and prints status information and/or
errors on the terminal.
+ * Execute content from Sql file and prints status information and/or
errors on the terminal.
*
* @param content SQL file content
*/
- public void executeSqlFile(String content) {
+ private boolean executeFile(String content, ExecutionMode mode) {
terminal.writer().println(CliStrings.messageInfo(CliStrings.MESSAGE_WILL_EXECUTE).toAnsi());
Review comment:
also change the variable name to `MESSAGE_EXECUTE_FILE`
--
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:
[email protected]
