[FEATURE] Support Apache Phoenix engine base on jdbc engine #4000

531651225 · 2022-12-18T04:20:18Z

Why are the changes needed?

refer to #3999

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
config Apache Phoenix jdbc engine in kyuubi-defaults.conf

- use beeline connected

- query phoenix in kyuubi

codecov-commenter · 2022-12-18T06:15:34Z

Codecov Report

Merging #4000 (df18b4b) into master (c577dc7) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #4000      +/-   ##
============================================
- Coverage     52.11%   52.10%   -0.01%     
  Complexity       13       13              
============================================
  Files           541      541              
  Lines         29468    29485      +17     
  Branches       3938     3939       +1     
============================================
+ Hits          15357    15363       +6     
- Misses        12709    12710       +1     
- Partials       1402     1412      +10

Impacted Files	Coverage Δ
...rg/apache/kyuubi/ctl/cmd/log/LogBatchCommand.scala	`60.00% <0.00%> (-7.70%)`	⬇️
.../engine/spark/session/SparkSQLSessionManager.scala	`78.48% <0.00%> (-1.27%)`	⬇️
...apache/kyuubi/service/TBinaryFrontendService.scala	`48.38% <0.00%> (-1.08%)`	⬇️
...n/scala/org/apache/kyuubi/engine/ProcBuilder.scala	`80.74% <0.00%> (-0.63%)`	⬇️
...he/kyuubi/ha/client/etcd/EtcdDiscoveryClient.scala	`68.50% <0.00%> (-0.56%)`	⬇️
...apache/kyuubi/session/KyuubiBatchSessionImpl.scala	`86.66% <0.00%> (+0.08%)`	⬆️
.../org/apache/kyuubi/session/KyuubiSessionImpl.scala	`78.98% <0.00%> (+0.15%)`	⬆️
...rg/apache/kyuubi/events/KyuubiOperationEvent.scala	`96.96% <0.00%> (+0.19%)`	⬆️
...g/apache/kyuubi/operation/BatchJobSubmission.scala	`75.75% <0.00%> (+0.28%)`	⬆️

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

pom.xml

...kyuubi-jdbc-engine/src/main/scala/org/apache/kyuubi/engine/jdbc/dialect/PhoenixDialect.scala

pan3793 · 2022-12-22T02:17:47Z

Could you add integration tests for Phoenix?

531651225 · 2023-01-05T01:56:58Z

Could you add integration tests for Phoenix?

thanks your advice，i added it in new commit

pan3793 · 2023-01-05T02:08:04Z

...kyuubi-jdbc-engine/src/main/scala/org/apache/kyuubi/engine/jdbc/dialect/PhoenixDialect.scala

