Expose `transaction_depth` through `get_transaction_depth()` method

[mpyw]

Issue

• Resolves #3426

Description

This PR introduces the get_transaction_depth() method for exposing the current transaction depth in various database connections.

• MySqlConnection and PgConnection now implement the TransactionDepth::get_transaction_depth() method to synchronously retrieve the transaction depth.
• SqliteConnection implements the AsyncTransactionDepth::get_transaction_depth() method to retrieve the transaction depth asynchronously by acquiring a lock (as the existing implementation requires a lock to access the transaction level).
• AnyConnection uses the AsyncTransactionDepth::get_transaction_depth() method to handle SqliteConnection, and in the dyn AnyConnectionBackend implementation, it dispatches to the respective connection's implementation.

Concerns

• Since the method get_transaction_depth() is used for both traits, there is a potential for import collisions between the two traits. However, it is unlikely that both traits would be imported simultaneously, so this should not pose a significant issue.
• Regarding the SqliteConnection implementation, I'm unsure if this approach is optimal. Specifically, is it acceptable to acquire a lock just to retrieve the transaction nesting level? Additionally, my understanding is that the lock is automatically released when it goes out of scope—is this correct?

Additional Context

I am relatively new to using Rust professionally, having only recently started working with it. If there are any issues with my approach or if I am violating any best practices, please feel free to point them out.

[mpyw]

@abonander

Thank you for the feedback. I have removed the emojis from the commit for now. I have some questions regarding the refactoring approach.

• Would it be acceptable to implement the following changes to the TransactionManager trait?

  diff --git a/sqlx-core/src/transaction.rs b/sqlx-core/src/transaction.rs
  index 9cd38aab..4be5ab6b 100644
  --- a/sqlx-core/src/transaction.rs
  +++ b/sqlx-core/src/transaction.rs
  @@ -32,6 +32,24 @@ pub trait TransactionManager {
  
       /// Starts to abort the active transaction or restore from the most recent snapshot.
       fn start_rollback(conn: &mut <Self::Database as Database>::Connection);
  +
  +    /// Returns the current transaction depth.
  +    ///
  +    /// Transaction depth indicates the level of nested transactions:
  +    /// - Level 0: No active transaction.
  +    /// - Level 1: A transaction is active.
  +    /// - Level 2 or higher: A transaction is active and one or more SAVEPOINTs have been created within it.
  +    fn get_transaction_depth(conn: &<Self::Database as Database>::Connection) -> usize;
  +
  +    /// Checks if the connection is currently in a transaction.
  +    ///
  +    /// This method returns `true` if the current transaction depth is greater than 0,
  +    /// indicating that a transaction is active. It returns `false` if the transaction depth is 0,
  +    /// meaning no transaction is active.
  +    #[inline]
  +    fn is_in_transaction(conn: &<Self::Database as Database>::Connection) -> bool {
  +        conn.get_transaction_depth() != 0
  +    }
   }
  

• Even if we forward to the TransactionManager trait from the Connection trait, adding a get_transaction_depth() method to the Connection trait still results in a breaking change. To avoid a breaking change, I believe it would only be possible to implement it directly on the concrete types at least for MySQL and Postgres. What are your thoughts on this?
• Refactoring the SQLite driver seems challenging, so could you please provide more guidance on how to approach this? (I will try my best, but if it turns out to be too difficult for me, I might need to focus on implementing the non-SQLite parts and leave some of the work for you with todo!() markers.)

---

[Edit]

I just realized something. If the goal is to delegate the logic, then implementing is_in_transaction directly within TransactionManager itself is not necessary.

diff --git a/sqlx-core/src/transaction.rs b/sqlx-core/src/transaction.rs
index 9cd38aab..c2d179f0 100644
--- a/sqlx-core/src/transaction.rs
+++ b/sqlx-core/src/transaction.rs
@@ -32,6 +32,14 @@ pub trait TransactionManager {
 
     /// Starts to abort the active transaction or restore from the most recent snapshot.
     fn start_rollback(conn: &mut <Self::Database as Database>::Connection);
+
+    /// Returns the current transaction depth.
+    ///
+    /// Transaction depth indicates the level of nested transactions:
+    /// - Level 0: No active transaction.
+    /// - Level 1: A transaction is active.
+    /// - Level 2 or higher: A transaction is active and one or more SAVEPOINTs have been created within it.
+    fn get_transaction_depth(conn: &<Self::Database as Database>::Connection) -> usize;
 }

[mpyw]

I now understand what you mean by provided method. It means "default implementation" in other languages.

[mpyw]

@abonander I have an additional question regarding the SQLite refactoring approach.

Would it be better to keep the existing transaction_depth field within LockedSqliteHandle and add a synchronized field to SqliteConnection, ConnectionWorker, or WorkerSharedState? Or would it be preferable to completely move the transaction_depth field to one of these structures? Additionally, among SqliteConnection, ConnectionWorker, and WorkerSharedState, which one would be the most suitable candidate for holding this field?

