From 7478895a33518d8bacd63a587a1e58624ff989c2 Mon Sep 17 00:00:00 2001 From: Aggelos Karalias Date: Mon, 15 Dec 2025 18:09:27 +0200 Subject: [PATCH 1/2] MyXQL: Use INSERT IGNORE for on_conflict: :nothing The previous implementation used `ON DUPLICATE KEY UPDATE col = col` which had incorrect semantics: 1. It reported 1 row affected even when a row was skipped due to a duplicate key conflict, because the UPDATE clause still matched the existing row 2. This caused insert_all to return {1, nil} instead of {0, nil} for ignored duplicates, misrepresenting the actual number of inserted records 3. The UPDATE clause could trigger unnecessary row-level operations This change: - Uses `INSERT IGNORE INTO` which properly ignores duplicate key conflicts without affecting existing rows or incrementing the affected row count - Handles num_rows: 0 in the adapter by returning {:ok, []} for on_conflict: :nothing, since 0 rows is expected behavior when all rows are duplicates - Updates tests to verify correct row counts: {0, nil} for all-duplicates, {N, nil} for N successfully inserted non-duplicate rows --- integration_test/myxql/upsert_all_test.exs | 61 ++++++++++++++++++++-- lib/ecto/adapters/myxql.ex | 14 +++-- lib/ecto/adapters/myxql/connection.ex | 12 +++-- test/ecto/adapters/myxql_test.exs | 7 ++- 4 files changed, 79 insertions(+), 15 deletions(-) diff --git a/integration_test/myxql/upsert_all_test.exs b/integration_test/myxql/upsert_all_test.exs index 0f15f1d8..606e5d72 100644 --- a/integration_test/myxql/upsert_all_test.exs +++ b/integration_test/myxql/upsert_all_test.exs @@ -13,10 +13,63 @@ defmodule Ecto.Integration.UpsertAllTest do test "on conflict ignore" do post = [title: "first", uuid: "6fa459ea-ee8a-3ca4-894e-db77e160355e"] - assert TestRepo.insert_all(Post, [post], on_conflict: :nothing) == - {1, nil} - assert TestRepo.insert_all(Post, [post], on_conflict: :nothing) == - {1, nil} + # First insert succeeds - 1 row inserted + assert TestRepo.insert_all(Post, [post], on_conflict: :nothing) == {1, nil} + # Second insert is ignored due to duplicate - 0 rows inserted (INSERT IGNORE behavior) + assert TestRepo.insert_all(Post, [post], on_conflict: :nothing) == {0, nil} + end + + test "on conflict ignore with mixed records (some conflicts, some new)" do + # Insert an existing post + existing_uuid = "6fa459ea-ee8a-3ca4-894e-db77e160355e" + existing_post = [title: "existing", uuid: existing_uuid] + assert TestRepo.insert_all(Post, [existing_post], on_conflict: :nothing) == {1, nil} + + # Now insert a batch with one duplicate and two new records + new_uuid1 = "7fa459ea-ee8a-3ca4-894e-db77e160355f" + new_uuid2 = "8fa459ea-ee8a-3ca4-894e-db77e160355a" + + posts = [ + [title: "new post 1", uuid: new_uuid1], # new - should be inserted + [title: "duplicate", uuid: existing_uuid], # duplicate - should be ignored + [title: "new post 2", uuid: new_uuid2] # new - should be inserted + ] + + # With INSERT IGNORE, only 2 rows should be inserted (the non-duplicates) + assert TestRepo.insert_all(Post, posts, on_conflict: :nothing) == {2, nil} + + # Verify the data - should have 3 posts total (1 existing + 2 new) + assert length(TestRepo.all(Post)) == 3 + + # Verify the existing post was not modified + [original] = TestRepo.all(from p in Post, where: p.uuid == ^existing_uuid) + assert original.title == "existing" # title unchanged + + # Verify new posts were inserted + assert TestRepo.exists?(from p in Post, where: p.uuid == ^new_uuid1) + assert TestRepo.exists?(from p in Post, where: p.uuid == ^new_uuid2) + end + + test "on conflict ignore with all duplicates" do + # Insert initial posts + uuid1 = "1fa459ea-ee8a-3ca4-894e-db77e160355e" + uuid2 = "2fa459ea-ee8a-3ca4-894e-db77e160355e" + initial_posts = [ + [title: "first", uuid: uuid1], + [title: "second", uuid: uuid2] + ] + assert TestRepo.insert_all(Post, initial_posts, on_conflict: :nothing) == {2, nil} + + # Try to insert all duplicates + duplicate_posts = [ + [title: "dup1", uuid: uuid1], + [title: "dup2", uuid: uuid2] + ] + # All are duplicates, so 0 rows inserted + assert TestRepo.insert_all(Post, duplicate_posts, on_conflict: :nothing) == {0, nil} + + # Verify count unchanged + assert length(TestRepo.all(Post)) == 2 end test "on conflict keyword list" do diff --git a/lib/ecto/adapters/myxql.ex b/lib/ecto/adapters/myxql.ex index dc595d06..75eb0180 100644 --- a/lib/ecto/adapters/myxql.ex +++ b/lib/ecto/adapters/myxql.ex @@ -330,9 +330,17 @@ defmodule Ecto.Adapters.MyXQL do case Ecto.Adapters.SQL.query(adapter_meta, sql, values ++ query_params, opts) do {:ok, %{num_rows: 0}} -> - raise "insert operation failed to insert any row in the database. " <> - "This may happen if you have trigger or other database conditions rejecting operations. " <> - "The emitted SQL was: #{sql}" + # With INSERT IGNORE (on_conflict: :nothing), 0 rows means the row was + # ignored due to a conflict, which is expected behavior + case on_conflict do + {:nothing, _, _} -> + {:ok, []} + + _ -> + raise "insert operation failed to insert any row in the database. " <> + "This may happen if you have trigger or other database conditions rejecting operations. " <> + "The emitted SQL was: #{sql}" + end # We were used to check if num_rows was 1 or 2 (in case of upserts) # but MariaDB supports tables with System Versioning, and in those diff --git a/lib/ecto/adapters/myxql/connection.ex b/lib/ecto/adapters/myxql/connection.ex index ae378beb..6ab887a5 100644 --- a/lib/ecto/adapters/myxql/connection.ex +++ b/lib/ecto/adapters/myxql/connection.ex @@ -181,9 +181,10 @@ if Code.ensure_loaded?(MyXQL) do @impl true def insert(prefix, table, header, rows, on_conflict, [], []) do fields = quote_names(header) + insert_keyword = insert_keyword(on_conflict) [ - "INSERT INTO ", + insert_keyword, quote_table(prefix, table), " (", fields, @@ -192,6 +193,9 @@ if Code.ensure_loaded?(MyXQL) do ] end + defp insert_keyword({:nothing, _, []}), do: "INSERT IGNORE INTO " + defp insert_keyword(_), do: "INSERT INTO " + def insert(_prefix, _table, _header, _rows, _on_conflict, _returning, []) do error!(nil, ":returning is not supported in insert/insert_all by MySQL") end @@ -208,9 +212,9 @@ if Code.ensure_loaded?(MyXQL) do [] end - defp on_conflict({:nothing, _, []}, [field | _]) do - quoted = quote_name(field) - [" ON DUPLICATE KEY UPDATE ", quoted, " = " | quoted] + defp on_conflict({:nothing, _, []}, _header) do + # Handled by INSERT IGNORE + [] end defp on_conflict({fields, _, []}, _header) when is_list(fields) do diff --git a/test/ecto/adapters/myxql_test.exs b/test/ecto/adapters/myxql_test.exs index 34641e0d..df5261f6 100644 --- a/test/ecto/adapters/myxql_test.exs +++ b/test/ecto/adapters/myxql_test.exs @@ -1465,11 +1465,10 @@ defmodule Ecto.Adapters.MyXQLTest do end end - test "insert with on duplicate key" do + test "insert with on conflict" do + # Using INSERT IGNORE for :nothing on_conflict query = insert(nil, "schema", [:x, :y], [[:x, :y]], {:nothing, [], []}, []) - - assert query == - ~s{INSERT INTO `schema` (`x`,`y`) VALUES (?,?) ON DUPLICATE KEY UPDATE `x` = `x`} + assert query == ~s{INSERT IGNORE INTO `schema` (`x`,`y`) VALUES (?,?)} update = from("schema", update: [set: [z: "foo"]]) |> plan(:update_all) query = insert(nil, "schema", [:x, :y], [[:x, :y]], {update, [], []}, []) From 8c7955df9dae7ed92959f2f7bdad814656a2bbef Mon Sep 17 00:00:00 2001 From: Aggelos Karalias Date: Tue, 16 Dec 2025 11:41:15 +0200 Subject: [PATCH 2/2] Add opt-in INSERT IGNORE support for MySQL via conflict_target The default on_conflict: :nothing behavior now uses the ON DUPLICATE KEY UPDATE x = x workaround (restoring original behavior), which always reports 1 affected row regardless of whether the row was inserted or ignored. For users who need accurate row counts (0 when ignored, 1 when inserted), INSERT IGNORE can be explicitly enabled via: Repo.insert_all(Post, posts, on_conflict: :nothing, conflict_target: {:unsafe_fragment, "insert_ignore"}) This approach was chosen to avoid modifying Ecto core with a new on_conflict type like :ignore for MySQL-specific intricacies. By leveraging the existing {:unsafe_fragment, _} mechanism, MySQL users can opt into INSERT IGNORE semantics when needed while maintaining backward compatibility. Note that INSERT IGNORE has broader semantics in MySQL - it ignores certain type conversion errors in addition to duplicate key conflicts - which is why it's not the default behavior. --- integration_test/myxql/upsert_all_test.exs | 64 +++++++++++++++------- lib/ecto/adapters/myxql.ex | 23 +++++++- lib/ecto/adapters/myxql/connection.ex | 15 ++++- test/ecto/adapters/myxql_test.exs | 23 +++++++- 4 files changed, 96 insertions(+), 29 deletions(-) diff --git a/integration_test/myxql/upsert_all_test.exs b/integration_test/myxql/upsert_all_test.exs index 606e5d72..2e593f8b 100644 --- a/integration_test/myxql/upsert_all_test.exs +++ b/integration_test/myxql/upsert_all_test.exs @@ -13,60 +13,84 @@ defmodule Ecto.Integration.UpsertAllTest do test "on conflict ignore" do post = [title: "first", uuid: "6fa459ea-ee8a-3ca4-894e-db77e160355e"] - # First insert succeeds - 1 row inserted + # Default :nothing behavior uses ON DUPLICATE KEY UPDATE x = x workaround + # which always reports rows as affected + assert TestRepo.insert_all(Post, [post], on_conflict: :nothing) == {1, nil} assert TestRepo.insert_all(Post, [post], on_conflict: :nothing) == {1, nil} + end + + test "explicit insert ignore" do + post = [title: "first", uuid: "6fa459ea-ee8a-3ca4-894e-db77e160355e"] + # First insert succeeds - 1 row inserted + assert TestRepo.insert_all(Post, [post], + on_conflict: :nothing, + conflict_target: {:unsafe_fragment, "insert_ignore"} + ) == {1, nil} + # Second insert is ignored due to duplicate - 0 rows inserted (INSERT IGNORE behavior) - assert TestRepo.insert_all(Post, [post], on_conflict: :nothing) == {0, nil} + assert TestRepo.insert_all(Post, [post], + on_conflict: :nothing, + conflict_target: {:unsafe_fragment, "insert_ignore"} + ) == {0, nil} end - test "on conflict ignore with mixed records (some conflicts, some new)" do + test "explicit insert ignore with mixed records (some conflicts, some new)" do # Insert an existing post existing_uuid = "6fa459ea-ee8a-3ca4-894e-db77e160355e" existing_post = [title: "existing", uuid: existing_uuid] - assert TestRepo.insert_all(Post, [existing_post], on_conflict: :nothing) == {1, nil} + + assert TestRepo.insert_all(Post, [existing_post], + on_conflict: :nothing, + conflict_target: {:unsafe_fragment, "insert_ignore"} + ) == {1, nil} # Now insert a batch with one duplicate and two new records new_uuid1 = "7fa459ea-ee8a-3ca4-894e-db77e160355f" new_uuid2 = "8fa459ea-ee8a-3ca4-894e-db77e160355a" posts = [ - [title: "new post 1", uuid: new_uuid1], # new - should be inserted - [title: "duplicate", uuid: existing_uuid], # duplicate - should be ignored - [title: "new post 2", uuid: new_uuid2] # new - should be inserted + [title: "new post 1", uuid: new_uuid1], + [title: "duplicate", uuid: existing_uuid], + [title: "new post 2", uuid: new_uuid2] ] # With INSERT IGNORE, only 2 rows should be inserted (the non-duplicates) - assert TestRepo.insert_all(Post, posts, on_conflict: :nothing) == {2, nil} + assert TestRepo.insert_all(Post, posts, + on_conflict: :nothing, + conflict_target: {:unsafe_fragment, "insert_ignore"} + ) == {2, nil} # Verify the data - should have 3 posts total (1 existing + 2 new) assert length(TestRepo.all(Post)) == 3 # Verify the existing post was not modified [original] = TestRepo.all(from p in Post, where: p.uuid == ^existing_uuid) - assert original.title == "existing" # title unchanged + assert original.title == "existing" # Verify new posts were inserted assert TestRepo.exists?(from p in Post, where: p.uuid == ^new_uuid1) assert TestRepo.exists?(from p in Post, where: p.uuid == ^new_uuid2) end - test "on conflict ignore with all duplicates" do + test "explicit insert ignore with all duplicates" do # Insert initial posts uuid1 = "1fa459ea-ee8a-3ca4-894e-db77e160355e" uuid2 = "2fa459ea-ee8a-3ca4-894e-db77e160355e" - initial_posts = [ - [title: "first", uuid: uuid1], - [title: "second", uuid: uuid2] - ] - assert TestRepo.insert_all(Post, initial_posts, on_conflict: :nothing) == {2, nil} + initial_posts = [[title: "first", uuid: uuid1], [title: "second", uuid: uuid2]] + + assert TestRepo.insert_all(Post, initial_posts, + on_conflict: :nothing, + conflict_target: {:unsafe_fragment, "insert_ignore"} + ) == {2, nil} # Try to insert all duplicates - duplicate_posts = [ - [title: "dup1", uuid: uuid1], - [title: "dup2", uuid: uuid2] - ] + duplicate_posts = [[title: "dup1", uuid: uuid1], [title: "dup2", uuid: uuid2]] + # All are duplicates, so 0 rows inserted - assert TestRepo.insert_all(Post, duplicate_posts, on_conflict: :nothing) == {0, nil} + assert TestRepo.insert_all(Post, duplicate_posts, + on_conflict: :nothing, + conflict_target: {:unsafe_fragment, "insert_ignore"} + ) == {0, nil} # Verify count unchanged assert length(TestRepo.all(Post)) == 2 diff --git a/lib/ecto/adapters/myxql.ex b/lib/ecto/adapters/myxql.ex index 75eb0180..5a5e675b 100644 --- a/lib/ecto/adapters/myxql.ex +++ b/lib/ecto/adapters/myxql.ex @@ -103,6 +103,23 @@ defmodule Ecto.Adapters.MyXQL do automatically commits after some commands like CREATE TABLE. Therefore MySQL migrations does not run inside transactions. + ### Upserts + + When using `on_conflict: :nothing`, the adapter uses the + `ON DUPLICATE KEY UPDATE x = x` workaround to simulate "do nothing" + behavior. This always reports 1 affected row regardless of whether + the row was actually inserted or ignored. + + If you need accurate row counts (0 when ignored, 1 when inserted), + you can opt into MySQL's `INSERT IGNORE` by specifying: + + Repo.insert_all(Post, posts, + on_conflict: :nothing, + conflict_target: {:unsafe_fragment, "insert_ignore"}) + + Note that `INSERT IGNORE` has broader semantics in MySQL - it also + ignores certain type conversion errors, not just duplicate key conflicts. + ## Old MySQL versions ### JSON support @@ -330,10 +347,10 @@ defmodule Ecto.Adapters.MyXQL do case Ecto.Adapters.SQL.query(adapter_meta, sql, values ++ query_params, opts) do {:ok, %{num_rows: 0}} -> - # With INSERT IGNORE (on_conflict: :nothing), 0 rows means the row was - # ignored due to a conflict, which is expected behavior + # With INSERT IGNORE (explicit via conflict_target), 0 rows means the row + # was ignored due to a conflict, which is expected behavior case on_conflict do - {:nothing, _, _} -> + {:nothing, _, {:unsafe_fragment, "insert_ignore"}} -> {:ok, []} _ -> diff --git a/lib/ecto/adapters/myxql/connection.ex b/lib/ecto/adapters/myxql/connection.ex index 6ab887a5..145bc264 100644 --- a/lib/ecto/adapters/myxql/connection.ex +++ b/lib/ecto/adapters/myxql/connection.ex @@ -193,7 +193,10 @@ if Code.ensure_loaded?(MyXQL) do ] end - defp insert_keyword({:nothing, _, []}), do: "INSERT IGNORE INTO " + # INSERT IGNORE only when explicitly requested via conflict_target + defp insert_keyword({:nothing, _, {:unsafe_fragment, "insert_ignore"}}), + do: "INSERT IGNORE INTO " + defp insert_keyword(_), do: "INSERT INTO " def insert(_prefix, _table, _header, _rows, _on_conflict, _returning, []) do @@ -212,11 +215,17 @@ if Code.ensure_loaded?(MyXQL) do [] end - defp on_conflict({:nothing, _, []}, _header) do - # Handled by INSERT IGNORE + # Explicit INSERT IGNORE - no ON DUPLICATE KEY clause needed + defp on_conflict({:nothing, _, {:unsafe_fragment, "insert_ignore"}}, _header) do [] end + # Default :nothing - uses workaround to simulate "do nothing" behavior + defp on_conflict({:nothing, _, []}, [field | _]) do + quoted = quote_name(field) + [" ON DUPLICATE KEY UPDATE ", quoted, " = " | quoted] + end + defp on_conflict({fields, _, []}, _header) when is_list(fields) do [ " ON DUPLICATE KEY UPDATE " diff --git a/test/ecto/adapters/myxql_test.exs b/test/ecto/adapters/myxql_test.exs index df5261f6..6909ed42 100644 --- a/test/ecto/adapters/myxql_test.exs +++ b/test/ecto/adapters/myxql_test.exs @@ -1465,10 +1465,12 @@ defmodule Ecto.Adapters.MyXQLTest do end end - test "insert with on conflict" do - # Using INSERT IGNORE for :nothing on_conflict + test "insert with on duplicate key" do + # Default :nothing uses ON DUPLICATE KEY UPDATE workaround query = insert(nil, "schema", [:x, :y], [[:x, :y]], {:nothing, [], []}, []) - assert query == ~s{INSERT IGNORE INTO `schema` (`x`,`y`) VALUES (?,?)} + + assert query == + ~s{INSERT INTO `schema` (`x`,`y`) VALUES (?,?) ON DUPLICATE KEY UPDATE `x` = `x`} update = from("schema", update: [set: [z: "foo"]]) |> plan(:update_all) query = insert(nil, "schema", [:x, :y], [[:x, :y]], {update, [], []}, []) @@ -1498,6 +1500,21 @@ defmodule Ecto.Adapters.MyXQLTest do end end + test "insert with explicit insert ignore" do + # Explicit INSERT IGNORE via conflict_target: {:unsafe_fragment, "insert_ignore"} + query = + insert( + nil, + "schema", + [:x, :y], + [[:x, :y]], + {:nothing, [], {:unsafe_fragment, "insert_ignore"}}, + [] + ) + + assert query == ~s{INSERT IGNORE INTO `schema` (`x`,`y`) VALUES (?,?)} + end + test "insert with query" do select_query = from("schema", select: [:id]) |> plan(:all)