Skip to content

Commit

Permalink
add check constraints for MySQL (#621)
Browse files Browse the repository at this point in the history
  • Loading branch information
petermueller committed Jul 6, 2024
1 parent ccb62ea commit b2c9c98
Show file tree
Hide file tree
Showing 7 changed files with 223 additions and 40 deletions.
83 changes: 83 additions & 0 deletions integration_test/myxql/constraints_test.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
defmodule Ecto.Integration.ConstraintsTest do
use ExUnit.Case, async: true

import Ecto.Migrator, only: [up: 4]
alias Ecto.Integration.PoolRepo

defmodule ConstraintMigration do
use Ecto.Migration

@table table(:constraints_test)

def change do
create @table do
add :price, :integer
add :from, :integer
add :to, :integer
end

# Only valid after MySQL 8.0.19
create constraint(@table.name, :positive_price, check: "price > 0")
end
end

defmodule Constraint do
use Ecto.Integration.Schema

schema "constraints_test" do
field :price, :integer
field :from, :integer
field :to, :integer
end
end

@base_migration 2_000_000

setup_all do
ExUnit.CaptureLog.capture_log(fn ->
num = @base_migration + System.unique_integer([:positive])
up(PoolRepo, num, ConstraintMigration, log: false)
end)

:ok
end

@tag :create_constraint
test "check constraint" do
# When the changeset doesn't expect the db error
changeset = Ecto.Changeset.change(%Constraint{}, price: -10)
exception =
assert_raise Ecto.ConstraintError, ~r/constraint error when attempting to insert struct/, fn ->
PoolRepo.insert(changeset)
end

assert exception.message =~ "\"positive_price\" (check_constraint)"
assert exception.message =~ "The changeset has not defined any constraint."
assert exception.message =~ "call `check_constraint/3`"

# When the changeset does expect the db error, but doesn't give a custom message
{:error, changeset} =
changeset
|> Ecto.Changeset.check_constraint(:price, name: :positive_price)
|> PoolRepo.insert()
assert changeset.errors == [price: {"is invalid", [constraint: :check, constraint_name: "positive_price"]}]
assert changeset.data.__meta__.state == :built

# When the changeset does expect the db error and gives a custom message
changeset = Ecto.Changeset.change(%Constraint{}, price: -10)
{:error, changeset} =
changeset
|> Ecto.Changeset.check_constraint(:price, name: :positive_price, message: "price must be greater than 0")
|> PoolRepo.insert()
assert changeset.errors == [price: {"price must be greater than 0", [constraint: :check, constraint_name: "positive_price"]}]
assert changeset.data.__meta__.state == :built

# When the change does not violate the check constraint
changeset = Ecto.Changeset.change(%Constraint{}, price: 10, from: 100, to: 200)
{:ok, changeset} =
changeset
|> Ecto.Changeset.check_constraint(:price, name: :positive_price, message: "price must be greater than 0")
|> PoolRepo.insert()
assert is_integer(changeset.id)
end
end
2 changes: 1 addition & 1 deletion integration_test/myxql/test_helper.exs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ excludes = [
if Version.match?(version, ">= 8.0.0") do
ExUnit.configure(exclude: excludes)
else
ExUnit.configure(exclude: [:values_list, :rename_column | excludes])
ExUnit.configure(exclude: [:create_constraint, :values_list, :rename_column | excludes])
end

:ok = Ecto.Migrator.up(TestRepo, 0, Ecto.Integration.Migration, log: false)
Expand Down
59 changes: 51 additions & 8 deletions lib/ecto/adapters/myxql/connection.ex
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,18 @@ if Code.ensure_loaded?(MyXQL) do
end
end

def to_constraints(
%MyXQL.Error{mysql: %{name: :ER_CHECK_CONSTRAINT_VIOLATED}, message: message},
_opts
) do
with [_, quoted] <- :binary.split(message, ["Check constraint "]),
[_, constraint | _] <- :binary.split(quoted, @quotes, [:global]) do
[check: constraint]
else
_ -> []
end
end

def to_constraints(_, _),
do: []

Expand Down Expand Up @@ -1026,12 +1038,31 @@ if Code.ensure_loaded?(MyXQL) do
def execute_ddl({:create_if_not_exists, %Index{}}),
do: error!(nil, "MySQL adapter does not support create if not exists for index")

def execute_ddl({:create, %Constraint{check: check}}) when is_binary(check),
do: error!(nil, "MySQL adapter does not support check constraints")
def execute_ddl({:create, %Constraint{check: check} = constraint}) when is_binary(check) do
table_name = quote_name(constraint.prefix, constraint.table)
[["ALTER TABLE ", table_name, " ADD ", new_constraint_expr(constraint)]]
end

def execute_ddl({:create, %Constraint{exclude: exclude}}) when is_binary(exclude),
do: error!(nil, "MySQL adapter does not support exclusion constraints")

def execute_ddl({:drop, %Constraint{}, :cascade}),
do: error!(nil, "MySQL does not support `CASCADE` in `DROP CONSTRAINT` commands")

def execute_ddl({:drop, %Constraint{} = constraint, _}) do
[
[
"ALTER TABLE ",
quote_name(constraint.prefix, constraint.table),
" DROP CONSTRAINT ",
quote_name(constraint.name)
]
]
end

def execute_ddl({:drop_if_exists, %Constraint{}, _}),
do: error!(nil, "MySQL adapter does not support `drop_if_exists` for constraints")

def execute_ddl({:drop, %Index{}, :cascade}),
do: error!(nil, "MySQL adapter does not support cascade in drop index")

Expand All @@ -1047,12 +1078,6 @@ if Code.ensure_loaded?(MyXQL) do
]
end

def execute_ddl({:drop, %Constraint{}, _}),
do: error!(nil, "MySQL adapter does not support constraints")

def execute_ddl({:drop_if_exists, %Constraint{}, _}),
do: error!(nil, "MySQL adapter does not support constraints")

def execute_ddl({:drop_if_exists, %Index{}, _}),
do: error!(nil, "MySQL adapter does not support drop if exists for index")

Expand Down Expand Up @@ -1244,6 +1269,17 @@ if Code.ensure_loaded?(MyXQL) do
defp null_expr(true), do: " NULL"
defp null_expr(_), do: []

defp new_constraint_expr(%Constraint{check: check} = constraint) when is_binary(check) do
[
"CONSTRAINT ",
quote_name(constraint.name),
" CHECK (",
check,
")",
validate(constraint.validate)
]
end

defp default_expr({:ok, nil}),
do: " DEFAULT NULL"

Expand Down Expand Up @@ -1401,6 +1437,9 @@ if Code.ensure_loaded?(MyXQL) do
defp reference_on_update(:restrict), do: " ON UPDATE RESTRICT"
defp reference_on_update(_), do: []

defp validate(false), do: " NOT ENFORCED"
defp validate(_), do: []

## Helpers

defp get_source(query, sources, ix, source) do
Expand All @@ -1423,6 +1462,10 @@ if Code.ensure_loaded?(MyXQL) do

defp maybe_add_column_names(_, name), do: name

defp quote_name(nil, name), do: quote_name(name)

defp quote_name(prefix, name), do: [quote_name(prefix), ?., quote_name(name)]

defp quote_name(name) when is_atom(name) do
quote_name(Atom.to_string(name))
end
Expand Down
29 changes: 26 additions & 3 deletions lib/ecto/migration.ex
Original file line number Diff line number Diff line change
Expand Up @@ -1491,11 +1491,34 @@ defmodule Ecto.Migration do
* `:check` - A check constraint expression. Required when creating a check constraint.
* `:exclude` - An exclusion constraint expression. Required when creating an exclusion constraint.
* `:prefix` - The prefix for the table.
* `:validate` - Whether or not to validate the constraint on creation (true by default). Only
available in PostgreSQL, and should be followed by a command to validate the new constraint in
a following migration if false.
* `:validate` - Whether or not to validate the constraint on creation (true by default). See the section below for more information
* `:comment` - adds a comment to the constraint.
## Using `validate: false`
Validation/Enforcement of a constraint is enabled by default, but disabling on constraint
creation is supported by PostgreSQL, and MySQL, and can be done by setting `validate: false`.
Setting `validate: false` as an option can be useful, as the creation of a constraint will cause
a full table scan to check existing rows. The constraint will still be enforced for subsequent
inserts and updates, but should then be updated in a following command or migration to enforce
the new constraint.
Validating / Enforcing the constraint in a later command, or migration, can be done like so:
```
def change do
# PostgreSQL
  execute "ALTER TABLE products VALIDATE CONSTRAINT price_must_be_positive", ""
# MySQL
  execute "ALTER TABLE products ALTER CONSTRAINT price_must_be_positive ENFORCED", ""
end
```
See the [Safe Ecto Migrations guide](https://fly.io/phoenix-files/safe-ecto-migrations/) for an
in-depth explanation of the benefits of this approach.
"""
def constraint(table, name, opts \\ [])

Expand Down
2 changes: 1 addition & 1 deletion mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ defmodule EctoSQL.MixProject do
if path = System.get_env("MYXQL_PATH") do
{:myxql, path: path}
else
{:myxql, "~> 0.6", optional: true}
{:myxql, "~> 0.7", optional: true}
end
end

Expand Down
2 changes: 1 addition & 1 deletion mix.lock
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"makeup": {:hex, :makeup, "1.1.2", "9ba8837913bdf757787e71c1581c21f9d2455f4dd04cfca785c70bbfff1a76a3", [:mix], [{:nimble_parsec, "~> 1.2.2 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "cce1566b81fbcbd21eca8ffe808f33b221f9eee2cbc7a1706fc3da9ff18e6cac"},
"makeup_elixir": {:hex, :makeup_elixir, "0.16.2", "627e84b8e8bf22e60a2579dad15067c755531fea049ae26ef1020cad58fe9578", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "41193978704763f6bbe6cc2758b84909e62984c7752b3784bd3c218bb341706b"},
"makeup_erlang": {:hex, :makeup_erlang, "1.0.0", "6f0eff9c9c489f26b69b61440bf1b238d95badae49adac77973cbacae87e3c2e", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "ea7a9307de9d1548d2a72d299058d1fd2339e3d398560a0e46c27dab4891e4d2"},
"myxql": {:hex, :myxql, "0.6.3", "3d77683a09f1227abb8b73d66b275262235c5cae68182f0cfa5897d72a03700e", [:mix], [{:db_connection, "~> 2.4.1 or ~> 2.5", [hex: :db_connection, repo: "hexpm", optional: false]}, {:decimal, "~> 1.6 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:geo, "~> 3.4", [hex: :geo, repo: "hexpm", optional: true]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:table, "~> 0.1.0", [hex: :table, repo: "hexpm", optional: true]}], "hexpm", "af9eb517ddaced5c5c28e8749015493757fd4413f2cfccea449c466d405d9f51"},
"myxql": {:hex, :myxql, "0.7.1", "7c7b75aa82227cd2bc9b7fbd4de774fb19a1cdb309c219f411f82ca8860f8e01", [:mix], [{:db_connection, "~> 2.4.1 or ~> 2.5", [hex: :db_connection, repo: "hexpm", optional: false]}, {:decimal, "~> 1.6 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:geo, "~> 3.4", [hex: :geo, repo: "hexpm", optional: true]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:table, "~> 0.1.0", [hex: :table, repo: "hexpm", optional: true]}], "hexpm", "a491cdff53353a09b5850ac2d472816ebe19f76c30b0d36a43317a67c9004936"},
"nimble_parsec": {:hex, :nimble_parsec, "1.4.0", "51f9b613ea62cfa97b25ccc2c1b4216e81df970acd8e16e8d1bdc58fef21370d", [:mix], [], "hexpm", "9c565862810fb383e9838c1dd2d7d2c437b3d13b267414ba6af33e50d2d1cf28"},
"postgrex": {:hex, :postgrex, "0.17.3", "c92cda8de2033a7585dae8c61b1d420a1a1322421df84da9a82a6764580c503d", [:mix], [{:db_connection, "~> 2.1", [hex: :db_connection, repo: "hexpm", optional: false]}, {:decimal, "~> 1.5 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:table, "~> 0.1.0", [hex: :table, repo: "hexpm", optional: true]}], "hexpm", "946cf46935a4fdca7a81448be76ba3503cff082df42c6ec1ff16a4bdfbfb098d"},
"statistex": {:hex, :statistex, "1.0.0", "f3dc93f3c0c6c92e5f291704cf62b99b553253d7969e9a5fa713e5481cd858a5", [:mix], [], "hexpm", "ff9d8bee7035028ab4742ff52fc80a2aa35cece833cf5319009b52f1b5a86c27"},
Expand Down
86 changes: 60 additions & 26 deletions test/ecto/adapters/myxql_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -1621,7 +1621,8 @@ defmodule Ecto.Adapters.MyXQLTest do

# DDL

import Ecto.Migration, only: [table: 1, table: 2, index: 2, index: 3, constraint: 3]
import Ecto.Migration,
only: [table: 1, table: 2, index: 2, index: 3, constraint: 2, constraint: 3]

test "executing a string during migration" do
assert execute_ddl("example") == ["example"]
Expand Down Expand Up @@ -1963,23 +1964,6 @@ defmodule Ecto.Adapters.MyXQLTest do
assert execute_ddl(drop) == [~s|DROP TABLE `foo`.`posts`|]
end

test "drop constraint" do
assert_raise ArgumentError, ~r/MySQL adapter does not support constraints/, fn ->
execute_ddl(
{:drop, constraint(:products, "price_must_be_positive", prefix: "foo"), :restrict}
)
end
end

test "drop_if_exists constraint" do
assert_raise ArgumentError, ~r/MySQL adapter does not support constraints/, fn ->
execute_ddl(
{:drop_if_exists, constraint(:products, "price_must_be_positive", prefix: "foo"),
:restrict}
)
end
end

test "alter table" do
alter =
{:alter, table(:posts),
Expand Down Expand Up @@ -2152,15 +2136,34 @@ defmodule Ecto.Adapters.MyXQLTest do
end

test "create constraints" do
assert_raise ArgumentError, "MySQL adapter does not support check constraints", fn ->
create = {:create, constraint(:products, "foo", check: "price")}
assert execute_ddl(create)
end
create = {:create, constraint(:products, "price_must_be_positive", check: "price > 0")}

assert_raise ArgumentError, "MySQL adapter does not support check constraints", fn ->
create = {:create, constraint(:products, "foo", check: "price", validate: false)}
assert execute_ddl(create)
end
assert execute_ddl(create) ==
[
~s|ALTER TABLE `products` ADD CONSTRAINT `price_must_be_positive` CHECK (price > 0)|
]

create =
{:create,
constraint(:products, "price_must_be_positive", check: "price > 0", prefix: "foo")}

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

create =
{:create,
constraint(:products, "price_must_be_positive",
check: "price > 0",
prefix: "foo",
validate: false
)}

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

assert_raise ArgumentError, "MySQL adapter does not support exclusion constraints", fn ->
create = {:create, constraint(:products, "bar", exclude: "price")}
Expand All @@ -2173,6 +2176,37 @@ defmodule Ecto.Adapters.MyXQLTest do
end
end

test "drop constraint" do
drop = {:drop, constraint(:products, "price_must_be_positive"), :restrict}

assert execute_ddl(drop) ==
[~s|ALTER TABLE `products` DROP CONSTRAINT `price_must_be_positive`|]

drop = {:drop, constraint(:products, "price_must_be_positive", prefix: "foo"), :restrict}

assert execute_ddl(drop) ==
[~s|ALTER TABLE `foo`.`products` DROP CONSTRAINT `price_must_be_positive`|]

drop_cascade = {:drop, constraint(:products, "price_must_be_positive"), :cascade}

assert_raise ArgumentError,
~r/MySQL does not support `CASCADE` in `DROP CONSTRAINT` commands/,
fn ->
execute_ddl(drop_cascade)
end
end

test "drop_if_exists constraint" do
assert_raise ArgumentError,
~r/MySQL adapter does not support `drop_if_exists` for constraints/,
fn ->
execute_ddl(
{:drop_if_exists,
constraint(:products, "price_must_be_positive", prefix: "foo"), :restrict}
)
end
end

test "create an index using a different type" do
create = {:create, index(:posts, [:permalink], using: :hash)}

Expand Down

0 comments on commit b2c9c98

Please sign in to comment.