Skip to content

Commit 15f4330

Browse files
committed
Do not encourage disabling constraint checks
1 parent 89bdbd2 commit 15f4330

File tree

4 files changed

+5
-107
lines changed

4 files changed

+5
-107
lines changed

integration_test/pg/constraints_test.exs

+2-2
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ defmodule Ecto.Integration.ConstraintsTest do
4141
:ok
4242
end
4343

44-
test "creating, using, and dropping an exclusion constraint" do
44+
test "exclusion constraint" do
4545
changeset = Ecto.Changeset.change(%Constraint{}, from: 0, to: 10)
4646
{:ok, _} = PoolRepo.insert(changeset)
4747

@@ -75,7 +75,7 @@ defmodule Ecto.Integration.ConstraintsTest do
7575
assert changeset.data.__meta__.state == :built
7676
end
7777

78-
test "creating, using, and dropping a check constraint" do
78+
test "check constraint" do
7979
# When the changeset doesn't expect the db error
8080
changeset = Ecto.Changeset.change(%Constraint{}, price: -10)
8181
exception =

integration_test/tds/constraints_test.exs

+1-92
Original file line numberDiff line numberDiff line change
@@ -16,24 +16,6 @@ defmodule Ecto.Integration.ConstraintsTest do
1616
add :to, :integer
1717
end
1818
create constraint(@table.name, :cannot_overlap, check: "[from] < [to]")
19-
create constraint(@table.name, "positive_price", check: "[price] > 0")
20-
end
21-
end
22-
23-
defmodule ConstraintMigration2 do
24-
use Ecto.Migration
25-
26-
def change do
27-
opts = [with: "NOCHECK", check: "[from] < 200"]
28-
create constraint(:constraints_test, "from_max", opts)
29-
end
30-
end
31-
32-
defmodule ConstraintMigration3 do
33-
use Ecto.Migration
34-
35-
def change do
36-
drop constraint(:constraints_test, "from_max")
3719
end
3820
end
3921

@@ -58,7 +40,7 @@ defmodule Ecto.Integration.ConstraintsTest do
5840
:ok
5941
end
6042

61-
test "creating, using, and dropping an exclusion constraint" do
43+
test "check constraint" do
6244
changeset = Ecto.Changeset.change(%Constraint{}, from: 0, to: 10)
6345
{:ok, _} = PoolRepo.insert(changeset)
6446

@@ -75,84 +57,11 @@ defmodule Ecto.Integration.ConstraintsTest do
7557
assert exception.message =~ "The changeset has not defined any constraint."
7658
assert exception.message =~ "call `check_constraint/3`"
7759

