-
Notifications
You must be signed in to change notification settings - Fork 531
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add clickhouse-replicated dialect #809
Conversation
ee7131e
to
8a0af1a
Compare
8a0af1a
to
cf87405
Compare
date Date default now(), | ||
tstamp DateTime default now() | ||
) | ||
ENGINE = ReplicatedMergeTree() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be awesome to be able to pass keeper path here as well, like
ENGINE = ReplicatedMergeTree('/clickhouse/tables/common/migrations', '{replica}')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's an endless number of ways to configure clickhouse, which means goose will need to know about all these and expose knobs for them.
I think the most practical solution would be for projects to implement the Store
interface, e.g.,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe an alternative would be to expose configuration options via a .yaml file. I think what I'm getting at is it's not sustainable to maintain dozens of custom clickhouse dialects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Totally agree! The Store interface looks much better
@yakovlevmichaelv are you planning on implementing store interface? For now I manually initialize version tables (in my case multiple)
|
This PR introduces a new clickhouse-replicated dialect, and here’s why it’s needed:
ReplicatedMergeTree
engine is required.ON CLUSTER
clause is a must forCREATE TABLE
queries.These differences between a single-node ClickHouse instance and a replicated cluster are why we’re adding this new dialect.
Added the environment variable
GOOSE_CLICKHOUSE_CLUSTER_NAME
(default:{cluster}
) allows to set Clickhouse cluster name