cube-js / cube

📊 Cube — The Semantic Layer for Building Data Applications
https://cube.dev
Other
17.49k stars 1.74k forks source link

Implement driver for CockroachDB #1164

Open ovr opened 3 years ago

ovr commented 3 years ago

Hey!

Currently, there are no plans to add support for it (from core team), but we are happy to merge and help with contribution from the community.

For driver implementer, take a look at

https://github.com/cube-js/cube.js/blob/master/CONTRIBUTING.md#implementing-sql-dialect

Example of driver implementation

https://github.com/cube-js/cube.js/pull/1099

Who are interested in preparing PR, don't forget to comment this issue

Thanks

ronan696 commented 3 years ago

@ovr I would like to work on this issue. I see that Cockroach DB fully supports the Postgres JDBC driver(https://www.cockroachlabs.com/docs/stable/install-client-drivers.html#java). So planning to add JDBC config for the same. Do let me know your thoughts. Thanks!

ovr commented 3 years ago

Hello Ronan D'Souza,

The JDBC driver is outdated and ignored on our CI because it's working on Node.js <= 8. It requires migration from one JDBC vendor to another because the current one is outdated and not supported.

  1. https://github.com/CraZySacX/node-jdbc (current one)
  2. https://github.com/naxmefy/node-jdbc (probably it's better module with active support)

So planning to add JDBC config for the same.

It's not the only thing that should be done to support CockroachDB. Each driver requires a dialect for SQL.

Implementing a "native" driver is a more useful and stable solution that we should choose.

ronan696 commented 3 years ago

Thanks for your feedback, Dmitry.

Since cockroachDB supports Postgres clients, I tried connecting to a cockroachDB using the Postgres driver in cube.js. I found that it partly works. For some reason, this query returns an SQL syntax error select 1 id, 100 amount, 'new' status; in cockroachDB and so have opened an issue in the cockroachDB repository https://github.com/cockroachdb/cockroach/issues/55826.

Will also try exploring the native driver route.

ovr commented 3 years ago

Driver contains SQL dialect. It's a bad idea to use PostgreSQL Dialect with CockroachDB.

Take a look at https://github.com/cube-js/cube.js/blob/v0.22.1/packages/cubejs-schema-compiler/adapter/PostgresQuery.js https://github.com/cube-js/cube.js/blob/v0.22.1/packages/cubejs-schema-compiler/adapter/BaseQuery.js

0xVolodya commented 3 years ago

Hi, I'm looking at some drivers. When I run a test inside drivers which I'm looking at, the test fails. What am I doing wrong? Screenshot from 2020-12-01 18-44-35 Screenshot from 2020-12-01 18-44-45

Screenshot from 2020-12-01 18-37-49

paveltiunov commented 3 years ago

@nadalfederer Do you have a running Docker on your machine? Makes to add it to the contributing guide cc @ovr @hassankhan .

hassankhan commented 3 years ago

Hi @nadalfederer :wave:

We do have some instructions for setting up a local development environment using Docker in our Contributor's Guide.

Please do let us know if you face any issues with the instructions by opening an issue on GitHub 😄