[FLINK-20320][sql-client] support init sql file in sql client #15437

fsk119 · 2021-03-30T16:39:22Z

What is the purpose of the change

Support to start the sql client with -i init file.

If the execution get error, the sql client will stop and exit. But currently we print the error message to the LOG not console. The idea is from the HIVE.

Brief change log

Add options
Add init logic before enter execution mode
Use client to init session.

Verifying this change

This change added tests and can be verified as follows:

END TO END test in the SqlClientTest
Add a test that init file contains insert into

Does this pull request potentially affect one of the following parts:

Dependencies (does it add or upgrade a dependency): (yes / no)
The public API, i.e., is any changed class annotated with @Public(Evolving): (yes / no)
The serializers: (yes / no / don't know)
The runtime per-record code paths (performance sensitive): (yes / no / don't know)
Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn/Mesos, ZooKeeper: (yes / no / don't know)
The S3 file system connector: (yes / no / don't know)

Documentation

Does this pull request introduce a new feature? (yes / no)
If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

flinkbot · 2021-03-30T16:49:04Z

Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.

Automated Checks

Last check on commit 3ff81e4 (Sat Aug 28 11:12:45 UTC 2021)

Warnings:

No documentation files were touched! Remember to keep the Flink docs up to date!

_{Mention the bot in a comment to re-run the automated checks.}

Review Progress

❓ 1. The [description] looks good.
❓ 2. There is [consensus] that the contribution should go into to Flink.
❓ 3. Needs [attention] from.
❓ 4. The change fits into the overall [architecture].
❓ 5. Overall code [quality] is good.

Please see the Pull Request Review Guide for a full explanation of the review process.

The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot approve description to approve one or more aspects (aspects: description, consensus, architecture and quality)
@flinkbot approve all to approve all aspects
@flinkbot approve-until architecture to approve everything until architecture
@flinkbot attention @username1 [@username2 ..] to require somebody's attention
@flinkbot disapprove architecture to remove an approval you gave earlier

flinkbot · 2021-03-30T17:11:10Z

CI report:

71c1edd Azure: FAILURE
3ff81e4 Azure: PENDING

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot run travis re-run the last Travis build
@flinkbot run azure re-run the last Azure build

flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/SqlClient.java

flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/CliClient.java

KurtYoung · 2021-03-31T02:14:00Z

flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/SqlClient.java

@@ -124,6 +125,22 @@ private void openCli(String sessionId, Executor executor) {
        }

        boolean isInteractiveMode = !hasSqlFile && !hasUpdateStatement;
+        if (hasInitFile) {
+            boolean success =
+                    CliClient.initializeSession(


Why init client here instead of after you created the real cli client?

flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/SqlClient.java

KurtYoung · 2021-03-31T09:16:54Z

flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/CliClient.java

-    public CliClient(
-            Terminal terminal,
+    CliClient(
+            @Nullable Terminal terminal,


unnecessary to pass a nullable terminal here.
The class field this.terminal will naturally be NULL

flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/CliClient.java

KurtYoung · 2021-03-31T09:31:26Z

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;


unnecessary Nullable
you can also re-organize the class fields, move this one below to all the final ones

flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/CliClient.java

KurtYoung

Some final comments.

flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/CliClient.java

flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/SqlClient.java

flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/CliClient.java

KurtYoung

LGTM, verified locally and the timeout issue seems fixed.

rmetzger added the review=description? label Mar 30, 2021

rmetzger added the component=TableSQL/Client label Mar 30, 2021

KurtYoung reviewed Mar 31, 2021

View reviewed changes

fsk119 added 2 commits March 31, 2021 10:36

[FLINK-20320][sql-client] support init sql file in sql client

12c1985

[fix] address Kurt's comments

eab3cfc

fsk119 force-pushed the init-sql-file branch from 5953b35 to eab3cfc Compare March 31, 2021 03:25

fsk119 added 2 commits March 31, 2021 15:10

[fix] address Kurt's comments

f4b9d4a

[fix] fix broken test

2665e42

KurtYoung reviewed Mar 31, 2021

View reviewed changes

[fix] address Kurt's comments

1d0a5c2

KurtYoung reviewed Mar 31, 2021

View reviewed changes

fsk119 added 2 commits March 31, 2021 22:02

[fix] address Kurt's comments

71c1edd

[fix] rm use of TerminalStreamsResource in CliClientTest/ITCase

3ff81e4

KurtYoung approved these changes Apr 1, 2021

View reviewed changes

KurtYoung closed this in c36bce8 Apr 1, 2021

fsk119 deleted the init-sql-file branch September 26, 2022 02:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FLINK-20320][sql-client] support init sql file in sql client #15437

[FLINK-20320][sql-client] support init sql file in sql client #15437

fsk119 commented Mar 30, 2021

flinkbot commented Mar 30, 2021 •

edited

Loading

flinkbot commented Mar 30, 2021 •

edited

Loading

KurtYoung Mar 31, 2021

KurtYoung Mar 31, 2021

KurtYoung Mar 31, 2021

KurtYoung left a comment

KurtYoung left a comment

[FLINK-20320][sql-client] support init sql file in sql client #15437

[FLINK-20320][sql-client] support init sql file in sql client #15437

Conversation

fsk119 commented Mar 30, 2021

What is the purpose of the change

Brief change log

Verifying this change

Does this pull request potentially affect one of the following parts:

Documentation

flinkbot commented Mar 30, 2021 • edited Loading

Automated Checks

Review Progress

flinkbot commented Mar 30, 2021 • edited Loading

CI report:

KurtYoung Mar 31, 2021

Choose a reason for hiding this comment

KurtYoung Mar 31, 2021

Choose a reason for hiding this comment

KurtYoung Mar 31, 2021

Choose a reason for hiding this comment

KurtYoung left a comment

Choose a reason for hiding this comment

KurtYoung left a comment

Choose a reason for hiding this comment

flinkbot commented Mar 30, 2021 •

edited

Loading

flinkbot commented Mar 30, 2021 •

edited

Loading