[feature] Added constraint support in migrations #108
Conversation
| :create_if_not_exists -> " ADD CONSTRAINT IF NOT EXISTS " | ||
| end | ||
|
|
||
| constraint_expr = constraint |> constraint_expr() |> Enum.join("") |
There was a problem hiding this comment.
We probably don't need to Enum.join since we are returning an iolist anyway.
There was a problem hiding this comment.
Removed Enum.join as removed constraint_expr/1
|
|
||
| defp constraint_expr(%Constraint{check: check, validate: true, comment: comment}) | ||
| when is_binary(check) and is_binary(comment) do | ||
| raise "Clickhouse adapter does not support comments on check constraints" |
There was a problem hiding this comment.
I'd probably put all these checks into a single function clause like this
defp constraint_expr(%Constraint{} = constraint) do
if constraint.comment do
raise "Clickhouse adapter does not support comments on check constraints"
end
unless constraint.validate do
raise ...
end
# etc. checks for other unsupported fields / options
[@conn.quote_name(constraint.name), " CHECK (", constraint.check, ?)]
endThere was a problem hiding this comment.
In that case maybe it's more appropriate to move them inside execute_ddl/1
|
@senconscious thank you! I released these changes as |
| end | ||
|
|
||
| unless constraint.validate do | ||
| raise "Clickhouse adapter does not support check constraints without validation on creation" |
There was a problem hiding this comment.
I've been refactoring migrations and noticed
Constraint check will not be executed on existing data if it was added.
on https://clickhouse.com/docs/en/sql-reference/statements/alter/constraint
There was a problem hiding this comment.
Doesn't it mean that we should have the opposite check?
if constraint.validate do
raise "ClickHouse does not support check constraints with validation on creation"
end
Hello, there was no support for constraint in create, create_if_not_exists, drop, drop_if_exists. Need to run those through execute. So I decided to contribute and add this small feature.