+  override def createStatement(connection: Connection, fetchSize: Int): Statement = {
+    val statement =
+      connection.createStatement(ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY)
+    statement.setFetchSize(Integer.MIN_VALUE)


why ignore fetchSize and set it to Integer.MIN_VALUE?

why ignore fetchSize and set it to Integer.MIN_VALUE?

fix it

pan3793 · 2023-01-05T02:10:34Z

...kyuubi-jdbc-engine/src/main/scala/org/apache/kyuubi/engine/jdbc/dialect/PhoenixDialect.scala

+  }
+
+  override def getTableTypesOperation(session: Session): Operation = {
+    throw KyuubiSQLException.featureNotSupported()


I guess this can be implemented since line 62 returns Set("s", "u")

I guess this can be implemented since line 62 returns Set("s", "u")

thanks your advice， Whether it can be improved in the future, and how to implement it has not been decided

pan3793 · 2023-01-05T02:17:40Z

pom.xml

        <ldapsdk.version>6.0.5</ldapsdk.version>
        <log4j.version>2.19.0</log4j.version>
        <mysql.jdbc.version>8.0.27</mysql.jdbc.version>
+        <phoenix.version>6.0.0</phoenix.version>


please sort alphabetically

please sort alphabetically

have fix it

pan3793

LGTM on my side, cc @zhaomin1423

zhaomin1423 · 2023-01-05T10:23:48Z

LGTM. maybe we need improve configs later, now can't use multiple jdbc source meanwhile.

531651225 · 2023-01-05T12:29:42Z

LGTM. maybe we need improve configs later, now can't use multiple jdbc source meanwhile.

There are many requirements for interactive query of multiple data sources. I hope that when the leaders have plans, I have the opportunity to participate

pan3793 · 2023-01-05T13:57:40Z

Thanks, merged to master

### _Why are the changes needed?_ As #4000 adds the Apache Phoenix support for the JDBC engine without upgrading the configuration's description. This PR aims to update the outdated configuration description to reflect the capabilities of the Kyuubi JDBC engine. ### _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](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 #5294 from ymZhao1001/doc0915. Closes #5294 8d61c36 [yangming] update jdbc doc d082b4d [yangming] run dev/reformat and dev/gen/gen_all_config_docs.sh 5b814d8 [zhaoyangming] add jdbc Phoenix dialect Lead-authored-by: yangming <[email protected]> Co-authored-by: zhaoyangming <[email protected]> Signed-off-by: Cheng Pan <[email protected]>

### _Why are the changes needed?_ As #4000 adds the Apache Phoenix support for the JDBC engine without upgrading the configuration's description. This PR aims to update the outdated configuration description to reflect the capabilities of the Kyuubi JDBC engine. ### _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](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 #5294 from ymZhao1001/doc0915. Closes #5294 8d61c36 [yangming] update jdbc doc d082b4d [yangming] run dev/reformat and dev/gen/gen_all_config_docs.sh 5b814d8 [zhaoyangming] add jdbc Phoenix dialect Lead-authored-by: yangming <[email protected]> Co-authored-by: zhaoyangming <[email protected]> Signed-off-by: Cheng Pan <[email protected]> (cherry picked from commit 289549f) Signed-off-by: Cheng Pan <[email protected]>

[FEATURE] Support Apache Phoenix engine base on jdbc engine

89feb69

github-actions bot added kind:build module:jdbc labels Dec 18, 2022

yaooqinn reviewed Dec 19, 2022

View reviewed changes

pom.xml Outdated Show resolved Hide resolved

using the origin artifact/group org.apache.phoenix in pom

df18b4b

531651225 requested a review from yaooqinn December 20, 2022 01:35

pan3793 reviewed Dec 22, 2022

View reviewed changes

pom.xml Outdated Show resolved Hide resolved

pan3793 reviewed Dec 22, 2022

View reviewed changes

...kyuubi-jdbc-engine/src/main/scala/org/apache/kyuubi/engine/jdbc/dialect/PhoenixDialect.scala Outdated Show resolved Hide resolved

pan3793 reviewed Dec 22, 2022

View reviewed changes

...kyuubi-jdbc-engine/src/main/scala/org/apache/kyuubi/engine/jdbc/dialect/PhoenixDialect.scala Outdated Show resolved Hide resolved

毕博 added 3 commits January 3, 2023 18:26

add integration test

c666f35

add integration test

812e4a0

Merge branch 'master' into add_phonix_jdbc-engin

1fd13dc

github-actions bot added the module:integration-tests label Jan 4, 2023

531651225 requested review from pan3793 and removed request for yaooqinn January 5, 2023 01:57

pan3793 reviewed Jan 5, 2023

View reviewed changes

fix PhoenixDialect fetchSize and pom dependency alphabetically

2b947fd

pan3793 approved these changes Jan 5, 2023

View reviewed changes

pan3793 assigned 531651225 Jan 5, 2023

pan3793 added this to the v1.7.0 milestone Jan 5, 2023

pan3793 closed this in 32b06c6 Jan 5, 2023

pan3793 mentioned this pull request Jan 5, 2023

[FEATURE] Support Apache Phoenix Engine #3999

Closed

5 tasks

pan3793 mentioned this pull request Sep 15, 2023

[DOC] Update supported dialects for JDBC engine #5294

Closed

3 tasks

[FEATURE] Support Apache Phoenix engine base on jdbc engine #4000

[FEATURE] Support Apache Phoenix engine base on jdbc engine #4000

Uh oh!

Conversation

531651225 commented Dec 18, 2022 • edited by pan3793 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?

Uh oh!

codecov-commenter commented Dec 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pan3793 commented Dec 22, 2022

Uh oh!

531651225 commented Jan 5, 2023

Uh oh!

pan3793 Jan 5, 2023

Choose a reason for hiding this comment

Uh oh!

531651225 Jan 5, 2023

Choose a reason for hiding this comment

Uh oh!

pan3793 Jan 5, 2023

Choose a reason for hiding this comment

Uh oh!

531651225 Jan 5, 2023 • edited by pan3793 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pan3793 Jan 5, 2023

Choose a reason for hiding this comment

Uh oh!

531651225 Jan 5, 2023

Choose a reason for hiding this comment

Uh oh!

pan3793 left a comment

Choose a reason for hiding this comment

Uh oh!

zhaomin1423 commented Jan 5, 2023

Uh oh!

531651225 commented Jan 5, 2023

Uh oh!

pan3793 commented Jan 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

531651225 commented Dec 18, 2022 •

edited by pan3793

Loading

codecov-commenter commented Dec 18, 2022 •

edited

Loading

531651225 Jan 5, 2023 •

edited by pan3793

Loading