+
Skip to content

Hotfixed compatibility with sqlx 0.8.4 TransactionManager interface #2562

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

Merged
merged 1 commit into from
Apr 14, 2025

Conversation

antoinecharbonneau
Copy link
Contributor

PR Info

Bug Fixes

  • Fixes issue where library wouldn't compile due to sqlx 0.8.4 change of interface for TransactionManager interface

Breaking Changes

Should be all backwards compatible as it doesn't add any code ran to the transaction as it used to. Works with all tests.

Changes

Added None option to TransactionManager begin function, implying no query should be ran if a transaction is initiated.

@antoinecharbonneau
Copy link
Contributor Author

This would require hotfixing latest release of sea-orm

@tyt2y3
Copy link
Member

tyt2y3 commented Apr 14, 2025

thank you for submitting this quickly!

@tyt2y3
Copy link
Member

tyt2y3 commented Apr 14, 2025

Let me release this quick

@tyt2y3 tyt2y3 merged commit 6579706 into SeaQL:master Apr 14, 2025
35 of 36 checks passed
Copy link

🎉 Released In 1.1.10 🎉

Thank you everyone for the contribution!
This feature is now available in the latest release. Now is a good time to upgrade!
Your participation is what makes us unique; your adoption is what drives us forward.
You can support SeaQL 🌊 by starring our repos, sharing our libraries and becoming a sponsor ⭐.

@abonander
Copy link

TransactionManager is a #[doc(hidden)] trait in an explicitly SemVer-exempt crate. This is a temporary fix at best.

@abonander
Copy link

I just noticed that it's actually exported from sqlx. This export will be deleted in 0.9.0 as it's not meant to be used directly.

@abonander
Copy link

I see what's going on: you're using your own RAII wrapper that's not compatible with sqlx::Transaction. I can see the motivation for that.

The issue is that TransactionManager is an implementation detail that needs to remain flexible so we can do things like add begin_with backwards-compatibly. That's why it's a #[doc(hidden)] trait.

We need to figure out a long-term solution here:

  • You could use the trait from sqlx-core with a pinned version, but then you have to cut a release every time SQLx does.
  • You could implement your own transaction handling and execute BEGIN, COMMIT and ROLLBACK statements directly. This offers SeaORM the most potential flexibility.
  • We could design a non-RAII transaction begin/end API for SQLx. My main concern with this is that the RAII-based API should still be the preferred method for 99% of use-cases, so the design should make that obvious.

However, perhaps in general we need a better strategy for distinguishing whether SemVer applies only to usages of a trait, and not implementations. Then TransactionManager could just be a public trait. It should probably just be a supertrait of Connection, honestly.

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.

Sqlx0.8.4 breaks sea-orm
3 participants
点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载