78-
# Seems like below `Ecto.Changeset.check_constraint(:from)` is not valid for some reason,
79-
# constrint name is mandatory and ecto raises ArgumentError
80-
81-
# message = ~r/constraint error when attempting to insert struct/
82-
# exception =
83-
# assert_raise Ecto.ConstraintError, message, fn ->
84-
# overlapping_changeset
85-
# |> Ecto.Changeset.check_constraint(:from)
86-
# |> PoolRepo.insert()
87-
# end
88-
# assert exception.message =~ "cannot_overlap (check_constraint)"
89-
9060
{:error, changeset} =
9161
overlapping_changeset
9262
|> Ecto.Changeset.check_constraint(:from, name: :cannot_overlap)
9363
|> PoolRepo.insert()
9464
assert changeset.errors == [from: {"is invalid", [constraint: :check, constraint_name: "cannot_overlap"]}]
9565
assert changeset.data.__meta__.state == :built
96-
97-
ExUnit.CaptureLog.capture_log(fn ->
98-
# migrate over existing data, it should pass since `with: NOCHECK` is set
99-
num = @base_migration + System.unique_integer([:positive])
100-
assert :ok == up(PoolRepo, num, ConstraintMigration2, log: false)
101-
end)
102-
103-
# from is greated than max allowed by database, so check constrint should
104-
# forbid insert
105-
from_max_changeset = Ecto.Changeset.change(%Constraint{}, from: 300, to: 400)
106-
107-
exception =
108-
assert_raise Ecto.ConstraintError, ~r/constraint error when attempting to insert struct/, fn ->
109-
PoolRepo.insert(from_max_changeset)
110-
end
111-
assert exception.message =~ "from_max (check_constraint)"
112-
assert exception.message =~ "The changeset has not defined any constraint."
113-
assert exception.message =~ "call `check_constraint/3`"
114-
115-
ExUnit.CaptureLog.capture_log(fn ->
116-
num = @base_migration + System.unique_integer([:positive])
117-
assert :ok == up(PoolRepo, num, ConstraintMigration3, log: false)
118-
end)
119-
end
120-
121-
test "creating, using, and dropping a check constraint" do
122-
# When the changeset doesn't expect the db error
123-
changeset = Ecto.Changeset.change(%Constraint{}, price: -10)
124-
exception =
125-
assert_raise Ecto.ConstraintError, ~r/constraint error when attempting to insert struct/, fn ->
126-
PoolRepo.insert(changeset)
127-
end
128-
129-
assert exception.message =~ "positive_price (check_constraint)"
130-
assert exception.message =~ "The changeset has not defined any constraint."
131-
assert exception.message =~ "call `check_constraint/3`"
132-
133-
# When the changeset does expect the db error, but doesn't give a custom message
134-
{:error, changeset} =
135-
changeset
136-
|> Ecto.Changeset.check_constraint(:price, name: :positive_price)
137-
|> PoolRepo.insert()
138-
assert changeset.errors == [price: {"is invalid", [constraint: :check, constraint_name: "positive_price"]}]
139-
assert changeset.data.__meta__.state == :built
140-
141-
# When the changeset does expect the db error and gives a custom message
142-
changeset = Ecto.Changeset.change(%Constraint{}, price: -10)
143-
{:error, changeset} =
144-
changeset
145-
|> Ecto.Changeset.check_constraint(:price, name: :positive_price, message: "price must be greater than 0")
146-
|> PoolRepo.insert()
147-
assert changeset.errors == [price: {"price must be greater than 0", [constraint: :check, constraint_name: "positive_price"]}]
148-
assert changeset.data.__meta__.state == :built
149-
150-
# When the change does not violate the check constraint
151-
changeset = Ecto.Changeset.change(%Constraint{}, price: 10, from: 100, to: 200)
152-
{:ok, changeset} =
153-
changeset
154-
|> Ecto.Changeset.check_constraint(:price, name: :positive_price, message: "price must be greater than 0")
155-
|> PoolRepo.insert()
156-
assert is_integer(changeset.id)
15766
end
15867
end

lib/ecto/adapters/tds/connection.ex

-7
Original file line numberDiff line numberDiff line change
@@ -1040,20 +1040,13 @@ if Code.ensure_loaded?(Tds) do
10401040
error!(nil, msg)
10411041
end
10421042

1043-
def execute_ddl({:create, %Constraint{with: nocheck}})
1044-
when nocheck not in [nil, "NOCHECK"] do
1045-
error!(nil, ~s{Check constraint `:with` valid values are `nil` and "NOCHECK"})
1046-
end
1047-
10481043
def execute_ddl({:create, %Constraint{} = constraint}) do
1049-
with_nocheck = if_do(constraint.with != nil, [" WITH NOCHECK"])
10501044
table_name = quote_table(constraint.prefix, constraint.table)
10511045

10521046
[
10531047
[
10541048
"ALTER TABLE ",
10551049
table_name,
1056-
with_nocheck,
10571050
" ADD CONSTRAINT ",
10581051
quote_name(constraint.name),
10591052
" ",

lib/ecto/migration.ex

+2-6
Original file line numberDiff line numberDiff line change
@@ -362,10 +362,9 @@ defmodule Ecto.Migration do
362362
363363
To define a constraint in a migration, see `Ecto.Migration.constraint/3`.
364364
"""
365-
defstruct name: nil, table: nil, check: nil, exclude: nil, prefix: nil, comment: nil, with: nil
365+
defstruct name: nil, table: nil, check: nil, exclude: nil, prefix: nil, comment: nil
366366
@type t :: %__MODULE__{name: atom, table: String.t, prefix: atom | nil,
367-
check: String.t | nil, exclude: String.t | nil, comment: String.t | nil,
368-
with: String.t | nil}
367+
check: String.t | nil, exclude: String.t | nil, comment: String.t | nil}
369368
end
370369

371370
defmodule Command do
@@ -1116,9 +1115,6 @@ defmodule Ecto.Migration do
11161115
* `:check` - A check constraint expression. Required when creating a check constraint.
11171116
* `:exclude` - An exclusion constraint expression. Required when creating an exclusion constraint.
11181117
* `:prefix` - The prefix for the table.
1119-
* `:with` - Optional parameter for Tds adapter for create constrint.
1120-
Valid value is `"NOCHECK"` and it will prevent constraint checking
1121-
existing rows in the table, and allow for constrint to be added.
11221118
11231119
"""
11241120
def constraint(table, name, opts \\ [])

0 commit comments

Comments
 (0)