Skip to content

(Please Review Before 2.0 releases!) Remove need to use Box::pin for transactions.#3053

Open
landonpoch wants to merge 2 commits into
SeaQL:masterfrom
landonpoch:async-fn-once-transaction
Open

(Please Review Before 2.0 releases!) Remove need to use Box::pin for transactions.#3053
landonpoch wants to merge 2 commits into
SeaQL:masterfrom
landonpoch:async-fn-once-transaction

Conversation

@landonpoch

@landonpoch landonpoch commented Apr 22, 2026

Copy link
Copy Markdown

PR Info

With rust >= 1.85 we shouldn't need to Box::pin async closures anymore. This fixes that. I'm hoping you'll accept this because it is a breaking change and 2.0 is in RC right now. This would be timely to get it in prior to a major upgrade.

New Features

  • Just uses the new async closures built in to the language now.

Bug Fixes

Breaking Changes

  • You'll need to remove those nasty Box::pin from your transaction closures. <- this is a good thing.

Note!!! If you don't want this as a breaking change consider modifying this PR to just create new methods for this and leave the old stuff in tact. However, I think it's much better just to rip the bandaid off in this release.

@landonpoch landonpoch changed the title Remove need to use Box::pin for transactions. (Please Review Before 2.0 releases!) Remove need to use Box::pin for transactions. Apr 22, 2026
@landonpoch

Copy link
Copy Markdown
Author

@tyt2y3 - I know you guys are pretty far along in the RC process at this point. I just want to make sure you get some eyes on this to see if it could make the cut. This would be a really nice thing to include as it makes using the api a lot easier.

@ProbstDJakob

Copy link
Copy Markdown

The problem with this PR is that, as @Huliiiiii wrote, you can not specify the Sync requirement for the returned Future. Since those are only AsyncFnOnce traits, one could add <F as AsyncFnOnce>::CallOnceFuture: Send but this, as well as the RTN, is nightly only at the moment. Thus there is currently no way to add this without nightly.

@landonpoch

landonpoch commented Apr 22, 2026

Copy link
Copy Markdown
Author

@ProbstDJakob - Got it. Waiting for CallOnceFuture is probably the move then. Once that is in, I can update this to add the Send contraint and then it should be ready to go. Oh well.

@tyt2y3

tyt2y3 commented Apr 23, 2026

Copy link
Copy Markdown
Member

would be nice if this works on stable!

@landonpoch

Copy link
Copy Markdown
Author

Realized I needed to update the tests to remove the Box::pin syntax to get them to pass. This makes sense because this is the breaking change called out earlier.

@Huliiiiii

Copy link
Copy Markdown
Member

#2749 (comment)

I think we could follow diesel's approach here. I don't really have time to work on this recently. Anyone interested in experimenting with it?

https://blog.weiznich.de/blog/diesel-async-0-9/

@tyt2y3

tyt2y3 commented Jun 1, 2026

Copy link
Copy Markdown
Member

I've tried the approach Hulii suggested, but it couldn't work because of the associated type TransactionTrait::Transaction.

I've added an experimental API in

pub async fn transaction_async<F, T, E>(&self, callback: F) -> Result<T, TransactionError<E>>
where
F: for<'c> AsyncFnOnce(&'c DatabaseTransaction) -> Result<T, E> + Send,
T: Send,
E: std::fmt::Display + std::fmt::Debug + Send,

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.

Feature Request: Support for async closures

4 participants