Skip to content

Commit 7478895

Browse files
committed
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
1 parent 059a059 commit 7478895

4 files changed

Lines changed: 79 additions & 15 deletions

File tree

integration_test/myxql/upsert_all_test.exs

Lines changed: 57 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,63 @@ defmodule Ecto.Integration.UpsertAllTest do
1313

1414
test "on conflict ignore" do
1515
post = [title: "first", uuid: "6fa459ea-ee8a-3ca4-894e-db77e160355e"]
16-
assert TestRepo.insert_all(Post, [post], on_conflict: :nothing) ==
17-
{1, nil}
18-
assert TestRepo.insert_all(Post, [post], on_conflict: :nothing) ==
19-
{1, nil}
16+
# First insert succeeds - 1 row inserted
17+
assert TestRepo.insert_all(Post, [post], on_conflict: :nothing) == {1, nil}
18+
# Second insert is ignored due to duplicate - 0 rows inserted (INSERT IGNORE behavior)
19+
assert TestRepo.insert_all(Post, [post], on_conflict: :nothing) == {0, nil}
20+
end
21+
22+
test "on conflict ignore with mixed records (some conflicts, some new)" do
23+
# Insert an existing post
24+
existing_uuid = "6fa459ea-ee8a-3ca4-894e-db77e160355e"
25+
existing_post = [title: "existing", uuid: existing_uuid]
26+
assert TestRepo.insert_all(Post, [existing_post], on_conflict: :nothing) == {1, nil}
27+
28+
# Now insert a batch with one duplicate and two new records
29+
new_uuid1 = "7fa459ea-ee8a-3ca4-894e-db77e160355f"
30+
new_uuid2 = "8fa459ea-ee8a-3ca4-894e-db77e160355a"
31+
32+
posts = [
33+
[title: "new post 1", uuid: new_uuid1], # new - should be inserted
34+
[title: "duplicate", uuid: existing_uuid], # duplicate - should be ignored
35+
[title: "new post 2", uuid: new_uuid2] # new - should be inserted
36+
]
37+
38+
# With INSERT IGNORE, only 2 rows should be inserted (the non-duplicates)
39+
assert TestRepo.insert_all(Post, posts, on_conflict: :nothing) == {2, nil}
40+
41+
# Verify the data - should have 3 posts total (1 existing + 2 new)
42+
assert length(TestRepo.all(Post)) == 3
43+
44+
# Verify the existing post was not modified
45+
[original] = TestRepo.all(from p in Post, where: p.uuid == ^existing_uuid)
46+
assert original.title == "existing" # title unchanged
47+
48+
# Verify new posts were inserted
49+
assert TestRepo.exists?(from p in Post, where: p.uuid == ^new_uuid1)
50+
assert TestRepo.exists?(from p in Post, where: p.uuid == ^new_uuid2)
51+
end
52+
53+
test "on conflict ignore with all duplicates" do
54+
# Insert initial posts
55+
uuid1 = "1fa459ea-ee8a-3ca4-894e-db77e160355e"
56+
uuid2 = "2fa459ea-ee8a-3ca4-894e-db77e160355e"
57+
initial_posts = [
58+
[title: "first", uuid: uuid1],
59+
[title: "second", uuid: uuid2]
60+
]
61+
assert TestRepo.insert_all(Post, initial_posts, on_conflict: :nothing) == {2, nil}
62+
63+
# Try to insert all duplicates
64+
duplicate_posts = [
65+
[title: "dup1", uuid: uuid1],
66+
[title: "dup2", uuid: uuid2]
67+
]
68+
# All are duplicates, so 0 rows inserted
69+
assert TestRepo.insert_all(Post, duplicate_posts, on_conflict: :nothing) == {0, nil}
70+
71+
# Verify count unchanged
72+
assert length(TestRepo.all(Post)) == 2
2073
end
2174

2275
test "on conflict keyword list" do

lib/ecto/adapters/myxql.ex

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -330,9 +330,17 @@ defmodule Ecto.Adapters.MyXQL do
330330

331331
case Ecto.Adapters.SQL.query(adapter_meta, sql, values ++ query_params, opts) do
332332
{:ok, %{num_rows: 0}} ->
333-
raise "insert operation failed to insert any row in the database. " <>
334-
"This may happen if you have trigger or other database conditions rejecting operations. " <>
335-
"The emitted SQL was: #{sql}"
333+
# With INSERT IGNORE (on_conflict: :nothing), 0 rows means the row was
334+
# ignored due to a conflict, which is expected behavior
335+
case on_conflict do
336+
{:nothing, _, _} ->
337+
{:ok, []}
338+
339+
_ ->
340+
raise "insert operation failed to insert any row in the database. " <>
341+
"This may happen if you have trigger or other database conditions rejecting operations. " <>
342+
"The emitted SQL was: #{sql}"
343+
end
336344

337345
# We were used to check if num_rows was 1 or 2 (in case of upserts)
338346
# but MariaDB supports tables with System Versioning, and in those

lib/ecto/adapters/myxql/connection.ex

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -181,9 +181,10 @@ if Code.ensure_loaded?(MyXQL) do
181181
@impl true
182182
def insert(prefix, table, header, rows, on_conflict, [], []) do
183183
fields = quote_names(header)
184+
insert_keyword = insert_keyword(on_conflict)
184185

185186
[
186-
"INSERT INTO ",
187+
insert_keyword,
187188
quote_table(prefix, table),
188189
" (",
189190
fields,
@@ -192,6 +193,9 @@ if Code.ensure_loaded?(MyXQL) do
192193
]
193194
end
194195

196+
defp insert_keyword({:nothing, _, []}), do: "INSERT IGNORE INTO "
197+
defp insert_keyword(_), do: "INSERT INTO "
198+
195199
def insert(_prefix, _table, _header, _rows, _on_conflict, _returning, []) do
196200
error!(nil, ":returning is not supported in insert/insert_all by MySQL")
197201
end
@@ -208,9 +212,9 @@ if Code.ensure_loaded?(MyXQL) do
208212
[]
209213
end
210214

211-
defp on_conflict({:nothing, _, []}, [field | _]) do
212-
quoted = quote_name(field)
213-
[" ON DUPLICATE KEY UPDATE ", quoted, " = " | quoted]
215+
defp on_conflict({:nothing, _, []}, _header) do
216+
# Handled by INSERT IGNORE
217+
[]
214218
end
215219

216220
defp on_conflict({fields, _, []}, _header) when is_list(fields) do

test/ecto/adapters/myxql_test.exs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1465,11 +1465,10 @@ defmodule Ecto.Adapters.MyXQLTest do
14651465
end
14661466
end
14671467

1468-
test "insert with on duplicate key" do
1468+
test "insert with on conflict" do
1469+
# Using INSERT IGNORE for :nothing on_conflict
14691470
query = insert(nil, "schema", [:x, :y], [[:x, :y]], {:nothing, [], []}, [])
1470-
1471-
assert query ==
1472-
~s{INSERT INTO `schema` (`x`,`y`) VALUES (?,?) ON DUPLICATE KEY UPDATE `x` = `x`}
1471+
assert query == ~s{INSERT IGNORE INTO `schema` (`x`,`y`) VALUES (?,?)}
14731472

14741473
update = from("schema", update: [set: [z: "foo"]]) |> plan(:update_all)
14751474
query = insert(nil, "schema", [:x, :y], [[:x, :y]], {update, [], []}, [])

0 commit comments

Comments
 (0)