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

fix: translate SQLITE_CONSTRAINT_PRIMARYKEY to ErrDuplicatedKey #152

Merged
merged 1 commit into from Jun 9, 2023

Conversation

bfabio
Copy link
Contributor

@bfabio bfabio commented Jun 1, 2023

  • Do only one thing
  • Non breaking API changes
  • Tested

What did this pull request do?

Translate the SQLITE_CONSTRAINT_PRIMARYKEY error code to ErrDuplicatedKey as well for consistency.

User Case Description

We refactored our code to take advantage of ErrDuplicatedKey, but the tests fail because ErrDuplicatedKey only gets returned for non primary key fields (and it should IMO):

italia/developers-italia-api#197
https://github.com/italia/developers-italia-api/actions/runs/5143820348/jobs/9259314436?pr=197

Also, should this go in https://github.com/go-gorm/postgres and https://github.com/go-gorm/mysql as well?

@jinzhu
Copy link
Member

jinzhu commented Jun 7, 2023

Hi @bfabio

Can you fix the conflicts? thank you

Translate the SQLITE_CONSTRAINT_PRIMARYKEY error code to
ErrDuplicatedKey as well for consistency.
@bfabio
Copy link
Contributor Author

bfabio commented Jun 7, 2023

@jinzhu sure, done

@@ -7,6 +7,7 @@ import (
)

var errCodes = map[int]error{
1555: gorm.ErrDuplicatedKey,
Copy link
Member

@saeidee saeidee Jun 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not DuplicatedKey violation error, please check out this https://www.sqlite.org/rescode.html#constraint_primarykey

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jinzhu what do you think if we add a new CONSTRAINT_PRIMARYKEY error instead of using the same ErrDuplicatedKey?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about other databases, do they have any error code like CONSTRAINT_PRIMARYKEY, if not we can use the same one.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find this error type for Postgres and MySQL, @bfabio feel free to continue with your PR.

Mysql error codes: https://dev.mysql.com/doc/mysql-errors/8.0/en/server-error-reference.html
Postgres error codes: https://www.postgresql.org/docs/current/errcodes-appendix.html

@jinzhu jinzhu merged commit 2a60d4f into go-gorm:master Jun 9, 2023
4 checks passed
@bfabio bfabio deleted the primary-key-constraint branch June 19, 2023 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants