Support composite foreign keys with match

integration_test/sql/migration.exs

@@ -133,10 +133,6 @@ defmodule Ecto.Integration.MigrationTest do
       alter table(:alter_fk_posts) do
         modify :alter_fk_user_id, references(:alter_fk_users, on_delete: :nilify_all)
       end
-
-      execute "INSERT INTO alter_fk_users (id) VALUES ('1')"
-      execute "INSERT INTO alter_fk_posts (id, alter_fk_user_id) VALUES ('1', '1')"
-      execute "DELETE FROM alter_fk_users"
     end
 
     def down do
@@ -158,10 +154,6 @@ defmodule Ecto.Integration.MigrationTest do
       alter table(:alter_fk_posts) do
         modify :alter_fk_user_id, references(:alter_fk_users, on_update: :update_all)
       end
-
-      execute "INSERT INTO alter_fk_users (id) VALUES ('1')"
-      execute "INSERT INTO alter_fk_posts (id, alter_fk_user_id) VALUES ('1', '1')"
-      execute "UPDATE alter_fk_users SET id = '2'"
     end
 
     def down do
@@ -232,6 +224,23 @@ defmodule Ecto.Integration.MigrationTest do
     end
   end
 
+  defmodule CompositeForeignKeyMigration do
+    use Ecto.Migration
+
+    def change do
+      create table(:composite_parent) do
+        add :key_id, :integer
+      end
+
+      create unique_index(:composite_parent, [:id, :key_id])
+
+      create table(:composite_child) do
+        add :parent_key_id, :integer
+        add :parent_id, references(:composite_parent, with: [parent_key_id: :key_id])
+      end
+    end
+  end
+
   defmodule ReferencesRollbackMigration do
     use Ecto.Migration
 
@@ -431,7 +440,7 @@ defmodule Ecto.Integration.MigrationTest do
     assert :ok == down(PoolRepo, num, InferredDropIndexMigration, log: false)
   end
 
-  test "supports references", %{migration_number: num} do
+  test "supports on delete", %{migration_number: num} do
     assert :ok == up(PoolRepo, num, OnDeleteMigration, log: false)
 
     parent1 = PoolRepo.insert! Ecto.put_meta(%Parent{}, source: "parent1")
@@ -452,6 +461,18 @@ defmodule Ecto.Integration.MigrationTest do
     assert :ok == down(PoolRepo, num, OnDeleteMigration, log: false)
   end
 
+  test "composite foreign keys", %{migration_number: num} do
+    assert :ok == up(PoolRepo, num, CompositeForeignKeyMigration, log: false)
+
+    PoolRepo.insert_all("composite_parent", [[key_id: 2]])
+    assert [id] = PoolRepo.all(from p in "composite_parent", select: p.id)
+
+    catch_error(PoolRepo.insert_all("composite_child", [[parent_id: id, parent_key_id: 1]]))
+    assert {1, nil} = PoolRepo.insert_all("composite_child", [[parent_id: id, parent_key_id: 2]])
+
+    assert :ok == down(PoolRepo, num, CompositeForeignKeyMigration, log: false)
+  end
+
   test "rolls back references in change/1", %{migration_number: num} do
     assert :ok == up(PoolRepo, num, ReferencesRollbackMigration, log: false)
     assert :ok == down(PoolRepo, num, ReferencesRollbackMigration, log: false)
@@ -498,7 +519,6 @@ defmodule Ecto.Integration.MigrationTest do
     :ok = down(PoolRepo, num, AlterColumnMigration, log: false)
   end
 
-  @tag :modify_column_with_from
   test "modify column with from", %{migration_number: num} do
     assert :ok == up(PoolRepo, num, AlterColumnFromMigration, log: false)
 
@@ -508,9 +528,8 @@ defmodule Ecto.Integration.MigrationTest do
     :ok = down(PoolRepo, num, AlterColumnFromMigration, log: false)
   end
 
-  @tag :modify_column_with_from
   @tag :alter_primary_key
