Repository: elixir-ecto/ecto
Pull request #2420 (2018-02-13 19:16:03)

Issue #2418: Adds reversible version of remove subcommand
An attempt to fix [#2416](https://github.com/elixir-ecto/ecto/pull/2416) - add reversible version of remove migration subcommand

I might miss something since I haven't submitted to OSS in a while %-)

ebert[bot]: Hello, @bobishh! This is your first Pull Request that will be reviewed by [Ebert](https://ebertapp.io/), an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its [review page](https://ebertapp.io/github/elixir-ecto/ecto/pulls/2420). Please check our [documentation](https://docs.ebertapp.io) for more information. 
josevalim: Beautiful, I have added one tiny commit, other than this is perfect!

Patch:
diff --git a/lib/ecto/adapter/migration.ex b/lib/ecto/adapter/migration.ex
index 55b722bb..f2b424ab 100644
--- a/lib/ecto/adapter/migration.ex
+++ b/lib/ecto/adapter/migration.ex
@@ -24,6 +24,7 @@ defmodule Ecto.Adapter.Migration  do
   @type table_subcommand ::
     {:add, field :: atom, type :: Ecto.Type.t | Reference.t, Keyword.t} |
     {:modify, field :: atom, type :: Ecto.Type.t | Reference.t, Keyword.t} |
+    {:remove, field :: atom, type :: Ecto.Type.t | Reference.t, Keyword.t} |
     {:remove, field :: atom}
 
   @typedoc """
diff --git a/lib/ecto/migration.ex b/lib/ecto/migration.ex
index 6eab8378..ec2d9fcf 100644
--- a/lib/ecto/migration.ex
+++ b/lib/ecto/migration.ex
@@ -820,6 +820,10 @@ defmodule Ecto.Migration do
     Runner.subcommand {:remove, column}
   end
 
+  def remove(column, type, opts \\ []) when is_atom(column) do
+    Runner.subcommand {:remove, column, type, opts}
+  end
+
   @doc ~S"""
   Defines a foreign key.
 
diff --git a/lib/ecto/migration/runner.ex b/lib/ecto/migration/runner.ex
index 168dd0ae..95161b35 100644
--- a/lib/ecto/migration/runner.ex
+++ b/lib/ecto/migration/runner.ex
@@ -226,6 +226,10 @@ defmodule Ecto.Migration.Runner do
     do: {:drop, constraint}
   defp reverse(_command), do: false
 
+  defp table_reverse([{:remove, name, type, opts}| t], acc) do
+    table_reverse(t, [{:add, name, type, opts} | acc])
+  end
+
   defp table_reverse([{:add, name, _type, _opts} | t], acc) do
     table_reverse(t, [{:remove, name} | acc])
   end


Test patch:
diff --git a/test/ecto/migration_test.exs b/test/ecto/migration_test.exs
index ef7b2cb8..2711591e 100644
--- a/test/ecto/migration_test.exs
+++ b/test/ecto/migration_test.exs
@@ -565,6 +565,22 @@ defmodule Ecto.MigrationTest do
     end
   end
 
+  test "backward: removing a column (remove/3 called)" do
+    alter table(:posts) do
+      remove :title, :string, []
+    end
+    flush()
+    assert {:alter, %Table{name: "posts"}, [{:add, :title, :string, []}]} = last_command()
+  end
+
+  test "backward: removing a column (remove/2 called)" do
+    alter table(:posts) do
+      remove :title, :string
+    end
+    flush()
+    assert {:alter, %Table{name: "posts"}, [{:add, :title, :string, []}]} = last_command()
+  end
+
   test "backward: rename column" do
     rename table(:posts), :given_name, to: :first_name
     flush()