Skip to content

Add Send bounds for transactions in ActiveModelEx methods - #3149

Closed
Huliiiiii wants to merge 1 commit into
SeaQL:masterfrom
Huliiiiii:fix-3147
Closed

Add Send bounds for transactions in ActiveModelEx methods#3149
Huliiiiii wants to merge 1 commit into
SeaQL:masterfrom
Huliiiiii:fix-3147

Conversation

@Huliiiiii

Copy link
Copy Markdown
Member

@Huliiiiii
Huliiiiii requested a review from tyt2y3 July 24, 2026 18:25

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 880dc9340e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread sea-orm-macros/src/derives/active_model_ex.rs
@tyt2y3

tyt2y3 commented Jul 31, 2026

Copy link
Copy Markdown
Member

the reason I tried another (more invasive) approach: this compiles on 2.0.0, but not after this fix

async fn create<C: ConnectionTrait + TransactionTrait>(db: &C) -> Result<user::ModelEx, DbErr> {
    user::ActiveModel::builder().set_name("a").insert(db).await
}

@Huliiiiii

Copy link
Copy Markdown
Member Author

the reason I tried another (more invasive) approach: this compiles on 2.0.0, but not after this fix

async fn create<C: ConnectionTrait + TransactionTrait>(db: &C) -> Result<user::ModelEx, DbErr> {
    user::ActiveModel::builder().set_name("a").insert(db).await
}

Ah, that makes sense. I hadn't considered this case. Your PR does seem like the most practical solution.

@Huliiiiii Huliiiiii closed this Jul 31, 2026
@Huliiiiii
Huliiiiii deleted the fix-3147 branch July 31, 2026 15:06
tyt2y3 added a commit that referenced this pull request Jul 31, 2026
The methods generated by `#[sea_orm::model]` (`insert`/`update`/`save`/
`delete`/`action`/`cascade_delete`) open a transaction and then save related
models inside it, so `action::<C>` recurses into `action::<C::Transaction>`.
Proving those futures `Send` without a concrete `C` walked the unbounded
`C::Transaction::Transaction::...` chain and overflowed (E0275 /
`clippy::future_not_send`), so the documented 2.0 entity format failed to
compile under clippy.

Fix it on the trait rather than on the generated methods: `TransactionTrait`
now requires `Sync` and pins its associated `Transaction` to a fixed point
(`Transaction::Transaction == Transaction`) that is `Send`. That collapses the
chain to a single type, so the obligation terminates.

`Sync` and `Send` exclude no implementor: the `#[async_trait]` desugaring
already forces both, since `begin` holds `&self` across an await and
`TransactionSession::commit` takes `self` by value. Only a non-fixed-point
`Transaction` is newly rejected, and it errors at the offending `impl` rather
than at `#[sea_orm::model]`. All in-crate implementors are fixed points already.

Putting the bounds on the generated methods instead (as in #3149) fixes the
same overflow but leaks into user call sites: a helper generic over the
connection, e.g. `fn create<C: ConnectionTrait + TransactionTrait>(db: &C)`,
compiles on 2.0.0 and stops compiling, with no exported alias to restate. On
the trait the requirement is invisible to callers.

The regression test is async-only (the sync crate has no futures to prove
`Send`), so it lives in the main crate's `tests/` gated on `feature =
"sqlx-dep"` and compiles out in `sea-orm-sync`. Its `assert_send` calls are the
actual checks, covering every generated method generically over the connection
across has_many / has_one / belongs_to / many-to-many / self-ref / no-relation
shapes, the `ConnectionTrait + TransactionTrait` and `impl Trait` caller forms,
the concrete connection types, and a downstream `TransactionTrait` impl. Without
the fix it fails with 7 E0275, checked by plain `cargo test`.
tyt2y3 added a commit that referenced this pull request Jul 31, 2026
The methods generated by `#[sea_orm::model]` (`insert`/`update`/`save`/
`delete`/`action`/`cascade_delete`) open a transaction and then save related
models inside it, so `action::<C>` recurses into `action::<C::Transaction>`.
Proving those futures `Send` without a concrete `C` walked the unbounded
`C::Transaction::Transaction::...` chain and overflowed (E0275 /
`clippy::future_not_send`), so the documented 2.0 entity format failed to
compile under clippy.