-  test "modify column with from andd pkey", %{migration_number: num} do
+  test "modify column with from and pkey", %{migration_number: num} do
     assert :ok == up(PoolRepo, num, AlterColumnFromPkeyMigration, log: false)
 
     assert [1] ==
@@ -519,17 +538,31 @@ defmodule Ecto.Integration.MigrationTest do
     :ok = down(PoolRepo, num, AlterColumnFromPkeyMigration, log: false)
   end
 
-  @tag :modify_foreign_key_on_delete
   test "modify foreign key's on_delete constraint", %{migration_number: num} do
     assert :ok == up(PoolRepo, num, AlterForeignKeyOnDeleteMigration, log: false)
+
+    PoolRepo.insert_all("alter_fk_users", [[]])
+    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.delete_all("alter_fk_users")
     assert [nil] == PoolRepo.all from p in "alter_fk_posts", select: p.alter_fk_user_id
+
     :ok = down(PoolRepo, num, AlterForeignKeyOnDeleteMigration, log: false)
   end
 
-  @tag :modify_foreign_key_on_update
+  @tag :assigns_id_type
   test "modify foreign key's on_update constraint", %{migration_number: num} do
     assert :ok == up(PoolRepo, num, AlterForeignKeyOnUpdateMigration, log: false)
-    assert [2] == PoolRepo.all from p in "alter_fk_posts", select: p.alter_fk_user_id
+
+    PoolRepo.insert_all("alter_fk_users", [[]])
+    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.update_all("alter_fk_users", set: [id: 12345])
+    assert [12345] == PoolRepo.all from p in "alter_fk_posts", select: p.alter_fk_user_id
+
+    PoolRepo.delete_all("alter_fk_posts")
     :ok = down(PoolRepo, num, AlterForeignKeyOnUpdateMigration, log: false)
   end
 

integration_test/sql/transaction.exs

@@ -100,7 +100,7 @@ defmodule Ecto.Integration.TransactionTest do
     end
   end
 
-  @tag :pk_insert
+  @tag :assigns_id_type
   test "transaction rolls back with reason on aborted transaction" do
     e1 = PoolRepo.insert!(%Trans{num: 13})
 

integration_test/tds/test_helper.exs

@@ -8,17 +8,12 @@ ExUnit.start(
     :subquery_aggregates,
     # sql don't have array type
     :array_type,
-    # I'm not sure if this is even possible if we SET IDENTITY_INSERT ON
-    :modify_foreign_key_on_update,
-    :modify_foreign_key_on_delete,
-    # NEXT 4 exclusions: can only be supported with MERGE statement and it is tricky to make it fast
-    :with_conflict_target,
-    :without_conflict_target,
-    :upsert_all,
+    # upserts can only be supported with MERGE statement and it is tricky to make it fast
     :upsert,
-    # I'm not sure why, but if decimal is passed as parameter mssql server will round differently ecto/integration_test/cases/interval.exs:186
+    :upsert_all,
+    # mssql rounds differently than ecto/integration_test/cases/interval.exs
     :uses_msec,
-    # Unique index compares even NULL values for post_id, so below fails inserting permalinks without setting valid post_id
+    # unique index compares even NULL values for post_id, so below fails inserting permalinks without setting valid post_id
     :insert_cell_wise_defaults,
     # MSSQL does not support strings on text fields
     :text_type_as_string,
@@ -45,10 +40,6 @@ ExUnit.start(
     # But I couldn't make it work :(
     :on_replace_nilify,
     :on_replace_update,
-    # This can't be executed since it requires
-    # `SET IDENTITY_INSERT [ [ database_name . ] schema_name . ] table_name  ON`
-    # and after insert we need to turn it on, must be run manually in transaction
-    :pk_insert,
     # Tds allows nested transactions so this will never raise and SQL query should be "BEGIN TRAN"
     :transaction_checkout_raises,
     # JSON_VALUE always returns strings (even for e.g. integers) and returns null for

lib/ecto/adapters/myxql/connection.ex

@@ -141,7 +141,7 @@ if Code.ensure_loaded?(MyXQL) do
 
     @impl true
     def insert(prefix, table, header, rows, on_conflict, []) do
-      fields = intersperse_map(header, ?,, &quote_name/1)
+      fields = quote_names(header)
       ["INSERT INTO ", quote_table(prefix, table), " (", fields, ") VALUES ",
        insert_all(rows) | on_conflict(on_conflict, header)]
     end
@@ -794,7 +794,7 @@ if Code.ensure_loaded?(MyXQL) do
 
       case pks do
         [] -> []
-        _  -> [[prefix, "PRIMARY KEY (", intersperse_map(pks, ", ", &quote_name/1), ?)]]
+        _  -> [[prefix, "PRIMARY KEY (", quote_names(pks), ?)]]
       end
     end
 
