[KYUUBI #5507][FLINK] Support Initialize SQL in Flink Engine #5518

hadoopkandy · 2023-10-25T08:13:58Z

Why are the changes needed?

We shoud support initialize SQL to init session context the in Flink SQL (e.g. setting up catalogs).

How was this patch tested?

Add some test cases that check the changes thoroughly including negative and positive cases if possible
Add screenshots for manual tests if appropriate
Run test locally before make a pull request

Was this patch authored or co-authored using generative AI tooling?

No.

link3280

Thanks a lot for the PR. The code generally looks good, but the tests could be improved a bit. Please take a look at my comments.

...ine/src/test/scala/org/apache/kyuubi/engine/flink/operation/FlinkEngineInitializeSuite.scala

...link-sql-engine/src/main/scala/org/apache/kyuubi/engine/flink/session/FlinkSessionImpl.scala

codecov-commenter · 2023-11-01T03:37:12Z

Codecov Report

Merging #5518 (0782738) into master (27d845c) will decrease coverage by 0.24%.
Report is 9 commits behind head on master.
The diff coverage is n/a.

❗ Current head 0782738 differs from pull request most recent head b1720ec. Consider uploading reports for the commit b1720ec to get more accurate results

@@             Coverage Diff              @@
##             master    #5518      +/-   ##
============================================
- Coverage     61.67%   61.44%   -0.24%     
  Complexity       23       23              
============================================
  Files           603      603              
  Lines         35670    35669       -1     
  Branches       4869     4869              
============================================
- Hits          21999    21916      -83     
- Misses        11291    11376      +85     
+ Partials       2380     2377       -3

see 23 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

...ine/src/test/scala/org/apache/kyuubi/engine/flink/operation/FlinkEngineInitializeSuite.scala

...s/kyuubi-flink-sql-engine/src/main/scala/org/apache/kyuubi/engine/flink/FlinkSQLEngine.scala

pan3793 · 2023-11-02T04:17:44Z

...flink-sql-engine/src/test/scala/org/apache/kyuubi/engine/flink/WithFlinkSQLEngineLocal.scala

      }
    }
    withKyuubiConf.foreach { case (k, v) =>
-      System.setProperty(k, v)


what's the background of this change?

It's not relevant. It's probably a cleanup of unused codes.

pan3793

Overall LGTM, just left a few comments

…uubi/engine/flink/FlinkSQLEngine.scala Co-authored-by: Cheng Pan <pan3793@gmail.com>

...ine/src/test/scala/org/apache/kyuubi/engine/flink/operation/FlinkEngineInitializeSuite.scala

…uubi/engine/flink/operation/FlinkEngineInitializeSuite.scala

pan3793 · 2023-11-02T10:19:53Z

Thanks, merged to master

### _Why are the changes needed?_ We shoud support initialize SQL to init session context the in Flink SQL (e.g. setting up catalogs). ### _How was this patch tested?_ - [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [ ] Add screenshots for manual tests if appropriate - [ ] [Run test](https://kyuubi.readthedocs.io/en/master/contributing/code/testing.html#running-tests) locally before make a pull request ### _Was this patch authored or co-authored using generative AI tooling?_ No. Closes apache#5518 from hadoopkandy/KYUUBI-5507. Closes apache#5507 b1720ec [Cheng Pan] Update externals/kyuubi-flink-sql-engine/src/test/scala/org/apache/kyuubi/engine/flink/operation/FlinkEngineInitializeSuite.scala 0782738 [Paul Lin] [KYUUBI-5507] Improve codestyle 13035f3 [Paul Lin] Update externals/kyuubi-flink-sql-engine/src/main/scala/org/apache/kyuubi/engine/flink/FlinkSQLEngine.scala 2ce8031 [Paul Lin] [KYUUBI-5507] Improve tests a29ac38 [Paul Lin] [KYUUBI-5507] Run engine initial SQL at Engine start b864af5 [wangkang] Merge branch 'apache:master' into KYUUBI-5507 b37d62d [kandy01.wang] [KYUUBI apache#5507] [FLINK] Support Initialize SQL in Flink Engine Lead-authored-by: Kang Wang <kandy01.wang@vipshop.com> Co-authored-by: Paul Lin <paullin3280@gmail.com> Co-authored-by: kandy01.wang <kandy01.wang@vipshop.com> Co-authored-by: Cheng Pan <pan3793@gmail.com> Co-authored-by: wangkang <525262800@qq.com> Signed-off-by: Cheng Pan <chengpan@apache.org>

pan3793 · 2023-11-24T08:10:50Z

I was asked frequently these days how to initialize the catalog in the Flink engine.

@link3280, if you don't mind, I propose to backport this patch to 1.8.1 so that we can deliver it soon.

link3280 · 2023-11-24T08:24:53Z

@pan3793 Sure. I'll create a backport PR in days.

pan3793 · 2023-11-24T08:36:00Z

@link3280 I cherry-pick it to branch-1.8 in 902815d

### _Why are the changes needed?_ We shoud support initialize SQL to init session context the in Flink SQL (e.g. setting up catalogs). ### _How was this patch tested?_ - [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [ ] Add screenshots for manual tests if appropriate - [ ] [Run test](https://kyuubi.readthedocs.io/en/master/contributing/code/testing.html#running-tests) locally before make a pull request ### _Was this patch authored or co-authored using generative AI tooling?_ No. Closes #5518 from hadoopkandy/KYUUBI-5507. Closes #5507 b1720ec [Cheng Pan] Update externals/kyuubi-flink-sql-engine/src/test/scala/org/apache/kyuubi/engine/flink/operation/FlinkEngineInitializeSuite.scala 0782738 [Paul Lin] [KYUUBI-5507] Improve codestyle 13035f3 [Paul Lin] Update externals/kyuubi-flink-sql-engine/src/main/scala/org/apache/kyuubi/engine/flink/FlinkSQLEngine.scala 2ce8031 [Paul Lin] [KYUUBI-5507] Improve tests a29ac38 [Paul Lin] [KYUUBI-5507] Run engine initial SQL at Engine start b864af5 [wangkang] Merge branch 'apache:master' into KYUUBI-5507 b37d62d [kandy01.wang] [KYUUBI #5507] [FLINK] Support Initialize SQL in Flink Engine Lead-authored-by: Kang Wang <kandy01.wang@vipshop.com> Co-authored-by: Paul Lin <paullin3280@gmail.com> Co-authored-by: kandy01.wang <kandy01.wang@vipshop.com> Co-authored-by: Cheng Pan <pan3793@gmail.com> Co-authored-by: wangkang <525262800@qq.com> Signed-off-by: Cheng Pan <chengpan@apache.org>

github-actions bot added the module:flink label Oct 25, 2023

link3280 assigned hadoopkandy Oct 25, 2023

link3280 added this to the v1.9.0 milestone Oct 25, 2023

link3280 requested changes Oct 25, 2023

View reviewed changes

...ine/src/test/scala/org/apache/kyuubi/engine/flink/operation/FlinkEngineInitializeSuite.scala Outdated Show resolved Hide resolved

...ine/src/test/scala/org/apache/kyuubi/engine/flink/operation/FlinkEngineInitializeSuite.scala Outdated Show resolved Hide resolved

link3280 requested a review from pan3793 October 25, 2023 11:44

hadoopkandy force-pushed the KYUUBI-5507 branch from f55e4fa to ee53a94 Compare October 25, 2023 13:21

cxzl25 changed the title ~~[KYUUBI #5507] [FLINK] Support Initialize SQL in Flink Engine~~ [KYUUBI #5507][FLINK] Support Initialize SQL in Flink Engine Oct 26, 2023

pan3793 reviewed Oct 26, 2023

View reviewed changes

...link-sql-engine/src/main/scala/org/apache/kyuubi/engine/flink/session/FlinkSessionImpl.scala Outdated Show resolved Hide resolved

hadoopkandy force-pushed the KYUUBI-5507 branch from ee53a94 to 20ce217 Compare November 1, 2023 01:50

[KYUUBI apache#5507] [FLINK] Support Initialize SQL in Flink Engine

b37d62d

hadoopkandy force-pushed the KYUUBI-5507 branch from 522a60f to b37d62d Compare November 1, 2023 12:03

hadoopkandy and others added 3 commits November 1, 2023 20:13

Merge branch 'apache:master' into KYUUBI-5507

b864af5

[KYUUBI-5507] Run engine initial SQL at Engine start

a29ac38

[KYUUBI-5507] Improve tests

2ce8031

link3280 approved these changes Nov 1, 2023

View reviewed changes

pan3793 reviewed Nov 2, 2023

View reviewed changes

...ine/src/test/scala/org/apache/kyuubi/engine/flink/operation/FlinkEngineInitializeSuite.scala Outdated Show resolved Hide resolved

pan3793 reviewed Nov 2, 2023

View reviewed changes

...s/kyuubi-flink-sql-engine/src/main/scala/org/apache/kyuubi/engine/flink/FlinkSQLEngine.scala Outdated Show resolved Hide resolved

pan3793 reviewed Nov 2, 2023

View reviewed changes

pan3793 approved these changes Nov 2, 2023

View reviewed changes

link3280 and others added 2 commits November 2, 2023 12:43

Update externals/kyuubi-flink-sql-engine/src/main/scala/org/apache/ky…

13035f3

…uubi/engine/flink/FlinkSQLEngine.scala Co-authored-by: Cheng Pan <pan3793@gmail.com>

[KYUUBI-5507] Improve codestyle

0782738

pan3793 reviewed Nov 2, 2023

View reviewed changes

...ine/src/test/scala/org/apache/kyuubi/engine/flink/operation/FlinkEngineInitializeSuite.scala Outdated Show resolved Hide resolved

Update externals/kyuubi-flink-sql-engine/src/test/scala/org/apache/ky…

b1720ec

…uubi/engine/flink/operation/FlinkEngineInitializeSuite.scala

pan3793 closed this in 4668baf Nov 2, 2023

hadoopkandy deleted the KYUUBI-5507 branch November 2, 2023 11:47

pan3793 modified the milestones: v1.9.0, v1.8.1 Nov 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[KYUUBI #5507][FLINK] Support Initialize SQL in Flink Engine #5518

[KYUUBI #5507][FLINK] Support Initialize SQL in Flink Engine #5518

Uh oh!

hadoopkandy commented Oct 25, 2023 •

edited

Loading

Uh oh!

link3280 left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Nov 1, 2023 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

pan3793 Nov 2, 2023

Uh oh!

link3280 Nov 2, 2023

Uh oh!

pan3793 left a comment

Uh oh!

Uh oh!

pan3793 commented Nov 2, 2023

Uh oh!

pan3793 commented Nov 24, 2023

Uh oh!

link3280 commented Nov 24, 2023

Uh oh!

pan3793 commented Nov 24, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[KYUUBI #5507][FLINK] Support Initialize SQL in Flink Engine #5518

[KYUUBI #5507][FLINK] Support Initialize SQL in Flink Engine #5518

Uh oh!

Conversation

hadoopkandy commented Oct 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why are the changes needed?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

Uh oh!

link3280 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Nov 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

pan3793 Nov 2, 2023

Choose a reason for hiding this comment

Uh oh!

link3280 Nov 2, 2023

Choose a reason for hiding this comment

Uh oh!

pan3793 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pan3793 commented Nov 2, 2023

Uh oh!

pan3793 commented Nov 24, 2023

Uh oh!

link3280 commented Nov 24, 2023

Uh oh!

pan3793 commented Nov 24, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hadoopkandy commented Oct 25, 2023 •

edited

Loading

codecov-commenter commented Nov 1, 2023 •

edited

Loading