Fix it on the trait rather than on the generated methods: `TransactionTrait`
now requires `Sync` and pins its associated `Transaction` to a fixed point
(`Transaction::Transaction == Transaction`) that is `Send`. That collapses the
chain to a single type, so the obligation terminates.

`Sync` and `Send` exclude no implementor: the `#[async_trait]` desugaring
already forces both, since `begin` holds `&self` across an await and
`TransactionSession::commit` takes `self` by value. Only a non-fixed-point
`Transaction` is newly rejected, and it errors at the offending `impl` rather
than at `#[sea_orm::model]`. All in-crate implementors are fixed points already.

Putting the bounds on the generated methods instead (as in #3149) fixes the
same overflow but leaks into user call sites: a helper generic over the
connection, e.g. `fn create<C: ConnectionTrait + TransactionTrait>(db: &C)`,
compiles on 2.0.0 and stops compiling, with no exported alias to restate. On
the trait the requirement is invisible to callers.

The regression test is async-only (the sync crate has no futures to prove
`Send`), so it lives in the main crate's `tests/` gated on `feature =
"sqlx-dep"` and compiles out in `sea-orm-sync`. Its `assert_send` calls are the
actual checks, covering every generated method generically over the connection
across has_many / has_one / belongs_to / many-to-many / self-ref / no-relation
shapes, the `ConnectionTrait + TransactionTrait` and `impl Trait` caller forms,
the concrete connection types, and a downstream `TransactionTrait` impl. Without
the fix it fails with 7 E0275, checked by plain `cargo test`.
tyt2y3 added a commit that referenced this pull request Jul 31, 2026
The methods generated by `#[sea_orm::model]` (`insert`/`update`/`save`/
`delete`/`action`/`cascade_delete`) open a transaction and then save related
models inside it, so `action::<C>` recurses into `action::<C::Transaction>`.
Proving those futures `Send` without a concrete `C` walked the unbounded
`C::Transaction::Transaction::...` chain and overflowed (E0275 /
`clippy::future_not_send`), so the documented 2.0 entity format failed to
compile under clippy.

Fix it on the trait rather than on the generated methods: `TransactionTrait`
now requires `Sync` and pins its associated `Transaction` to a fixed point
(`Transaction::Transaction == Transaction`) that is `Send`. That collapses the
chain to a single type, so the obligation terminates.

`Sync` and `Send` exclude no implementor: the `#[async_trait]` desugaring
already forces both, since `begin` holds `&self` across an await and
`TransactionSession::commit` takes `self` by value. Only a non-fixed-point
`Transaction` is newly rejected, and it errors at the offending `impl` rather
than at `#[sea_orm::model]`. All in-crate implementors are fixed points already.

Putting the bounds on the generated methods instead (as in #3149) fixes the
same overflow but leaks into user call sites: a helper generic over the
connection, e.g. `fn create<C: ConnectionTrait + TransactionTrait>(db: &C)`,
compiles on 2.0.0 and stops compiling, with no exported alias to restate. On
the trait the requirement is invisible to callers.

The regression test is async-only (the sync crate has no futures to prove
`Send`), so it lives in the main crate's `tests/` gated on `feature =
"sqlx-dep"` and compiles out in `sea-orm-sync`. Its `assert_send` calls are the
actual checks, covering every generated method generically over the connection
across has_many / has_one / belongs_to / many-to-many / self-ref / no-relation
shapes, the `ConnectionTrait + TransactionTrait` and `impl Trait` caller forms,
the concrete connection types, and a downstream `TransactionTrait` impl. Without
the fix it fails with 7 E0275, checked by plain `cargo test`.
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.

2.0 - New Entity Format - Compiler (recursion) error

2 participants