-
Notifications
You must be signed in to change notification settings - Fork 333
MyXQL: allow use of INSERT IGNORE for on_conflict: :nothing #703
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
MyXQL: allow use of INSERT IGNORE for on_conflict: :nothing #703
Conversation
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
|
I guess this means at least a PATCH version. This should be considered a breaking change. Or we can somehow put it behind a configuration with the default being the existing behavior. |
|
I don't believe it is as simple as this. Please take a read here: https://dev.mysql.com/doc/refman/8.4/en/sql-mode.html#ignore-effect-on-execution For example this part:
The issue seems to be that MySQL doesn't have any equivalent to |
|
Yes, I read the docs but for some reason the data conversion part didn't trigger. There are 3 parts in this section:
So if we have to choose our poison here we're indeed between 2 suboptimal solutions when we provide
I think preventing 2 is more important for a core piece like Thanks for pointing that out @greg-rychlewski. |
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.
on_conflict: :nothing, confict_target: {:unsafe_fragment, "insert_ignore"}}
on_conflict: :nothing, confict_target: {:unsafe_fragment, "insert_ignore"}}|
@greg-rychlewski did a last try to make this opt-in under a certain I understand it looks like a magic value but check this out and let me know what you think. |
|
Yeah, |
The default on_conflict: :nothing behavior uses ON DUPLICATE KEY UPDATE x = x
which has the following characteristics:
conflict, because the UPDATE clause still matches the existing row
For users who need accurate row counts (0 when ignored, 1 when inserted),
INSERT IGNORE can be explicitly enabled via:
With INSERT IGNORE:
expected behavior when all rows are duplicates
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.
ORIGINAL POST