@@ -923,26 +923,28 @@ if Code.ensure_loaded?(MyXQL) do
       end
     end
 
-    defp constraint_expr(%Reference{} = ref, table, name),
-      do: [", ADD CONSTRAINT ", reference_name(ref, table, name),
-           " FOREIGN KEY (", quote_name(name), ?),
-           " REFERENCES ", quote_table(ref.prefix || table.prefix, ref.table),
-           ?(, quote_name(ref.column), ?),
-           reference_on_delete(ref.on_delete), reference_on_update(ref.on_update)]
+    defp reference_expr(type, ref, table, name) do
+      {current_columns, reference_columns} = Enum.unzip([{name, ref.column} | ref.with])
 
-    defp constraint_if_not_exists_expr(%Reference{} = ref, table, name),
-      do: [", ADD CONSTRAINT ", reference_name(ref, table, name),
-           " FOREIGN KEY IF NOT EXISTS (", quote_name(name), ?),
-           " REFERENCES ", quote_table(ref.prefix || table.prefix, ref.table),
-           ?(, quote_name(ref.column), ?),
-           reference_on_delete(ref.on_delete), reference_on_update(ref.on_update)]
+      if ref.match do
+        error!(nil, ":match is not supported in references for tds")
+      end
+
+      ["CONSTRAINT ", reference_name(ref, table, name),
+       " ", type, " (", quote_names(current_columns), ?),
+       " REFERENCES ", quote_table(ref.prefix || table.prefix, ref.table),
+       ?(, quote_names(reference_columns), ?),
+       reference_on_delete(ref.on_delete), reference_on_update(ref.on_update)]
+    end
 
     defp reference_expr(%Reference{} = ref, table, name),
-      do: [", CONSTRAINT ", reference_name(ref, table, name),
-           " FOREIGN KEY (", quote_name(name), ?),
-           " REFERENCES ", quote_table(ref.prefix || table.prefix, ref.table),
-           ?(, quote_name(ref.column), ?),
-           reference_on_delete(ref.on_delete), reference_on_update(ref.on_update)]
+      do: [", " | reference_expr("FOREIGN KEY", ref, table, name)]
+
+    defp constraint_expr(%Reference{} = ref, table, name),
+      do: [", ADD " | reference_expr("FOREIGN KEY", ref, table, name)]
+
+    defp constraint_if_not_exists_expr(%Reference{} = ref, table, name),
+      do: [", ADD " | reference_expr("FOREIGN KEY IF NOT EXISTS", ref, table, name)]
 
     defp drop_constraint_expr(%Reference{} = ref, table, name),
       do: ["DROP FOREIGN KEY ", reference_name(ref, table, name), ", "]
@@ -980,9 +982,9 @@ if Code.ensure_loaded?(MyXQL) do
       {expr || expr(source, sources, query), name}
     end
 
-    defp quote_name(name)
     defp quote_name(name) when is_atom(name),
       do: quote_name(Atom.to_string(name))
+
     defp quote_name(name) do
       if String.contains?(name, "`") do
         error!(nil, "bad field name #{inspect name}")
@@ -991,6 +993,8 @@ if Code.ensure_loaded?(MyXQL) do
       [?`, name, ?`]
     end
 
+    defp quote_names(names), do: intersperse_map(names, ?,, &quote_name/1)
+
     defp quote_table(nil, name),    do: quote_table(name)
     defp quote_table(prefix, name), do: [quote_table(prefix), ?., quote_table(name)]