Skip to content

Commit cd7f585

Browse files
authored
Add ability to skip validation on postgres FK creation (#244)
Validation on FK constraints in postgres can cause large tables to stay locked for a relatively long time because it has to check all rows for valid data, while also preventing new entries. Postgres implemented a way to not validate the constraint immediately so the FK can be immediately inserted, improving concurrency, with the intention that a `VALIDATE CONSTRAINT` query will be following. The validation query uses a less expensive lock, because it only needs to check rows that existed before the constraint. This allows safer migrations, and also allows for users to clean up known violations while still enforcing the constraint for new entries. This commit implements the ability to skip validation, but it does not implement the validation query.
1 parent 462ae27 commit cd7f585

File tree

8 files changed

+41
-4
lines changed

8 files changed

+41
-4
lines changed

lib/ecto/adapters/myxql/connection.ex

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -854,6 +854,10 @@ if Code.ensure_loaded?(MyXQL) do
854854
intersperse_map(columns, ", ", &column_change(table, &1))
855855
end
856856

857+
defp column_change(_table, {_command, _name, %Reference{validate: false}, _opts}) do
858+
error!(nil, "validate: false on references is not supported in MyXQL")
859+
end
860+
857861
defp column_change(table, {:add, name, %Reference{} = ref, opts}) do
858862
["ADD ", quote_name(name), ?\s, reference_column_type(ref.type, opts),
859863
column_options(opts), constraint_expr(ref, table, name)]

lib/ecto/adapters/postgres/connection.ex

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1121,13 +1121,13 @@ if Code.ensure_loaded?(Postgrex) do
11211121
defp reference_expr(%Reference{} = ref, table, name),
11221122
do: [" CONSTRAINT ", reference_name(ref, table, name), " REFERENCES ",
11231123
quote_table(ref.prefix || table.prefix, ref.table), ?(, quote_name(ref.column), ?),
1124-
reference_on_delete(ref.on_delete), reference_on_update(ref.on_update)]
1124+
reference_on_delete(ref.on_delete), reference_on_update(ref.on_update), validate(ref.validate)]
11251125

11261126
defp constraint_expr(%Reference{} = ref, table, name),
11271127
do: [", ADD CONSTRAINT ", reference_name(ref, table, name), ?\s,
11281128
"FOREIGN KEY (", quote_name(name), ") REFERENCES ",
11291129
quote_table(ref.prefix || table.prefix, ref.table), ?(, quote_name(ref.column), ?),
1130-
reference_on_delete(ref.on_delete), reference_on_update(ref.on_update)]
1130+
reference_on_delete(ref.on_delete), reference_on_update(ref.on_update), validate(ref.validate)]
11311131

11321132
defp drop_constraint_expr(%Reference{} = ref, table, name),
11331133
do: ["DROP CONSTRAINT ", reference_name(ref, table, name), ", "]
@@ -1158,6 +1158,9 @@ if Code.ensure_loaded?(Postgrex) do
11581158
defp reference_on_update(:restrict), do: " ON UPDATE RESTRICT"
11591159
defp reference_on_update(_), do: []
11601160

1161+
defp validate(false), do: " NOT VALID"
1162+
defp validate(_), do: []
1163+
11611164
## Helpers
11621165

11631166
defp get_source(query, sources, ix, source) do

lib/ecto/adapters/tds/connection.ex

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1195,6 +1195,10 @@ if Code.ensure_loaded?(Tds) do
11951195
end
11961196
end
11971197

