Added enum for SQLx Error and basic functions for it#1707
Added enum for SQLx Error and basic functions for it#1707billy1624 merged 26 commits intoSeaQL:masterfrom
Conversation
darkmmon
changed the title
billy1624
left a comment
billy1624
left a comment
There was a problem hiding this comment.
Hey Ivan, nice! The sql_err method looks a lot cleaner than the first draft! Some comments...
src/error.rs
Outdated
| "23000" => return Some(SqlErr::UniqueConstraintViolation()), | ||
| "1586" => return Some(SqlErr::UniqueConstraintViolation()), |
There was a problem hiding this comment.
I wonder why we have two MySQL error codes representing the same unique constraint violation error?
Can we have a comment documenting the corresponding web page of each database's error code?
There was a problem hiding this comment.
Originally I put 23000 since I saw it in the error testing file, but later I found that 23000 is the sqlstate for both unique/foreign key constraint violation. Therefore I cannot use sqlstate there. I would update the error codes again once I make a foreign key constraint test.
Thank you for spotting this though!
| let error_number = e | ||
| .try_downcast_ref::<sqlx::mysql::MySqlDatabaseError>()? | ||
| .number(); | ||
| match error_number { |
There was a problem hiding this comment.
Hey @darkmmon, I wonder why for MySQL we use error_number to determine the error type instead of using _error_code_expanded just like other databases?
There was a problem hiding this comment.
@billy1624
The reason is that the error code for MySQL is ambiguous. If I wanted to do it with .code() like for other databases, I would not be able to distinguish the violation between unique constraint and foreign key constraint (they both return 23000).
I can only think of using the exact error number for distinguishing these errors, leading to the clumsy code. If there is an elegant way it would definitely be welcomed!
src/error.rs
Outdated
| ForeignKeyConstraintViolation(), | ||
| /// error for duplicate record in unique field or primary key field | ||
| #[error("Unique Constraint Violated")] | ||
| UniqueConstraintViolation, |
There was a problem hiding this comment.
My Question is why remove the ()? It is a placeholder, in case some day, we'd like to add some content.
There was a problem hiding this comment.
I believe if in the future we want to add content, then we will need to update everything no matter if we place the () there. So I thought it would be better for us to remove the bracket first and add it back if it is needed.
There was a problem hiding this comment.
Actually, I thought we shipped this enum somewhere already.
But then can we actually shove a little more context inside them?
Even just &'static str something like "Duplicate entry".
But of course more structure is welcomed.
src/error.rs
Outdated
| #[non_exhaustive] | ||
| pub enum SqlErr { | ||
| /// error for duplicate record in unique field or primary key field | ||
| #[error("Unique Constraint Violated")] |
There was a problem hiding this comment.
We can then do #[error("Unique Constraint Violated: {0}")]
tests/sql_err_tests.rs
Outdated
| assert_eq!( | ||
| error.sql_err(), | ||
| Some(SqlErr::UniqueConstraintViolation(String::from( | ||
| error_message | ||
| ))) | ||
| ); |
There was a problem hiding this comment.
The content is out of our control, so may be
assert!(matches!(error.sql_err(), Some(SqlErr::UniqueConstraintViolation(_)));would suffice.
* Optional Field SeaQL/sea-orm#1513 * .gitignore SeaQL/sea-orm#1334 * migration table custom name SeaQL/sea-orm#1511 * OR condition relation SeaQL/sea-orm#1433 * DerivePartialModel SeaQL/sea-orm#1597 * space for migration file naming SeaQL/sea-orm#1570 * composite key up to 12 SeaQL/sea-orm#1508 * seaography integration SeaQL/sea-orm#1599 * QuerySelect SimpleExpr SeaQL/sea-orm#1702 * sqlErr SeaQL/sea-orm#1707 * migration check SeaQL/sea-orm#1519 * postgres array SeaQL/sea-orm#1565 * param intoString SeaQL/sea-orm#1439 * **skipped** re-export SeaQL/sea-orm#1661 * ping SeaQL/sea-orm#1627 * on empty do nothing SeaQL/sea-orm#1708 * on conflict do nothing SeaQL/sea-orm#1712 * **skipped** upgrade versions * active enum fail safe SeaQL/sea-orm#1374 * relation generation check SeaQL/sea-orm#1435 * entity generation bug SeaQL/sea-schema#105 * **skipped** bug fix that does not require edits * EnumIter change SeaQL/sea-orm#1535 * completed and fixed a previous todo SeaQL/sea-orm#1570 * amended wordings and structures * Edit * Remove temp file --------- Co-authored-by: Billy Chan <[email protected]>
🎉 Released In 0.12.1 🎉Thank you everyone for the contribution! |
3 participants
PR Info
New Features
Changes