ClickHouse / metabase-clickhouse-driver

ClickHouse database driver for the Metabase business intelligence front-end
Apache License 2.0
476 stars 92 forks source link

New connection property "multiple-schema" #152

Closed veschin closed 1 year ago

veschin commented 1 year ago

Summary

Add multiple schemas feature (see Specify multiple schemes on connect)

Checklist

CLAassistant commented 1 year ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Oleg Veschin seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

slvrtrn commented 1 year ago

Thanks for the contribution.

We could implement that in a single field, renaming "Database" to "Databases", expecting either a single string (like now, i.e. default) or comma-separated (or space-separated, like in your implementation) values (default,data,datasets). WDYT?

veschin commented 1 year ago

Thanks for the contribution.

We could implement that in a single field, renaming "Database" to "Databases", expecting either a single string (like now, i.e. default) or comma-separated (or space-separated, like in your implementation) values (default,data,datasets). WDYT?

That would be great. Do you want to implement the new feature yourself or do you expect a patch from me?

slvrtrn commented 1 year ago

@veschin I'd appreciate the patch, but I could finish your PR as well (I will need to recreate it on another branch, though, cause it is coming from the fork)

veschin commented 1 year ago

@veschin I'd appreciate the patch, but I could finish your PR as well (I will need to recreate it on another branch, though, cause it is coming from the fork)

I'll write a patch and complete the test

veschin commented 1 year ago

@slvrtrn It's done?

slvrtrn commented 1 year ago

Thanks, I will merge it once CI is green and release it as 1.1.3