Skip to content
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

Feature Request: Expose transaction_depth with a Read-Only Getter Method #3426

Open
mpyw opened this issue Aug 10, 2024 · 2 comments · May be fixed by #3427
Open

Feature Request: Expose transaction_depth with a Read-Only Getter Method #3426

mpyw opened this issue Aug 10, 2024 · 2 comments · May be fixed by #3427
Labels
enhancement New feature or request

Comments

@mpyw
Copy link

mpyw commented Aug 10, 2024

Is your feature request related to a problem? Please describe.

transaction_depth is currently marked as pub(crate) in the connection struct, which makes it inaccessible from outside the crate. This limitation poses a challenge when working with advisory locks, as described in the following library's README:

mpyw/laravel-database-advisory-lock: Advisory Locking Features for Postgres/MySQL/MariaDB on Laravel

Session-level advisory locks must be acquired before entering a transaction and released only after the transaction has been committed. To ensure consistency, it's crucial to verify whether a transaction is already active before acquiring the lock.

Describe the solution you'd like

I would like to request a read-only Getter method for the transaction_depth field to be added, so that it can be accessed from outside the crate.

Describe alternatives you've considered

An alternative could be to use a custom wrapper around the connection, but this would add unnecessary complexity and overhead. Another option might be to manage transaction depth manually in user code, but this is error-prone and defeats the purpose of the existing transaction_depth field.

Additional context

Providing access to transaction_depth would allow for safer and more consistent handling of nested transactions, particularly in scenarios involving advisory locks.

@mpyw mpyw added the enhancement New feature or request label Aug 10, 2024
@mpyw
Copy link
Author

mpyw commented Aug 11, 2024

The trait method signature would be:

/// 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(&self) -> BoxFuture<'_, Result<usize, Error>>;

PgConnection MySqlConnection are easy to be inplemented:

// MySqlConnection
fn get_transaction_depth(&self) -> usize {
    self.inner.transaction_depth
}
// PgConnection
fn get_transaction_depth(&self) -> usize {
    self.transaction_depth
}

But SqliteConnection requires asynchronously locking:

// SqliteConnection
async fn get_transaction_depth(&mut self) -> Result<usize, Error> {
    Ok(self.lock_handle().await?.guard.transaction_depth)
}

How should we achive this?

@mpyw
Copy link
Author

mpyw commented Aug 11, 2024

Trait splitting?

pub trait TransactionDepth {
    /// Returns the current transaction depth synchronously.
    ///
    /// 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(&self) -> usize;
}

pub trait AsyncTransactionDepth {
    /// Returns the current transaction depth asynchronously.
    ///
    /// 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(&mut self) -> BoxFuture<'_, Result<usize, Error>>;
}
impl TransactionDepth for PgConnection {
    fn get_transaction_depth(&self) -> usize {
        self.transaction_depth
    }
}

impl TransactionDepth for MySqlConnection {
    fn get_transaction_depth(&self) -> usize {
        self.inner.transaction_depth
    }
}

impl AsyncTransactionDepth for SqliteConnection {
    fn get_transaction_depth(&mut self) -> BoxFuture<'_, Result<usize, Error>> {
        Box::pin(async move {
            Ok(self.lock_handle().await?.guard.transaction_depth)
        })
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant