Skip to content
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

Use unique_constraint in changeset for link_patch_batch #838

Merged
merged 1 commit into from Dec 30, 2019

Conversation

@ansonlc
Copy link
Contributor

ansonlc commented Dec 30, 2019

Recently, I saw few crashes are caused by this error

{%Ecto.ConstraintError{
   constraint: "link_patch_batch_patch_id_batch_id_index",
   message: "constraint error when attempting to insert struct:\n\n    * unique: link_patch_batch_patch_id_batch_id_index\n\nIf you would like to convert this constraint into an error, please\ncall unique_constraint/3 in your changeset and define the proper\nconstraint name. The changeset has not defined any constraint.\n",
   type: :unique
 },
 [
   {Ecto.Repo.Schema, :"-constraints_to_errors/3-fun-1-", 4,
    [file: 'lib/ecto/repo/schema.ex', line: 574]},
   {Enum, :"-map/2-lists^map/1-0-", 2,
    [file: 'lib/enum.ex', line: 1327]},
   {Ecto.Repo.Schema, :constraints_to_errors, 3,
    [file: 'lib/ecto/repo/schema.ex', line: 559]},
   {Ecto.Repo.Schema, :"-do_insert/4-fun-1-", 14,
    [file: 'lib/ecto/repo/schema.ex', line: 222]},
   {Ecto.Repo.Schema, :"-wrap_in_transaction/6-fun-0-", 3,
    [file: 'lib/ecto/repo/schema.ex', line: 774]},
   {Ecto.Adapters.SQL, :"-do_transaction/3-fun-1-", 3,
    [file: 'lib/ecto/adapters/sql.ex', line: 576]},
   {DBConnection, :transaction_run, 4,
    [file: 'lib/db_connection.ex', line: 1283]},
   {DBConnection, :run_begin, 3,
    [file: 'lib/db_connection.ex', line: 1207]}
 ]} 

Not sure what is the root cause (may be race condition in somewhere, or duplicate messages?).
But I saw we use this unique_constraint in other places, to be consistent and make the error more clear, I think it's better to add it in link_patch_batch as well.

@notriddle

This comment has been minimized.

Copy link
Member

notriddle commented Dec 30, 2019

I had to read the docs to find out that this would actually work, because that function is actually kinda weird. But here it is, in black-and-white:

Notice that the first param is just one of the unique index fields, this will be used as the error key to the changeset errors keyword list.

So it's perfectly fine that your call to the function only uses one of the two fields that the actual constraint is based on. The actual check is based on the name of the index.

bors r+

bors bot added a commit that referenced this pull request Dec 30, 2019
Merge #838
838: Use unique_constraint in changeset for link_patch_batch r=notriddle a=ansonlc

Recently, I saw few crashes are caused by this error 
```
{%Ecto.ConstraintError{
   constraint: "link_patch_batch_patch_id_batch_id_index",
   message: "constraint error when attempting to insert struct:\n\n    * unique: link_patch_batch_patch_id_batch_id_index\n\nIf you would like to convert this constraint into an error, please\ncall unique_constraint/3 in your changeset and define the proper\nconstraint name. The changeset has not defined any constraint.\n",
   type: :unique
 },
 [
   {Ecto.Repo.Schema, :"-constraints_to_errors/3-fun-1-", 4,
    [file: 'lib/ecto/repo/schema.ex', line: 574]},
   {Enum, :"-map/2-lists^map/1-0-", 2,
    [file: 'lib/enum.ex', line: 1327]},
   {Ecto.Repo.Schema, :constraints_to_errors, 3,
    [file: 'lib/ecto/repo/schema.ex', line: 559]},
   {Ecto.Repo.Schema, :"-do_insert/4-fun-1-", 14,
    [file: 'lib/ecto/repo/schema.ex', line: 222]},
   {Ecto.Repo.Schema, :"-wrap_in_transaction/6-fun-0-", 3,
    [file: 'lib/ecto/repo/schema.ex', line: 774]},
   {Ecto.Adapters.SQL, :"-do_transaction/3-fun-1-", 3,
    [file: 'lib/ecto/adapters/sql.ex', line: 576]},
   {DBConnection, :transaction_run, 4,
    [file: 'lib/db_connection.ex', line: 1283]},
   {DBConnection, :run_begin, 3,
    [file: 'lib/db_connection.ex', line: 1207]}
 ]} 
```
Not sure what is the root cause (may be race condition in somewhere, or duplicate messages?).
But I saw we use this `unique_constraint` in other places, to be consistent and make the error more clear, I think it's better to add it in link_patch_batch as well.

Co-authored-by: Chao Lin <linchaoc@gmail.com>
@bors

This comment has been minimized.

Copy link
Contributor

bors bot commented Dec 30, 2019

Build succeeded

@bors bors bot merged commit 5f002c0 into bors-ng:master Dec 30, 2019
3 checks passed
3 checks passed
Community-TC (pull_request) TaskGroup: success
Details
Travis CI - Pull Request Build Passed
Details
bors Build succeeded
Details
@ansonlc ansonlc deleted the Gusto:cl-add-unique_constraint branch Dec 30, 2019
@ansonlc

This comment has been minimized.

Copy link
Contributor Author

ansonlc commented Dec 30, 2019

Yeah, I saw the doc, it only allow one field to be passed in 🤦‍♂

notriddle added a commit to bors-ng/bors-ng.github.io that referenced this pull request Jan 2, 2020
Add @ansonlc as new contributor
notriddle added a commit to bors-ng/bors-ng.github.io that referenced this pull request Jan 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.