diff --git a/integration_test/sql/migration.exs b/integration_test/sql/migration.exs index 582078ee..a0a4042a 100644 --- a/integration_test/sql/migration.exs +++ b/integration_test/sql/migration.exs @@ -76,15 +76,17 @@ defmodule Ecto.Integration.MigrationTest do def change do create table(:modify_from_products) do add :value, :integer + add :nullable, :integer, null: false end if direction() == :up do flush() - PoolRepo.insert_all "modify_from_products", [[value: 1]] + PoolRepo.insert_all "modify_from_products", [[value: 1, nullable: 1]] end alter table(:modify_from_products) do modify :value, :bigint, from: :integer + modify :nullable, :bigint, null: true, from: {:integer, null: false} end end end diff --git a/lib/ecto/migration.ex b/lib/ecto/migration.ex index 0713380f..c4dc1af7 100644 --- a/lib/ecto/migration.ex +++ b/lib/ecto/migration.ex @@ -1035,10 +1035,12 @@ defmodule Ecto.Migration do Modifies the type of a column when altering a table. This command is not reversible unless the `:from` option is provided. - If the `:from` value is a `%Reference{}`, the adapter will try to drop + When the `:from` option is set, the adapter will try to drop the corresponding foreign key constraints before modifying the type. - Note `:from` cannot be used to modify primary keys, as those are - generally trickier to make reversible. + Generally speaking, you want to pass the type and each option + you are modifying to `:from`, so the column can be rolled back properly. + However, note that `:from` cannot be be used to modify primary keys, + as those are generally trickier to revert. See `add/3` for more information on supported types. @@ -1054,11 +1056,21 @@ defmodule Ecto.Migration do modify :title, :text end + # Self rollback when using the :from option + alter table("posts") do + modify :title, :text, from: :string + end + + # Modify column with rollback options + alter table("posts") do + modify :title, :text, null: false, from: {:string, null: true} + end + ## Options * `:null` - determines whether the column accepts null values. * `:default` - changes the default value of the column. - * `:from` - specifies the current type of the column. + * `:from` - specifies the current type and options of the column. * `:size` - specifies the size of the type (for example, the number of characters). The default is no size. * `:precision` - the precision for a numeric type. Required when `:scale` is diff --git a/lib/ecto/migration/runner.ex b/lib/ecto/migration/runner.ex index 1381b2eb..3de10ce0 100644 --- a/lib/ecto/migration/runner.ex +++ b/lib/ecto/migration/runner.ex @@ -253,8 +253,17 @@ defmodule Ecto.Migration.Runner do end defp table_reverse([{:modify, name, type, opts} | t], acc) do case opts[:from] do - nil -> false - from -> table_reverse(t, [{:modify, name, from, Keyword.put(opts, :from, type)} | acc]) + nil -> + false + + {reverse_type, from_opts} when is_list(from_opts) -> + reverse_from = {type, Keyword.delete(opts, :from)} + reverse_opts = Keyword.put(from_opts, :from, reverse_from) + table_reverse(t, [{:modify, name, reverse_type, reverse_opts} | acc]) + + reverse_type -> + reverse_opts = Keyword.put(opts, :from, type) + table_reverse(t, [{:modify, name, reverse_type, reverse_opts} | acc]) end end defp table_reverse([{:add, name, _type, _opts} | t], acc) do diff --git a/test/ecto/adapters/myxql_test.exs b/test/ecto/adapters/myxql_test.exs index e34d645f..f384bd97 100644 --- a/test/ecto/adapters/myxql_test.exs +++ b/test/ecto/adapters/myxql_test.exs @@ -1335,6 +1335,7 @@ defmodule Ecto.Adapters.MyXQLTest do {:modify, :status, :string, from: :integer}, {:modify, :user_id, :integer, from: %Reference{table: :users}}, {:modify, :group_id, %Reference{table: :groups, column: :gid}, from: %Reference{table: :groups}}, + {:modify, :status, :string, [null: false, size: 100, from: {:integer, null: true, size: 50}]}, {:remove, :summary}, {:remove, :body, :text, []}, {:remove, :space_id, %Reference{table: :author}, []}, @@ -1357,6 +1358,7 @@ defmodule Ecto.Adapters.MyXQLTest do DROP FOREIGN KEY `posts_group_id_fkey`, MODIFY `group_id` BIGINT UNSIGNED, ADD CONSTRAINT `posts_group_id_fkey` FOREIGN KEY (`group_id`) REFERENCES `groups`(`gid`), + MODIFY `status` varchar(100) NOT NULL, DROP `summary`, DROP `body`, DROP FOREIGN KEY `posts_space_id_fkey`, diff --git a/test/ecto/adapters/postgres_test.exs b/test/ecto/adapters/postgres_test.exs index e2ac22de..29e8f01d 100644 --- a/test/ecto/adapters/postgres_test.exs +++ b/test/ecto/adapters/postgres_test.exs @@ -1595,6 +1595,7 @@ defmodule Ecto.Adapters.PostgresTest do {:modify, :status, :string, from: :integer}, {:modify, :user_id, :integer, from: %Reference{table: :users}}, {:modify, :group_id, %Reference{table: :groups, column: :gid}, from: %Reference{table: :groups}}, + {:modify, :status, :string, [null: false, size: 100, from: {:integer, null: true, size: 50}]}, {:remove, :summary}, {:remove, :body, :text, []}, {:remove, :space_id, %Reference{table: :author}, []}, @@ -1625,6 +1626,8 @@ defmodule Ecto.Adapters.PostgresTest do DROP CONSTRAINT "posts_group_id_fkey", ALTER COLUMN "group_id" TYPE bigint, ADD CONSTRAINT "posts_group_id_fkey" FOREIGN KEY ("group_id") REFERENCES "groups"("gid"), + ALTER COLUMN "status" TYPE varchar(100), + ALTER COLUMN "status" SET NOT NULL, DROP COLUMN "summary", DROP COLUMN "body", DROP CONSTRAINT "posts_space_id_fkey", diff --git a/test/ecto/migration_test.exs b/test/ecto/migration_test.exs index a64b87d8..ee12fc88 100644 --- a/test/ecto/migration_test.exs +++ b/test/ecto/migration_test.exs @@ -718,12 +718,18 @@ defmodule Ecto.MigrationTest do alter table(:posts) do add :summary, :text modify :extension, :text, from: :string + modify :author, :string, null: false, from: :text + modify :title, :string, null: false, size: 100, from: {:text, null: true, size: 255} end flush() assert last_command() == - {:alter, %Table{name: "posts"}, - [{:modify, :extension, :string, from: :text}, {:remove, :summary}]} + {:alter, %Table{name: "posts"}, [ + {:modify, :title, :text, [from: {:string, null: false, size: 100}, null: true, size: 255]}, + {:modify, :author, :text, [from: :string, null: false]}, + {:modify, :extension, :string, from: :text}, + {:remove, :summary} + ]} assert_raise Ecto.MigrationError, ~r/cannot reverse migration command/, fn -> alter table(:posts) do