[KYUUBI #2721] Implement dedicated set/get catalog/database operators #2728

cxzl25 · 2022-05-23T21:15:02Z

Why are the changes needed?

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

codecov-commenter · 2022-05-23T22:21:13Z

Codecov Report

Merging #2728 (698fbbd) into master (49352b5) will increase coverage by 0.22%.
The diff coverage is 79.20%.

❗ Current head 698fbbd differs from pull request most recent head 8bc40b9. Consider uploading reports for the commit 8bc40b9 to get more accurate results

@@             Coverage Diff              @@
##             master    #2728      +/-   ##
============================================
+ Coverage     61.79%   62.02%   +0.22%     
+ Complexity      385      273     -112     
============================================
  Files           433      431       -2     
  Lines         20345    20070     -275     
  Branches       2760     2728      -32     
============================================
- Hits          12572    12448     -124     
+ Misses         6541     6424     -117     
+ Partials       1232     1198      -34

Impacted Files	Coverage Δ
...he/kyuubi/engine/spark/shim/CatalogShim_v2_4.scala	`71.64% <0.00%> (-4.55%)`	⬇️
...he/kyuubi/engine/spark/shim/SparkCatalogShim.scala	`78.26% <ø> (ø)`
.../org/apache/kyuubi/config/KyuubiReservedKeys.scala	`0.00% <ø> (ø)`
.../org/apache/kyuubi/jdbc/hive/KyuubiConnection.java	`3.96% <0.00%> (-0.06%)`	⬇️
...a/org/apache/kyuubi/jdbc/hive/KyuubiStatement.java	`23.96% <0.00%> (-1.30%)`	⬇️
...ubi/engine/spark/operation/SetCurrentCatalog.scala	`66.66% <66.66%> (ø)`
...bi/engine/spark/operation/SetCurrentDatabase.scala	`66.66% <66.66%> (ø)`
...rg/apache/kyuubi/operation/GetCurrentCatalog.scala	`80.00% <80.00%> (ø)`
...g/apache/kyuubi/operation/GetCurrentDatabase.scala	`80.00% <80.00%> (ø)`
...ubi/engine/flink/operation/GetCurrentCatalog.scala	`83.33% <83.33%> (ø)`
... and 53 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 49352b5...8bc40b9. Read the comment docs.

pan3793 · 2022-05-24T03:41:00Z

Thanks @cxzl25, I verified this patch works w/ DBeaver/Spark

cxzl25 · 2022-05-24T04:36:41Z

I verified this patch works w/ DBeaver/Spark

Thank you for helping me test, I haven't tested it yet, I'll add some UT later.

pan3793 · 2022-05-25T03:47:30Z

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

+      assert(statement.execute("drop catalog cat_a"))
+    })
+  }
+
  test("execute statement - create/alter/drop database") {
    // TODO: validate table results after FLINK-25558 is resolved


@link3280 I see you comment it's fixed by FLINK-24685, should we update it?

Yes, the comment could be deleted now.

kyuubi-common/src/main/scala/org/apache/kyuubi/operation/OperationManager.scala

pan3793 · 2022-05-25T04:27:20Z

kyuubi-common/src/test/scala/org/apache/kyuubi/HiveEngineTests.scala

@@ -452,4 +452,28 @@ trait HiveEngineTests extends HiveJDBCTestHelper {
      assert(resultSet.getString(1) === "hive.query.name=test")
    }
  }
