Skip to content
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

[feature] Added constraint support in migrations #108

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 49 additions & 4 deletions lib/ecto/adapters/clickhouse/migration.ex
Original file line number Diff line number Diff line change
Expand Up @@ -108,14 +108,59 @@ defmodule Ecto.Adapters.ClickHouse.Migration do
[[drop | @conn.quote_table(index.prefix, index.name)]]
end

def execute_ddl({command, %Constraint{} = _constraint})
def execute_ddl({command, %Constraint{} = constraint})
when command in [:create, :create_if_not_exists] do
raise "TODO"
if constraint.comment do
raise "Clickhouse adapter does not support comments on check constraints"
end

unless constraint.validate do
raise "Clickhouse adapter does not support check constraints without validation on creation"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@senconscious 👋

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

end

if constraint.exclude do
raise "Clickhouse adapter does not support exclude constraints"
end

unless constraint.check do
raise "Clickhouse adapter requires check option for constraints"
end

add =
case command do
:create -> " ADD CONSTRAINT "
:create_if_not_exists -> " ADD CONSTRAINT IF NOT EXISTS "
end

[
[
"ALTER TABLE ",
@conn.quote_table(constraint.prefix, constraint.table),
add,
@conn.quote_name(constraint.name),
" CHECK (",
constraint.check,
")"
]
]
end

def execute_ddl({command, %Constraint{} = _constraint, _mode})
def execute_ddl({command, %Constraint{} = constraint, _mode})
when command in [:drop, :drop_if_exists] do
raise "TODO"
drop =
case command do
:drop -> " DROP CONSTRAINT "
:drop_if_exists -> " DROP CONSTRAINT IF EXISTS "
end

[
[
"ALTER TABLE ",
@conn.quote_table(constraint.prefix, constraint.table),
drop,
@conn.quote_name(constraint.name)
]
]
end

def execute_ddl({:rename, %Table{} = current_table, %Table{} = new_table}) do
Expand Down
37 changes: 20 additions & 17 deletions test/ecto/adapters/clickhouse/connection_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -2057,21 +2057,21 @@ defmodule Ecto.Adapters.ClickHouse.ConnectionTest do
assert execute_ddl(drop) == [~s|DROP TABLE "foo"."posts"|]
end

@tag skip: true
test "drop constraint" do
drop = {:drop, constraint(:products, "price_must_be_positive", prefix: :foo), :restrict}

assert execute_ddl(drop) == [
~s{DROP CONSTRAINT}
~s|ALTER TABLE "foo"."products" DROP CONSTRAINT "price_must_be_positive"|
]
end

@tag skip: true
test "drop_if_exists constraint" do
drop =
{:drop_if_exists, constraint(:products, "price_must_be_positive", prefix: :foo), :restrict}

assert execute_ddl(drop) == []
assert execute_ddl(drop) == [
~s|ALTER TABLE "foo"."products" DROP CONSTRAINT IF EXISTS "price_must_be_positive"|
]
end

test "alter table" do
Expand Down Expand Up @@ -2355,16 +2355,6 @@ defmodule Ecto.Adapters.ClickHouse.ConnectionTest do
assert execute_ddl(drop) == [~s|DROP INDEX "posts$main"|]
end

@tag skip: true
test "create constraint" do
assert_raise ArgumentError,
"ClickHouse does not support ALTER TABLE ADD CONSTRAINT",
fn ->
{:create, constraint(:products, "price_must_be_positive", check: "price > 0")}
|> execute_ddl()
end
end

test "drop index with prefix" do
drop = {:drop, index(:posts, [:id], name: "posts$main", prefix: :foo), :restrict}
assert execute_ddl(drop) == [~s|DROP INDEX "foo"."posts$main"|]
Expand All @@ -2382,10 +2372,12 @@ defmodule Ecto.Adapters.ClickHouse.ConnectionTest do
assert execute_ddl(drop) == [~s|DROP INDEX "posts$main"|]
end

@tag skip: true
test "create check constraint" do
create = {:create, constraint(:products, "price_must_be_positive", check: "price > 0")}
assert execute_ddl(create) == []

assert execute_ddl(create) == [
~s|ALTER TABLE "products" ADD CONSTRAINT "price_must_be_positive" CHECK (price > 0)|
]

create =
{:create,
Expand All @@ -2394,7 +2386,18 @@ defmodule Ecto.Adapters.ClickHouse.ConnectionTest do
prefix: "foo"
)}

assert execute_ddl(create) == []
assert execute_ddl(create) == [
~s|ALTER TABLE "foo"."products" ADD CONSTRAINT "price_must_be_positive" CHECK (price > 0)|
]
end

test "create check constraint if not exists" do
create =
{:create_if_not_exists, constraint(:products, "price_must_be_positive", check: "price > 0")}

assert execute_ddl(create) == [
~s|ALTER TABLE "products" ADD CONSTRAINT IF NOT EXISTS "price_must_be_positive" CHECK (price > 0)|
]
end

@tag skip: true
Expand Down