Bring Clickhouse driver into Metabase repo#54740
Merged
rileythomp merged 40 commits intomasterfrom Mar 21, 2025
Merged
Conversation
metabase-bot
bot
added
the
.Team/Drivers
|
|
rileythomp
commented
Mar 10, 2025
| (is (= (format "Mismatch detected between Dashboard's `collection_id` (%d) and `collection_id` (%d)" | ||
| coll-id | ||
| nil) | ||
| (is (= (tru "Mismatch detected between Dashboard''s `collection_id` ({0}) and `collection_id` ({1})" |
Contributor
Author
There was a problem hiding this comment.
This was failing because coll-id was being formatted as 1234 instead of 1,234
rileythomp
commented
Mar 10, 2025
|
|
||
| (nippy/extend-thaw :clickhouse/UnsignedByte | ||
| [^DataInput data-input] | ||
| (com.clickhouse.data.value.UnsignedByte/valueOf ^String (nippy/thaw-from-in! data-input))) ;; TODO: confirm ^String is the correct type hint here and below |
Contributor
Author
There was a problem hiding this comment.
Wanted to confirm this
rileythomp
commented
Mar 10, 2025
| (comment | ||
| ;; This test doesn't pass because honey.sql/format doesn't generate a valid | ||
| ;; insert statement. We can exclude this test for now or modify it to test | ||
| ;; similar behaviour in a way that works. |
Contributor
Author
There was a problem hiding this comment.
Didn't realize this until later, but we can probably work around this now with the init.sql file.
luizarakaki
reviewed
Mar 11, 2025
…se/metabase into dri-159-bring-clickhouse-in-house
snoe
suggested changes
Mar 13, 2025
modules/drivers/clickhouse/test/metabase/test/data/clickhouse_datasets.sql
Outdated
Show resolved
Hide resolved
modules/drivers/clickhouse/test/metabase/test/data/clickhouse.clj
Outdated
Show resolved
Hide resolved
modules/drivers/clickhouse/test/metabase/test/data/clickhouse.clj
Outdated
Show resolved
Hide resolved
modules/drivers/clickhouse/test/metabase/driver/clickhouse_data_types_test.clj
Outdated
Show resolved
Hide resolved
modules/drivers/clickhouse/test/metabase/driver/clickhouse_data_types_test.clj
Show resolved
Hide resolved
modules/drivers/clickhouse/src/metabase/driver/clickhouse_introspection.clj
Show resolved
Hide resolved
snoe
approved these changes
Mar 18, 2025
Contributor
e2e tests failed on
|
| File | Test Name |
|---|---|
reproductions-3.cy.spec.js |
(flaky) issue 45063 > type/FK -> column remapping (#45063) > should work with questions |
reproductions-3.cy.spec.js |
(flaky) issue 45063 > type/FK -> column remapping (#45063) > should work with native models |
luizarakaki
added a commit
that referenced
this pull request
May 22, 2025
* Bring Clickhouse driver into Metabase repo --------- Co-authored-by: Luiz Arakaki <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes https://linear.app/metabase/issue/DRI-159/bring-clickhouse-in-house
Description
Bringing the Clickhouse driver into the Metabase repo as an officially supported driver.
See https://github.com/rileythomp/metabase-clickhouse-driver/pull/1/files for the diff of this and https://github.com/ClickHouse/metabase-clickhouse-driver.
The main changes (outside of fixing linter warnings) are moving the test datasets from a SQL file they had to
defdatasets, and removingcreate-test-db!anddo-with-test-dbin favour ofmt/dataset.How to verify
Demo
TODO
Checklist