kyuubi icon indicating copy to clipboard operation
kyuubi copied to clipboard

[KYUUBI #5469] JDBC Engine supports Redis

Open davidyuan1223 opened this issue 1 year ago โ€ข 8 comments

:mag: Description

This pr could support use jdbc mode to connect with redis, we can use the url like 'jdbc:redis://host:port' to connect redis server [This is a Draft pr, beacause there are some questions need resolve]

  1. Redis differs from common JDBC drivers like MySQL and Phoenix. Therefore, some common JDBC driver test cases are not applicable to Redis. Which components of Redis do we need to test?
  2. Should we add a test case for Redis cluster mode?

Issue References ๐Ÿ”—

This pull request fixes #5469

Describe Your Solution ๐Ÿ”ง

  1. Choose a redis-jdbc-driver for this component, currently, the redis jdbc driver is from https://github.com/eacdy/redis-jdbc
  2. Add Redis Dialect
  3. Write Redis Unit Test Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Types of changes :bookmark:

  • [ ] Bugfix (non-breaking change which fixes an issue)
  • [x] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to change)

Test Plan ๐Ÿงช

Behavior Without This Pull Request :coffin:

Behavior With This Pull Request :tada:

Related Unit Tests

org/apache/kyuubi/engine/jdbc/redis/RedisSessionSuite.scala org/apache/kyuubi/engine/jdbc/redis/RedisStatementSuite.scala


Checklist ๐Ÿ“

Be nice. Be informative.

davidyuan1223 avatar Jan 26 '24 11:01 davidyuan1223

@yaooqinn @cxzl25 please give me some advice

davidyuan1223 avatar Jan 26 '24 11:01 davidyuan1223

cc @pan3793 @zhaomin1423 @iodone

yaooqinn avatar Jan 30 '24 01:01 yaooqinn

cc @lsm1

cxzl25 avatar Jan 30 '24 06:01 cxzl25

@davidyuan1223 Before submitting the pull request, we can use ./dev/reformat to format the code.

iodone avatar Feb 07 '24 02:02 iodone

thanks for working on this area. my main concerns:

  1. the https://github.com/eacdy/redis-jdbc looks neither mature nor active
  2. the data model in Redis is different from RDBMS, how do we map the API?

pan3793 avatar Feb 20 '24 02:02 pan3793

2. the data model in Redis is different from RDBMS, how do we map the API?

  1. actually we also want use the redis-jdbc from Data-Grip but the Data-Grip not support the maven repository about the redis-jdbc,so i search a useful jdbc engine to replace it. If we find a more suitable component, we also can replace it.
  2. This question also is my concerns, It's different from RDBMS. The redis-jdbc connector has implementation the Mysql's Connection, we can execute redis commands in the form of JDBC. Therefore, this PR is mainly about how to test the connection between Redis and Kyuubi based on this format

davidyuan1223 avatar Feb 20 '24 09:02 davidyuan1223

It is not common to use jdbc to access redis. If the redis jdbc client supports the mysql dialect, I think we can use mysql type jdbc engine to access redis, like:

kyuubi.engine.jdbc.type=MYSQL
kyuubi.engine.jdbc.driver.class=RedisDriverClass
kyuubi.engine.jdbc.connection.url=jdbc:redis://XXXX

wForget avatar Feb 21 '24 07:02 wForget

It is not common to use jdbc to access redis. If the redis jdbc client supports the mysql dialect, I think we can use mysql type jdbc engine to access redis, like:

kyuubi.engine.jdbc.type=MYSQL
kyuubi.engine.jdbc.driver.class=RedisDriverClass
kyuubi.engine.jdbc.connection.url=jdbc:redis://XXXX

The Config Format remains consistent with the current logic. Because DriverClass is not mandatory to be itmuch's DriverClass. So curretly jdbc config format is

kyuubi.engine.type=jdbc
kyuubi.engine.jdbc.type=redis
kyuubi.engine.jdbc.driver.class=com.itmuch.redis.jdbc.redis.RedisDriver

I have implemented the redis's mysql dialect, in the pr, we need discuss how to test redis jdbc driver, because it's data format or metadata is not same as common jdbc engine

davidyuan1223 avatar Feb 27 '24 07:02 davidyuan1223