---

[Edit]

I'll try on WorkerSharedState with AtomicUsize

↓

• Feat: Implementation for SQLite a66787d
• Refactor: Same approach for cached_statements_size 54c73b6

[mpyw]

@abonander Strictly speaking, it seems more correct to use RwLock rather than AtomicUsize and to keep it locked during operations. However, since RwLock requires handling errors, I chose this approach for now. If you prefer an implementation using RwLock, please let me know your thoughts.

---

[Edit]

I am considering adopting RwLock from parking_lot crate.

[mpyw]

@abonander If we were to replace the implementation with parking_lot::RwLock, it would look like this:

diff --git a/sqlx-sqlite/src/connection/worker.rs b/sqlx-sqlite/src/connection/worker.rs
index 6dda814a..d36b1df2 100644
--- a/sqlx-sqlite/src/connection/worker.rs
+++ b/sqlx-sqlite/src/connection/worker.rs
@@ -34,14 +34,14 @@ pub(crate) struct ConnectionWorker {
 }
 
 pub(crate) struct WorkerSharedState {
-    transaction_depth: AtomicUsize,
+    transaction_depth: parking_lot::RwLock<usize>,
     cached_statements_size: AtomicUsize,
     pub(crate) conn: Mutex<ConnectionState>,
 }
 
 impl WorkerSharedState {
     pub(crate) fn get_transaction_depth(&self) -> usize {
-        self.transaction_depth.load(Ordering::Acquire)
+        *self.transaction_depth.read()
     }
 
     pub(crate) fn get_cached_statements_size(&self) -> usize {
@@ -104,7 +104,7 @@ impl ConnectionWorker {
                 };
 
                 let shared = Arc::new(WorkerSharedState {
-                    transaction_depth: AtomicUsize::new(0),
+                    transaction_depth: parking_lot::RwLock::new(0),
                     cached_statements_size: AtomicUsize::new(0),
                     // note: must be fair because in `Command::UnlockDb` we unlock the mutex
                     // and then immediately try to relock it; an unfair mutex would immediately
@@ -193,12 +193,12 @@ impl ConnectionWorker {
                             update_cached_statements_size(&conn, &shared.cached_statements_size);
                         }
                         Command::Begin { tx } => {
-                            let depth = shared.transaction_depth.load(Ordering::Acquire);
+                            let mut depth = shared.transaction_depth.write();
                             let res =
                                 conn.handle
-                                    .exec(begin_ansi_transaction_sql(depth))
+                                    .exec(begin_ansi_transaction_sql(*depth))
                                     .map(|_| {
-                                        shared.transaction_depth.fetch_add(1, Ordering::Release);
+                                        *depth += 1;
                                     });
                             let res_ok = res.is_ok();
 
@@ -209,9 +209,9 @@ impl ConnectionWorker {
                                 // immediately otherwise it would remain started forever.
                                 if let Err(error) = conn
                                     .handle
-                                    .exec(rollback_ansi_transaction_sql(depth + 1))
+                                    .exec(rollback_ansi_transaction_sql(*depth + 1))
                                     .map(|_| {
-                                        shared.transaction_depth.fetch_sub(1, Ordering::Release);
+                                        *depth -= 1;
                                     })
                                 {
                                     // The rollback failed. To prevent leaving the connection
@@ -223,13 +223,13 @@ impl ConnectionWorker {
                             }
                         }
                         Command::Commit { tx } => {
-                            let depth = shared.transaction_depth.load(Ordering::Acquire);
+                            let mut depth = shared.transaction_depth.write();
 
-                            let res = if depth > 0 {
+                            let res = if *depth > 0 {
                                 conn.handle
-                                    .exec(commit_ansi_transaction_sql(depth))
+                                    .exec(commit_ansi_transaction_sql(*depth))
                                     .map(|_| {
-                                        shared.transaction_depth.fetch_sub(1, Ordering::Release);
+                                        *depth -= 1;
                                     })
                             } else {
                                 Ok(())
@@ -249,13 +249,13 @@ impl ConnectionWorker {
                                 continue;
                             }
 
-                            let depth = shared.transaction_depth.load(Ordering::Acquire);
+                            let mut depth = shared.transaction_depth.write();
 
-                            let res = if depth > 0 {
+                            let res = if *depth > 0 {
                                 conn.handle
-                                    .exec(rollback_ansi_transaction_sql(depth))
+                                    .exec(rollback_ansi_transaction_sql(*depth))
                                     .map(|_| {
-                                        shared.transaction_depth.fetch_sub(1, Ordering::Release);
+                                        *depth -= 1;
                                     })
                             } else {
                                 Ok(())

This approach would ensure that operations on transaction_depth are handled atomically as a single unit, avoiding potential race conditions that could occur with separate AtomicUsize operations. If you believe this implementation with parking_lot::RwLock is preferable to the current AtomicUsize approach, we can consider adopting it. What are your thoughts?

[abonander]

Merged in #3765