1198+
defp column_change(_statement_prefix, _table, {_command, _name, %Reference{validate: false}, _opts}) do
1199+
error!(nil, "validate: false on references is not supported in Tds")
1200+
end
1201+
11981202
defp column_change(statement_prefix, table, {:add, name, %Reference{} = ref, opts}) do
11991203
[
12001204
[

lib/ecto/migration.ex

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -370,8 +370,8 @@ defmodule Ecto.Migration do
370370
371371
To define a reference in a migration, see `Ecto.Migration.references/2`.
372372
"""
373-
defstruct name: nil, prefix: nil, table: nil, column: :id, type: :bigserial, on_delete: :nothing, on_update: :nothing
374-
@type t :: %__MODULE__{table: String.t, prefix: atom | nil, column: atom, type: atom, on_delete: atom, on_update: atom}
373+
defstruct name: nil, prefix: nil, table: nil, column: :id, type: :bigserial, on_delete: :nothing, on_update: :nothing, validate: true
374+
@type t :: %__MODULE__{table: String.t, prefix: atom | nil, column: atom, type: atom, on_delete: atom, on_update: atom, validate: boolean}
375375
end
376376

377377
defmodule Constraint do
@@ -1094,6 +1094,9 @@ defmodule Ecto.Migration do
10941094
`:nothing` (default), `:delete_all`, `:nilify_all`, or `:restrict`.
10951095
* `:on_update` - What to do if the referenced entry is updated. May be
10961096
`:nothing` (default), `:update_all`, `:nilify_all`, or `:restrict`.
1097+
* `:validate` - Whether or not to validate the foreign key constraint on
1098+
creation or not. Only available in PostgreSQL, and should be followed by
1099+
a command to validate the foreign key in a following migration if false.
10971100
10981101
"""
10991102
def references(table, opts \\ [])

test/ecto/adapters/myxql_test.exs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1283,6 +1283,14 @@ defmodule Ecto.Adapters.MyXQLTest do
12831283
""" |> remove_newlines]
12841284
end
12851285

1286+
test "alter table with invalid reference opts" do
1287+
alter = {:alter, table(:posts), [{:add, :author_id, %Reference{table: :author, validate: false}, []}]}
1288+
1289+
assert_raise ArgumentError, "validate: false on references is not supported in MyXQL", fn ->
1290+
execute_ddl(alter)
1291+
end
1292+
end
1293+
12861294
test "create index" do
12871295
create = {:create, index(:posts, [:category_id, :permalink])}
12881296
assert execute_ddl(create) ==

test/ecto/adapters/postgres_test.exs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1435,6 +1435,7 @@ defmodule Ecto.Adapters.PostgresTest do
14351435
alter = {:alter, table(:posts),
14361436
[{:add, :title, :string, [default: "Untitled", size: 100, null: false]},
14371437
{:add, :author_id, %Reference{table: :author}, []},
1438+
{:add, :category_id, %Reference{table: :categories, validate: false}, []},
14381439
{:add_if_not_exists, :subtitle, :string, [size: 100, null: false]},
14391440
{:add_if_not_exists, :editor_id, %Reference{table: :editor}, []},
14401441
{:modify, :price, :numeric, [precision: 8, scale: 2, null: true]},
@@ -1453,6 +1454,7 @@ defmodule Ecto.Adapters.PostgresTest do
14531454
ALTER TABLE "posts"
14541455
ADD COLUMN "title" varchar(100) DEFAULT 'Untitled' NOT NULL,
14551456
ADD COLUMN "author_id" bigint CONSTRAINT "posts_author_id_fkey" REFERENCES "author"("id"),
1457+
ADD COLUMN "category_id" bigint CONSTRAINT "posts_category_id_fkey" REFERENCES "categories"("id") NOT VALID,
14561458
ADD COLUMN IF NOT EXISTS "subtitle" varchar(100) NOT NULL,
14571459
ADD COLUMN IF NOT EXISTS "editor_id" bigint CONSTRAINT "posts_editor_id_fkey" REFERENCES "editor"("id"),
14581460
ALTER COLUMN "price" TYPE numeric(8,2),

test/ecto/adapters/tds_test.exs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1214,6 +1214,14 @@ defmodule Ecto.Adapters.TdsTest do
12141214
]
12151215
end
12161216

1217+
test "alter table with invalid reference opts" do
1218+
alter = {:alter, table(:posts), [{:add, :author_id, %Reference{table: :author, validate: false}, []}]}
1219+
1220+
assert_raise ArgumentError, "validate: false on references is not supported in Tds", fn ->
1221+
execute_ddl(alter)
1222+
end
1223+
end
1224+
12171225
test "create check constraint" do
12181226
create = {:create, constraint(:products, "price_must_be_positive", check: "price > 0")}
12191227

test/ecto/migration_test.exs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,11 @@ defmodule Ecto.MigrationTest do
100100
%Reference{table: "posts", column: :id, type: :binary_id}
101101
end
102102

103+
test "creates a reference without validating" do
104+
assert references(:posts, validate: false) ==
105+
%Reference{table: "posts", column: :id, type: :bigserial, validate: false}
106+
end
107+
103108
test "creates a constraint" do
104109
assert constraint(:posts, :price_is_positive, check: "price > 0") ==
105110
%Constraint{table: "posts", name: :price_is_positive, check: "price > 0"}

0 commit comments

Comments
 (0)