Newtype primary keys#3076
Conversation
c82c606 to
8d32814
Compare
|
Could you split this into smaller PRs or commits? A 4k-loc change is quite difficult to review. |
|
Thanks for the suggestion @Huliiiiii! I have a few things I want to improve before setting this draft PR ready to review, so I'm happy to split the PR in places that make sense. I'm open for suggestions, here's the current breakdown;
I'm not sure I can easily split the core functionality into multiple PRs, but it would be easy to split this up into separate commits. I can also move If it seems like just making more commits won't help the problem, I can drill deeper into separating functionality into multiple PRs. Let me know what you think! |
|
I think you can keep going with the current implementations for now. If it ends up being too hard to review once it’s ready, I’ll ask you to split it up then. |
PR Info
New Features
1. Opt-in type-safe primary keys via newtypes
Adds a code-generation mode (
sea-orm-cli generate entity --with-pk-newtypes) plus a library surface (sea_orm::Id<E, T>,PkAutoIncrementHint,FindByIdArg) that together give compile-time protection against mixing up primary-key values across entities. With the flag,user::Entity::find_by_id(post.id)no longer compiles becausepost.idisPostId(Id<post::Entity, i32>), which has noInto<Id<user::Entity, i32>>impl by design.2.
sea_orm::Id<E, T>library typePhantom-typed PK wrapper, unique for each table's primary key. The only construction path is
Id::new(value), there is deliberately noimpl<E, T> From<T> for Id<E, T>blanket, which is what makes the compiler reject cross-entity confusion at every use site.3.
FindByIdArg<E>traitThe shared bound on
find_by_id/filter_by_id/delete_by_id. A thin sea-orm-owned wrapper aroundInto<<E::PrimaryKey as PrimaryKeyTrait>::ValueType>so we can attach a curated#[diagnostic::on_unimplemented]message.4.
PkAutoIncrementHinttraitTrait-resolved default for whether a PK column is
AUTO_INCREMENT(integers ->true,String/Vec<u8>/Uuid->false).DeriveValueTypeemits an impl so the new PK wrappers will resolve through their inner type. This will replace the previous text-search method.Breaking Changes
Notes: Backwards-compatible: nothing changes for entities generated without
--with-pk-newtypes.find_by_id(7u8)against ani32PK still works via theInto-forwarding blanket impl, and theauto_increment()body now resolves throughPkAutoIncrementHintwhile producing the same defaults as before.DeleteMany::filter_by_ids(plural) keeps its pre-PRIntoIterator<Item = ValueType>bound to avoid unrelated behavior changes.Changes
UserFollowerPkUserId), (2) single-parent FK column -> parent's PK alias, (3) own-PK alias (CakePk), (4) raw scalar fallback.Column::refs: Vec<ColumnRef>is populated from FK constraints (multi-parent supported).auto_increment()macro body: composite PK ->false; explicit annotation -> literal bool; otherwise<FieldType as PkAutoIncrementHint>::IS_AUTO.DeriveValueTypeemitsDelegatesPkAutoIncrementHint.--with-pk-newtypesCLI flag (sea-orm-cli/src/cli.rs)find_by_id/delete_by_id/filter_by_idnow takeT: FindByIdArg<Self>Out of scope