From 70a375f3b85fa556cfcf4874cc04622878e391f3 Mon Sep 17 00:00:00 2001 From: Milton Mazzarri Date: Thu, 17 Oct 2024 16:31:22 -0500 Subject: [PATCH 1/7] Do not modify column type while modifying fk constraint The following syntax: ```elixir alter table(:my_table) do modify :parent_id, references(:my_table, type: :binary_id, on_delete: :nilify_all, validate: false), from: {:binary_id, null: true} end ``` Produces a SQL equivalent like this: ```sql ALTER TABLE "my_table" ALTER COLUMN "parent_id" TYPE uuid, ADD CONSTRAINT "my_table_parent_id_fkey" FOREIGN KEY ("parent_id") REFERENCES "my_table"("id") ON DELETE SET NULL NOT VALID ``` The problem with the first directive, `ALTER COLUMN`, is that even the type is the same, at least in PostgreSQL, is creating bunch of `AccessExclusiveLock` (the most restrictive one) on each of the indexes associated to that field, and the table itself, when that's not needed at all. For example: ```console test_dev=# BEGIN; LOCK TABLE "schema_migrations" IN SHARE UPDATE EXCLUSIVE MODE; ALTER TABLE "my_table" ALTER COLUMN "parent_id" TYPE uuid, ADD CONSTRAINT "my_table_parent_id_fkey" FOREIGN KEY ("parent_id") REFERENCES "my_table"("id") ON DELETE SET NULL NOT VALID; INSERT INTO "schema_migrations" ("version","inserted_at") VALUES ('20210718210952',NOW()); SELECT locktype, relation::regclass, mode, transactionid AS tid, virtualtransaction AS vtid, pid, granted FROM pg_locks; COMMIT; BEGIN LOCK TABLE ALTER TABLE INSERT 0 1 locktype | relation | mode | tid | vtid | pid | granted ---------------+-----------------------------------------------------+--------------------------+---------+---------+-------+--------- relation | schema_migrations | RowExclusiveLock | | 4/14045 | 16813 | t virtualxid | | ExclusiveLock | | 4/14045 | 16813 | t relation | my_table_parent_id_ppppppp_id_index | AccessExclusiveLock | | 4/14045 | 16813 | t relation | pg_locks | AccessShareLock | | 4/14045 | 16813 | t relation | 134438 | AccessShareLock | | 4/14045 | 16813 | t relation | 134438 | AccessExclusiveLock | | 4/14045 | 16813 | t relation | my_table_parent_id_rrrrr_index | AccessExclusiveLock | | 4/14045 | 16813 | t relation | my_table | AccessShareLock | | 4/14045 | 16813 | t relation | my_table | ShareLock | | 4/14045 | 16813 | t relation | my_table | ShareRowExclusiveLock | | 4/14045 | 16813 | t relation | my_table | AccessExclusiveLock | | 4/14045 | 16813 | t relation | my_table_project_id_xxxx_index | AccessExclusiveLock | | 4/14045 | 16813 | t relation | 135159 | AccessShareLock | | 4/14045 | 16813 | t relation | 135159 | AccessExclusiveLock | | 4/14045 | 16813 | t relation | 132875 | AccessShareLock | | 4/14045 | 16813 | t relation | 132875 | AccessExclusiveLock | | 4/14045 | 16813 | t relation | 132507 | AccessExclusiveLock | | 4/14045 | 16813 | t relation | 132506 | AccessShareLock | | 4/14045 | 16813 | t relation | 132506 | AccessExclusiveLock | | 4/14045 | 16813 | t relation | schema_migrations | ShareUpdateExclusiveLock | | 4/14045 | 16813 | t relation | 132504 | AccessShareLock | | 4/14045 | 16813 | t relation | 132504 | AccessExclusiveLock | | 4/14045 | 16813 | t relation | 135184 | AccessShareLock | | 4/14045 | 16813 | t relation | 135184 | AccessExclusiveLock | | 4/14045 | 16813 | t relation | 132505 | AccessShareLock | | 4/14045 | 16813 | t relation | 132505 | AccessExclusiveLock | | 4/14045 | 16813 | t relation | my_table_parent_id_ttttt_index | AccessExclusiveLock | | 4/14045 | 16813 | t transactionid | | ExclusiveLock | 2175345 | 4/14045 | 16813 | t relation | my_table_parent_id_aaaaaaaaaaa_id_index | AccessExclusiveLock | | 4/14045 | 16813 | t relation | 132503 | AccessShareLock | | 4/14045 | 16813 | t relation | 132503 | AccessExclusiveLock | | 4/14045 | 16813 | t relation | my_table_parent_id_gggg_index | AccessExclusiveLock | | 4/14045 | 16813 | t (35 rows) COMMIT ``` So, the proposal is to avoid setting the type when you pass a `references` to `modify`, if there is any type incompatibility, PostgreSQL, will let you know about it right away: ``` ERROR: foreign key constraint "my_table_parent_id_fkey" cannot be implemented DETAIL: Key columns "parent_id" and "id" are of incompatible types: character varying and uuid. ``` Instead, the resulting SQL syntax should be something like: ```sql ALTER TABLE "my_table" ADD CONSTRAINT "my_table_parent_id_fkey" FOREIGN KEY ("parent_id") REFERENCES "my_table"("id") ON DELETE SET NULL NOT VALID ``` That will produce more manageable lock types: ```console locktype | relation | mode | tid | vtid | pid | granted ---------------+-------------------+--------------------------+---------+---------+-------+--------- relation | pg_locks | AccessShareLock | | 4/14540 | 16849 | t relation | schema_migrations | RowExclusiveLock | | 4/14540 | 16849 | t virtualxid | | ExclusiveLock | | 4/14540 | 16849 | t transactionid | | ExclusiveLock | 2175967 | 4/14540 | 16849 | t relation | schema_migrations | ShareUpdateExclusiveLock | | 4/14540 | 16849 | t relation | my_table | AccessShareLock | | 4/14540 | 16849 | t relation | my_table | ShareRowExclusiveLock | | 4/14540 | 16849 | t (7 rows) ``` --- lib/ecto/adapters/postgres/connection.ex | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/ecto/adapters/postgres/connection.ex b/lib/ecto/adapters/postgres/connection.ex index 996e8e18..035d0ce2 100644 --- a/lib/ecto/adapters/postgres/connection.ex +++ b/lib/ecto/adapters/postgres/connection.ex @@ -1548,11 +1548,7 @@ if Code.ensure_loaded?(Postgrex) do defp column_change(table, {:modify, name, %Reference{} = ref, opts}) do [ drop_reference_expr(opts[:from], table, name), - "ALTER COLUMN ", - quote_name(name), - " TYPE ", - reference_column_type(ref.type, opts), - ", ADD ", + "ADD ", reference_expr(ref, table, name), modify_null(name, opts), modify_default(name, ref.type, opts) From 475cf8c0a35a0e587f7e0223dce17afd44968c47 Mon Sep 17 00:00:00 2001 From: Milton Mazzarri Date: Thu, 17 Oct 2024 18:19:36 -0500 Subject: [PATCH 2/7] avoid alter column type on modify if not needed --- integration_test/sql/migration.exs | 4 ++- lib/ecto/adapters/postgres/connection.ex | 43 ++++++++++++++++++------ 2 files changed, 35 insertions(+), 12 deletions(-) diff --git a/integration_test/sql/migration.exs b/integration_test/sql/migration.exs index fe4429c5..d0c122bd 100644 --- a/integration_test/sql/migration.exs +++ b/integration_test/sql/migration.exs @@ -48,6 +48,7 @@ defmodule Ecto.Integration.MigrationTest do def up do create table(:alter_col_migration) do add :from_null_to_not_null, :integer + add :another_from_null_to_not_null, :string add :from_not_null_to_null, :integer, null: false add :from_default_to_no_default, :integer, default: 0 @@ -56,13 +57,14 @@ defmodule Ecto.Integration.MigrationTest do alter table(:alter_col_migration) do modify :from_null_to_not_null, :string, null: false + modify :another_from_null_to_not_null, :string, null: false, from: {:string, null: true} modify :from_not_null_to_null, :string, null: true modify :from_default_to_no_default, :integer, default: nil modify :from_no_default_to_default, :integer, default: 0 end - execute "INSERT INTO alter_col_migration (from_null_to_not_null) VALUES ('foo')" + execute "INSERT INTO alter_col_migration (from_null_to_not_null, another_from_null_to_not_null) VALUES ('foo', 'baz')" end def down do diff --git a/lib/ecto/adapters/postgres/connection.ex b/lib/ecto/adapters/postgres/connection.ex index 035d0ce2..bc7673c1 100644 --- a/lib/ecto/adapters/postgres/connection.ex +++ b/lib/ecto/adapters/postgres/connection.ex @@ -1556,15 +1556,33 @@ if Code.ensure_loaded?(Postgrex) do end defp column_change(table, {:modify, name, type, opts}) do - [ - drop_reference_expr(opts[:from], table, name), - "ALTER COLUMN ", - quote_name(name), - " TYPE ", - column_type(type, opts), - modify_null(name, opts), - modify_default(name, type, opts) - ] + column_type = column_type(type, opts) + + extract_type = fn + {type, _} when is_atom(type) -> type + type when is_atom(type) -> type + _ -> nil + end + + from_type = extract_type.(opts[:from]) + + if column_type == column_type(from_type, opts) do + [ + drop_reference_expr(opts[:from], table, name), + modify_null(name, Keyword.put(opts, :prefix_with_comma, false)), + modify_default(name, type, opts) + ] + else + [ + drop_reference_expr(opts[:from], table, name), + "ALTER COLUMN ", + quote_name(name), + " TYPE ", + column_type, + modify_null(name, opts), + modify_default(name, type, opts) + ] + end end defp column_change(_table, {:remove, name}), do: ["DROP COLUMN ", quote_name(name)] @@ -1591,9 +1609,12 @@ if Code.ensure_loaded?(Postgrex) do do: ["DROP COLUMN IF EXISTS ", quote_name(name)] defp modify_null(name, opts) do + prefix_with_comma = Keyword.get(opts, :prefix_with_comma, true) + prefix = if prefix_with_comma, do: ", ", else: "" + case Keyword.get(opts, :null) do - true -> [", ALTER COLUMN ", quote_name(name), " DROP NOT NULL"] - false -> [", ALTER COLUMN ", quote_name(name), " SET NOT NULL"] + true -> [prefix, "ALTER COLUMN ", quote_name(name), " DROP NOT NULL"] + false -> [prefix, "ALTER COLUMN ", quote_name(name), " SET NOT NULL"] nil -> [] end end From fc2e173a161c22635dffc9279679dbc893ca30a6 Mon Sep 17 00:00:00 2001 From: Milton Mazzarri Date: Fri, 18 Oct 2024 12:01:31 -0500 Subject: [PATCH 3/7] should be smarter now? --- integration_test/sql/migration.exs | 11 +++- lib/ecto/adapters/postgres/connection.ex | 71 +++++++++++++++++------- 2 files changed, 60 insertions(+), 22 deletions(-) diff --git a/integration_test/sql/migration.exs b/integration_test/sql/migration.exs index d0c122bd..48cca503 100644 --- a/integration_test/sql/migration.exs +++ b/integration_test/sql/migration.exs @@ -134,12 +134,21 @@ defmodule Ecto.Integration.MigrationTest do add :alter_fk_user_id, :id end + create table(:alter_fk_comments) do + add :alter_fk_user_id, references(:alter_fk_users) + end + alter table(:alter_fk_posts) do - modify :alter_fk_user_id, references(:alter_fk_users, on_delete: :nilify_all) + modify :alter_fk_user_id, references(:alter_fk_users, on_delete: :nilify_all), from: {:id, null: true} + end + + alter table(:alter_fk_comments) do + modify :alter_fk_user_id, references(:alter_fk_users, on_delete: :delete_all), from: references(:alter_fk_users, on_delete: :nothing) end end def down do + drop table(:alter_fk_comments) drop table(:alter_fk_posts) drop table(:alter_fk_users) end diff --git a/lib/ecto/adapters/postgres/connection.ex b/lib/ecto/adapters/postgres/connection.ex index bc7673c1..3b14e402 100644 --- a/lib/ecto/adapters/postgres/connection.ex +++ b/lib/ecto/adapters/postgres/connection.ex @@ -1546,35 +1546,54 @@ if Code.ensure_loaded?(Postgrex) do end defp column_change(table, {:modify, name, %Reference{} = ref, opts}) do - [ - drop_reference_expr(opts[:from], table, name), - "ADD ", - reference_expr(ref, table, name), - modify_null(name, opts), - modify_default(name, ref.type, opts) - ] + reference_column_type = reference_column_type(ref.type, opts) + from_column_type = extract_column_type(opts[:from]) + + drop_reference_expr = drop_reference_expr(opts[:from], table, name) + prefix_with_comma = (drop_reference_expr != [] && ", ") || "" + + if reference_column_type == column_type(from_column_type, opts) do + [ + drop_reference_expr, + prefix_with_comma, + "ADD ", + reference_expr(ref, table, name), + modify_null(name, opts), + modify_default(name, ref.type, opts) + ] + else + [ + drop_reference_expr, + prefix_with_comma, + "ALTER COLUMN ", + quote_name(name), + " TYPE ", + reference_column_type, + ", ADD ", + reference_expr(ref, table, name), + modify_null(name, opts), + modify_default(name, ref.type, opts) + ] + end end defp column_change(table, {:modify, name, type, opts}) do column_type = column_type(type, opts) + from_column_type = extract_column_type(opts[:from]) - extract_type = fn - {type, _} when is_atom(type) -> type - type when is_atom(type) -> type - _ -> nil - end - - from_type = extract_type.(opts[:from]) + drop_reference_expr = drop_reference_expr(opts[:from], table, name) + prefix_with_comma? = drop_reference_expr != [] - if column_type == column_type(from_type, opts) do + if column_type == column_type(from_column_type, opts) do [ - drop_reference_expr(opts[:from], table, name), - modify_null(name, Keyword.put(opts, :prefix_with_comma, false)), + drop_reference_expr, + modify_null(name, Keyword.put(opts, :prefix_with_comma, prefix_with_comma?)), modify_default(name, type, opts) ] else [ - drop_reference_expr(opts[:from], table, name), + drop_reference_expr, + (prefix_with_comma? && ", ") || "", "ALTER COLUMN ", quote_name(name), " TYPE ", @@ -1588,7 +1607,9 @@ if Code.ensure_loaded?(Postgrex) do defp column_change(_table, {:remove, name}), do: ["DROP COLUMN ", quote_name(name)] defp column_change(table, {:remove, name, %Reference{} = ref, _opts}) do - [drop_reference_expr(ref, table, name), "DROP COLUMN ", quote_name(name)] + drop_reference_expr = drop_reference_expr(ref, table, name) + prefix_with_comma = (drop_reference_expr != [] && ", ") || "" + [drop_reference_expr, prefix_with_comma, "DROP COLUMN ", quote_name(name)] end defp column_change(_table, {:remove, name, _type, _opts}), @@ -1620,9 +1641,12 @@ if Code.ensure_loaded?(Postgrex) do end defp modify_default(name, type, opts) do + prefix_with_comma = Keyword.get(opts, :prefix_with_comma, true) + prefix = if prefix_with_comma, do: ", ", else: "" + case Keyword.fetch(opts, :default) do {:ok, val} -> - [", ALTER COLUMN ", quote_name(name), " SET", default_expr({:ok, val}, type)] + [prefix, "ALTER COLUMN ", quote_name(name), " SET", default_expr({:ok, val}, type)] :error -> [] @@ -1814,6 +1838,11 @@ if Code.ensure_loaded?(Postgrex) do [type, generated_expr(generated)] end + defp extract_column_type({type, _}) when is_atom(type), do: type + defp extract_column_type(type) when is_atom(type), do: type + defp extract_column_type(%Reference{type: type}), do: type + defp extract_column_type(_), do: nil + defp generated_expr(nil), do: [] defp generated_expr(expr) when is_binary(expr) do @@ -1850,7 +1879,7 @@ if Code.ensure_loaded?(Postgrex) do do: drop_reference_expr(ref, table, name) defp drop_reference_expr(%Reference{} = ref, table, name), - do: ["DROP CONSTRAINT ", reference_name(ref, table, name), ", "] + do: ["DROP CONSTRAINT ", reference_name(ref, table, name)] defp drop_reference_expr(_, _, _), do: [] From daeed39a32fd67c39b1850747a44a5416667b4d3 Mon Sep 17 00:00:00 2001 From: Milton Mazzarri Date: Mon, 21 Oct 2024 14:29:41 -0500 Subject: [PATCH 4/7] fix syntax error bug when setting default value --- integration_test/sql/migration.exs | 16 ++++++++++++++-- lib/ecto/adapters/postgres/connection.ex | 15 +++++++++++---- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/integration_test/sql/migration.exs b/integration_test/sql/migration.exs index 48cca503..ea7215d4 100644 --- a/integration_test/sql/migration.exs +++ b/integration_test/sql/migration.exs @@ -53,6 +53,9 @@ defmodule Ecto.Integration.MigrationTest do add :from_default_to_no_default, :integer, default: 0 add :from_no_default_to_default, :integer + + add :another_from_default_to_no_default, :integer, default: 0 + add :another_from_no_default_to_default, :integer end alter table(:alter_col_migration) do @@ -62,6 +65,9 @@ defmodule Ecto.Integration.MigrationTest do modify :from_default_to_no_default, :integer, default: nil modify :from_no_default_to_default, :integer, default: 0 + + modify :another_from_default_to_no_default, :integer, default: nil, from: {:integer, default: 0} + modify :another_from_no_default_to_default, :integer, default: 0, from: {:integer, default: nil} end execute "INSERT INTO alter_col_migration (from_null_to_not_null, another_from_null_to_not_null) VALUES ('foo', 'baz')" @@ -143,7 +149,7 @@ defmodule Ecto.Integration.MigrationTest do end alter table(:alter_fk_comments) do - modify :alter_fk_user_id, references(:alter_fk_users, on_delete: :delete_all), from: references(:alter_fk_users, on_delete: :nothing) + modify :alter_fk_user_id, references(:alter_fk_users, on_delete: :nilify_all), from: references(:alter_fk_users, on_delete: :nothing) end end @@ -566,12 +572,16 @@ defmodule Ecto.Integration.MigrationTest do assert ["foo"] == PoolRepo.all from p in "alter_col_migration", select: p.from_null_to_not_null + assert ["baz"] == + PoolRepo.all from p in "alter_col_migration", select: p.another_from_null_to_not_null assert [nil] == PoolRepo.all from p in "alter_col_migration", select: p.from_not_null_to_null assert [nil] == PoolRepo.all from p in "alter_col_migration", select: p.from_default_to_no_default + assert [nil] == + PoolRepo.all from p in "alter_col_migration", select: p.another_from_default_to_no_default assert [0] == - PoolRepo.all from p in "alter_col_migration", select: p.from_no_default_to_default + PoolRepo.all from p in "alter_col_migration", select: p.another_from_no_default_to_default query = "INSERT INTO `alter_col_migration` (\"from_not_null_to_null\") VALUES ('foo')" assert catch_error(PoolRepo.query!(query)) @@ -607,8 +617,10 @@ defmodule Ecto.Integration.MigrationTest do assert [id] = PoolRepo.all from p in "alter_fk_users", select: p.id PoolRepo.insert_all("alter_fk_posts", [[alter_fk_user_id: id]]) + PoolRepo.insert_all("alter_fk_comments", [[alter_fk_user_id: id]]) PoolRepo.delete_all("alter_fk_users") assert [nil] == PoolRepo.all from p in "alter_fk_posts", select: p.alter_fk_user_id + assert [nil] == PoolRepo.all from p in "alter_fk_comments", select: p.alter_fk_user_id :ok = down(PoolRepo, num, AlterForeignKeyOnDeleteMigration, log: false) end diff --git a/lib/ecto/adapters/postgres/connection.ex b/lib/ecto/adapters/postgres/connection.ex index 3b14e402..1567f3ed 100644 --- a/lib/ecto/adapters/postgres/connection.ex +++ b/lib/ecto/adapters/postgres/connection.ex @@ -1582,18 +1582,25 @@ if Code.ensure_loaded?(Postgrex) do from_column_type = extract_column_type(opts[:from]) drop_reference_expr = drop_reference_expr(opts[:from], table, name) - prefix_with_comma? = drop_reference_expr != [] + any_drop_ref? = drop_reference_expr != [] if column_type == column_type(from_column_type, opts) do + modify_null = modify_null(name, Keyword.put(opts, :prefix_with_comma, any_drop_ref?)) + any_modify_null? = modify_null != [] + [ drop_reference_expr, - modify_null(name, Keyword.put(opts, :prefix_with_comma, prefix_with_comma?)), - modify_default(name, type, opts) + modify_null, + modify_default( + name, + type, + Keyword.put(opts, :prefix_with_comma, any_drop_ref? or any_modify_null?) + ) ] else [ drop_reference_expr, - (prefix_with_comma? && ", ") || "", + (any_drop_ref? && ", ") || "", "ALTER COLUMN ", quote_name(name), " TYPE ", From 79e647952008ab2fa7215f4dba0a5fab6cd59616 Mon Sep 17 00:00:00 2001 From: Milton Mazzarri Date: Mon, 21 Oct 2024 15:09:01 -0500 Subject: [PATCH 5/7] minor refactor --- integration_test/sql/migration.exs | 2 ++ lib/ecto/adapters/postgres/connection.ex | 23 +++++++++++------------ 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/integration_test/sql/migration.exs b/integration_test/sql/migration.exs index ea7215d4..c4ea287e 100644 --- a/integration_test/sql/migration.exs +++ b/integration_test/sql/migration.exs @@ -580,6 +580,8 @@ defmodule Ecto.Integration.MigrationTest do PoolRepo.all from p in "alter_col_migration", select: p.from_default_to_no_default assert [nil] == PoolRepo.all from p in "alter_col_migration", select: p.another_from_default_to_no_default + assert [0] == + PoolRepo.all from p in "alter_col_migration", select: p.from_no_default_to_default assert [0] == PoolRepo.all from p in "alter_col_migration", select: p.another_from_no_default_to_default diff --git a/lib/ecto/adapters/postgres/connection.ex b/lib/ecto/adapters/postgres/connection.ex index 1567f3ed..ecb8f8d7 100644 --- a/lib/ecto/adapters/postgres/connection.ex +++ b/lib/ecto/adapters/postgres/connection.ex @@ -1552,14 +1552,17 @@ if Code.ensure_loaded?(Postgrex) do drop_reference_expr = drop_reference_expr(opts[:from], table, name) prefix_with_comma = (drop_reference_expr != [] && ", ") || "" + common_suffix = [ + reference_expr(ref, table, name), + modify_null(name, opts), + modify_default(name, ref.type, opts) + ] + if reference_column_type == column_type(from_column_type, opts) do [ drop_reference_expr, prefix_with_comma, - "ADD ", - reference_expr(ref, table, name), - modify_null(name, opts), - modify_default(name, ref.type, opts) + "ADD " | common_suffix ] else [ @@ -1569,10 +1572,7 @@ if Code.ensure_loaded?(Postgrex) do quote_name(name), " TYPE ", reference_column_type, - ", ADD ", - reference_expr(ref, table, name), - modify_null(name, opts), - modify_default(name, ref.type, opts) + ", ADD " | common_suffix ] end end @@ -1588,15 +1588,14 @@ if Code.ensure_loaded?(Postgrex) do modify_null = modify_null(name, Keyword.put(opts, :prefix_with_comma, any_drop_ref?)) any_modify_null? = modify_null != [] - [ - drop_reference_expr, - modify_null, + modify_default = modify_default( name, type, Keyword.put(opts, :prefix_with_comma, any_drop_ref? or any_modify_null?) ) - ] + + [drop_reference_expr, modify_null, modify_default] else [ drop_reference_expr, From 609d4c1c751997942f85f6a1b64ac1c492f80be6 Mon Sep 17 00:00:00 2001 From: Milton Mazzarri Date: Mon, 21 Oct 2024 16:14:19 -0500 Subject: [PATCH 6/7] Update docs and add a warning block for this function --- lib/ecto/migration.ex | 45 ++++++++++++++++++++++++++++++++++++------- 1 file changed, 38 insertions(+), 7 deletions(-) diff --git a/lib/ecto/migration.ex b/lib/ecto/migration.ex index c1f28e1c..68628e51 100644 --- a/lib/ecto/migration.ex +++ b/lib/ecto/migration.ex @@ -1273,13 +1273,44 @@ defmodule Ecto.Migration do See `add/3` for more information on supported types. - If you want to modify a column without changing its type, - such as adding or dropping a null constraints, consider using - the `execute/2` command with the relevant SQL command instead - of `modify/3`, if supported by your database. This may avoid - redundant type updates and be more efficient, as an unnecessary - type update can lock the table, even if the type actually - doesn't change. + > #### Modifying a column without changing its type {: .warning} + > + > If you want to modify a column without changing its type, + > such as adding or dropping a null constraints, consider using + > the `execute/2` command with the relevant SQL command instead + > of `modify/3`, if supported by your database. This may avoid + > redundant type updates and be more efficient, as an unnecessary + > type update can lock the table, even if the type actually + > doesn't change. + > + > We have considered changing the column type even when it is not needed + > could lead to undesirable locks, that's why, at least in the PostgreSQL + > adapter, if you provide the option `:from`, and the type matches, we + > will avoid changing the type. + > + > Examples + > + > # modify column with rollback options + > alter table("posts") do + > modify :title, :text, null: false, from: {:text, null: true} + > end + > + > # adding a new foreign key constraint + > alter table("posts") do + > modify :author_id, references(:authors, type: :id), from: :id + > end + > + > # Modify the :on_delete option of an existing foreign key + > alter table("comments") do + > modify :post_id, references(:posts, on_delete: :delete_all), + > from: references(:posts, on_delete: :nothing) + > end + > + > + > The previous syntax will offer two benefits, at least in the PostgreSQL adapter, + > the migration is reversible and if the column type remains the same, the column + > type update will be skipped. + ## Examples From 6b902901e17d80bdc13208a7f5380e4c0cd3119d Mon Sep 17 00:00:00 2001 From: Milton Mazzarri Date: Mon, 21 Oct 2024 16:35:04 -0500 Subject: [PATCH 7/7] minor doc update --- lib/ecto/migration.ex | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/lib/ecto/migration.ex b/lib/ecto/migration.ex index 68628e51..ae05a342 100644 --- a/lib/ecto/migration.ex +++ b/lib/ecto/migration.ex @@ -1285,8 +1285,8 @@ defmodule Ecto.Migration do > > We have considered changing the column type even when it is not needed > could lead to undesirable locks, that's why, at least in the PostgreSQL - > adapter, if you provide the option `:from`, and the type matches, we - > will avoid changing the type. + > adapter, if you provide the option `:from`, and the column type matches, + > we will skip updating it. > > Examples > @@ -1297,7 +1297,7 @@ defmodule Ecto.Migration do > > # adding a new foreign key constraint > alter table("posts") do - > modify :author_id, references(:authors, type: :id), from: :id + > modify :author_id, references(:authors, type: :id, validate: false), from: :id > end > > # Modify the :on_delete option of an existing foreign key @@ -1306,12 +1306,10 @@ defmodule Ecto.Migration do > from: references(:posts, on_delete: :nothing) > end > - > - > The previous syntax will offer two benefits, at least in the PostgreSQL adapter, - > the migration is reversible and if the column type remains the same, the column + > The previous syntax will offer two benefits, apart from being a reversible migration, + > at least in the PostgreSQL adapter, if the column type remains the same, the column > type update will be skipped. - ## Examples alter table("posts") do