+
+  test("test set/get catalog") {
+    assume(SystemUtils.isJavaVersionAtMost(JavaVersion.JAVA_1_8))


where the restriction comes from?

Emm..., I see, it comes from Hive

pan3793 · 2022-05-25T04:36:04Z

Thanks @cxzl25, the change LGTM overall, only one concern, if user use Kyuubi driver to connect HS2/STS and invokes setCatalog/getCatalog setSchema/getSchema, will it break?

cxzl25 · 2022-05-25T04:44:11Z

if user use Kyuubi driver to connect HS2/STS and invokes setCatalog/getCatalog setSchema/getSchema, will it break?

If the connection is not to the kyuubi server, there should be problems in calling these methods.
Maybe add an option to the client to keep it compatible?(e.g. useKyuubiCatalogSchemaMethod)

...trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/operation/SetCurrentCatalog.scala

...rino-engine/src/main/scala/org/apache/kyuubi/engine/trino/operation/SetCurrentDatabase.scala

...k-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/operation/SetCurrentCatalog.scala

yaooqinn · 2022-05-25T05:34:24Z

kyuubi-hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/hive/KyuubiStatement.java

+
+  public ResultSet executeGetCurrentCatalog() throws SQLException {
+    if (!executeWithConfOverlay(
+        "", Collections.singletonMap("kyuubi.operation.get.current.catalog", ""))) {


any special reason to use such hacky way? no SQL supported?

Explained in #2721

Because different engines get and set the sql of the catalog database differently.

setCatalog getCatalog setSchema getSchema

Spark USE catalog_1.db_1 SELECT current_catalog() (Since 3.1) USE db_1 SELECT current_database()

Flink USE CATALOG catalog_1 SHOW CURRENT CATALOG USE db_1 SHOW CURRENT DATABASE

Trino USE catalog_1.db_1 x USE db_1 x

Hive x x USE db_1 SELECT current_database()

How about using placeholders for the statement argement?

For example SELECT 'kyuubi.operation.get.current.catalog' ?

yes, maybe something like _SET_CATALOG_${catalogName} / _GET_CATALOG

# Conflicts: # docs/deployment/settings.md # kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala

pan3793 · 2022-05-26T10:05:43Z

Thanks, merging to master

set/get catalog/database operators

21e5787

github-actions bot added module:common module:ctl module:flink module:hive module:server module:spark module:trino labels May 23, 2022

cxzl25 marked this pull request as draft May 23, 2022 21:15

cxzl25 added 3 commits May 24, 2022 21:50

add UT

9877038

hive engine 1.8

4982ec7

fix server spark catalog UT

7f13ad8

cxzl25 marked this pull request as ready for review May 24, 2022 17:15

pan3793 reviewed May 25, 2022

View reviewed changes

kyuubi-common/src/main/scala/org/apache/kyuubi/operation/OperationManager.scala Outdated Show resolved Hide resolved

pan3793 reviewed May 25, 2022

View reviewed changes

pan3793 approved these changes May 25, 2022

View reviewed changes

fix import

45caf55

pan3793 requested review from turboFei and yaooqinn May 25, 2022 04:36

yaooqinn approved these changes May 25, 2022

View reviewed changes

yaooqinn reviewed May 25, 2022

View reviewed changes

...trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/operation/SetCurrentCatalog.scala Outdated Show resolved Hide resolved

yaooqinn reviewed May 25, 2022

View reviewed changes

...rino-engine/src/main/scala/org/apache/kyuubi/engine/trino/operation/SetCurrentDatabase.scala Outdated Show resolved Hide resolved

yaooqinn reviewed May 25, 2022

View reviewed changes

...k-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/operation/SetCurrentCatalog.scala Show resolved Hide resolved

yaooqinn reviewed May 25, 2022

View reviewed changes

cxzl25 added 2 commits May 25, 2022 15:03

set catalog/schema empty resultset

698fbbd

use placeholders

f62461c

github-actions bot added the kind:documentation Documentation is a feature! label May 25, 2022

cxzl25 added 6 commits May 26, 2022 02:13

Merge branch 'master' into KYUUBI-2721

a0247c7

# Conflicts: # docs/deployment/settings.md # kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala

fix UT

ff9078a

style

e55bb2e

fix jdbc

3495b28

fix UT

8bc40b9

unused import

2acf0a4

yaooqinn approved these changes May 26, 2022

View reviewed changes

use placeholders and confOverlay

6705164

pan3793 approved these changes May 26, 2022

View reviewed changes

pan3793 requested a review from yaooqinn May 26, 2022 08:15

yaooqinn approved these changes May 26, 2022

View reviewed changes

pan3793 assigned cxzl25 May 26, 2022

pan3793 added this to the v1.6.0 milestone May 26, 2022

pan3793 closed this in 9b50230 May 26, 2022

pan3793 mentioned this pull request Jun 17, 2022

[Bug] Dbeaver cannot see the catalog and table information in the Flink engine #2901

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[KYUUBI #2721] Implement dedicated set/get catalog/database operators #2728

[KYUUBI #2721] Implement dedicated set/get catalog/database operators #2728

cxzl25 commented May 23, 2022 •

edited

Loading

codecov-commenter commented May 23, 2022 •

edited

Loading

pan3793 commented May 24, 2022

cxzl25 commented May 24, 2022

pan3793 May 25, 2022

link3280 May 25, 2022

pan3793 May 25, 2022

pan3793 May 25, 2022 •

edited

Loading

pan3793 commented May 25, 2022 •

edited

Loading

cxzl25 commented May 25, 2022 •

edited

Loading

yaooqinn May 25, 2022

pan3793 May 25, 2022

cxzl25 May 25, 2022 •

edited

Loading

yaooqinn May 25, 2022

cxzl25 May 25, 2022

yaooqinn May 25, 2022

pan3793 commented May 26, 2022

	setCatalog	getCatalog	setSchema	getSchema
Spark	USE catalog_1.db_1	SELECT current_catalog() (Since 3.1)	USE db_1	SELECT current_database()
Flink	USE CATALOG catalog_1	SHOW CURRENT CATALOG	USE db_1	SHOW CURRENT DATABASE
Trino	USE catalog_1.db_1	x	USE db_1	x
Hive	x	x	USE db_1	SELECT current_database()

[KYUUBI #2721] Implement dedicated set/get catalog/database operators #2728

[KYUUBI #2721] Implement dedicated set/get catalog/database operators #2728

Conversation

cxzl25 commented May 23, 2022 • edited Loading

Why are the changes needed?

How was this patch tested?

codecov-commenter commented May 23, 2022 • edited Loading

Codecov Report

pan3793 commented May 24, 2022

cxzl25 commented May 24, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pan3793 May 25, 2022 • edited Loading

Choose a reason for hiding this comment

pan3793 commented May 25, 2022 • edited Loading

cxzl25 commented May 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cxzl25 May 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pan3793 commented May 26, 2022

cxzl25 commented May 23, 2022 •

edited

Loading

codecov-commenter commented May 23, 2022 •

edited

Loading

pan3793 May 25, 2022 •

edited

Loading

pan3793 commented May 25, 2022 •

edited

Loading

cxzl25 commented May 25, 2022 •

edited

Loading

cxzl25 May 25, 2022 •

edited

Loading