Don't add pk on remove migration (#571)

Author: greg-rychlewski

lib/ecto/adapters/myxql/connection.ex

@@ -1086,7 +1086,8 @@ if Code.ensure_loaded?(MyXQL) do
 
     defp pk_definitions(columns, prefix) do
       pks =
-        for {_, name, _, opts} <- columns,
+        for {action, name, _, opts} <- columns,
+            action != :remove,
             opts[:primary_key],
             do: name
 

lib/ecto/adapters/postgres/connection.ex

@@ -1340,7 +1340,8 @@ if Code.ensure_loaded?(Postgrex) do
 
     defp pk_definition(columns, prefix) do
       pks =
-        for {_, name, _, opts} <- columns,
+        for {action, name, _, opts} <- columns,
+            action != :remove,
             opts[:primary_key],
             do: name
 

lib/ecto/adapters/tds/connection.ex

@@ -1307,7 +1307,8 @@ if Code.ensure_loaded?(Tds) do
 
     defp pk_definitions(columns, prefix) do
       pks =
-        for {_, name, _, opts} <- columns,
+        for {action, name, _, opts} <- columns,
+            action != :remove,
             opts[:primary_key],
             do: name
 
@@ -1435,6 +1436,9 @@ if Code.ensure_loaded?(Tds) do
       [statement_prefix, "DROP COLUMN ", quote_name(name), "; "]
     end
 
+    defp column_change(statement_prefix, _table, {:remove, name, _type, _opts}),
+      do: [statement_prefix, "DROP COLUMN ", quote_name(name)]
+
     defp column_change(
            statement_prefix,
            %{name: table, prefix: prefix},

test/ecto/adapters/myxql_test.exs

@@ -2009,8 +2009,9 @@ defmodule Ecto.Adapters.MyXQLTest do
            ]
   end
 
-  test "alter table with primary key" do
-    alter = {:alter, table(:posts), [{:add, :my_pk, :serial, [primary_key: true]}]}
+  test "alter table add/3 with primary key" do
+    alter =
+      {:alter, table(:posts), [{:add, :my_pk, :serial, [primary_key: true]}]}
 
     assert execute_ddl(alter) == [
              """
@@ -2022,6 +2023,13 @@ defmodule Ecto.Adapters.MyXQLTest do
            ]
   end
 
+  test "alter table remove/3 with primary key" do
+    alter =
+      {:alter, table(:posts), [{:remove, :my_pk, :serial, [primary_key: true]}]}
+
+    assert execute_ddl(alter) == ["ALTER TABLE `posts` DROP `my_pk`" |> remove_newlines]
+  end
+
   test "alter table with invalid reference opts" do
     alter =
       {:alter, table(:posts),

test/ecto/adapters/postgres_test.exs

@@ -2542,6 +2542,15 @@ defmodule Ecto.Adapters.PostgresTest do
            ]
   end
 
+  test "alter table remove/3 with primary key" do
+    alter =
+      {:alter, table(:posts), [{:remove, :my_pk, :serial, [primary_key: true]}]}
+
+    assert execute_ddl(alter) == [
+             "ALTER TABLE \"posts\" DROP COLUMN \"my_pk\"" |> remove_newlines
+           ]
+  end
+
   test "create index" do
     create = {:create, index(:posts, [:category_id, :permalink])}
 

test/ecto/adapters/tds_test.exs

@@ -1646,6 +1646,13 @@ defmodule Ecto.Adapters.TdsTest do
              ]
   end
 
+  test "alter table remove/3 with primary key" do
+    alter =
+      {:alter, table(:posts), [{:remove, :my_pk, :serial, [primary_key: true]}]}
+
+    assert execute_ddl(alter) == ["ALTER TABLE [posts] DROP COLUMN [my_pk]" |> remove_newlines]
+  end
+
   test "alter table with invalid reference opts" do
     alter =
       {:alter, table(:posts),