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.

Sign up for GitHub

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

Implement transaction options API #1827

Closed
wants to merge 6 commits into from
Closed

Implement transaction options API #1827

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b3fc0c9
implement transaction options API and tests
andrewwhitehead Apr 21, 2022
82efe5c
updates to support 'any' feature
andrewwhitehead Apr 21, 2022
87ca247
fix formatting
andrewwhitehead Apr 21, 2022
afba195
move table creation
andrewwhitehead Apr 21, 2022
ca7a9ad
mysql/mariadb version compatibility fixes
andrewwhitehead Apr 21, 2022
2b09ca9
fix unused variable warning
andrewwhitehead May 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 58 additions & 5 deletions sqlx-core/src/acquire.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::database::Database;
use crate::error::Error;
use crate::pool::{MaybePoolConnection, Pool, PoolConnection};
use crate::transaction::Transaction;
use crate::transaction::{Transaction, TransactionManager};
use futures_core::future::BoxFuture;
use std::ops::{Deref, DerefMut};

Expand Down Expand Up @@ -77,6 +77,11 @@ pub trait Acquire<'c> {
fn acquire(self) -> BoxFuture<'c, Result<Self::Connection, Error>>;

fn begin(self) -> BoxFuture<'c, Result<Transaction<'c, Self::Database>, Error>>;

fn begin_with(
self,
options: <<Self::Database as Database>::TransactionManager as TransactionManager>::Options,
) -> BoxFuture<'c, Result<Transaction<'c, Self::Database>, Error>>;
}

impl<'a, DB: Database> Acquire<'a> for &'_ Pool<DB> {
Expand All @@ -92,7 +97,22 @@ impl<'a, DB: Database> Acquire<'a> for &'_ Pool<DB> {
let conn = self.acquire();

Box::pin(async move {
Transaction::begin(MaybePoolConnection::PoolConnection(conn.await?)).await
Transaction::begin_with(
MaybePoolConnection::PoolConnection(conn.await?),
Default::default(),
)
.await
})
}

fn begin_with(
self,
options: <<Self::Database as Database>::TransactionManager as TransactionManager>::Options,
) -> BoxFuture<'static, Result<Transaction<'a, DB>, Error>> {
let conn = self.acquire();

Box::pin(async move {
Transaction::begin_with(MaybePoolConnection::PoolConnection(conn.await?), options).await
})
}
}
Expand Down Expand Up @@ -120,7 +140,18 @@ macro_rules! impl_acquire {
'c,
Result<crate::transaction::Transaction<'c, $DB>, crate::error::Error>,
> {
crate::transaction::Transaction::begin(self)
crate::transaction::Transaction::begin_with(self, Default::default())
}

#[inline]
fn begin_with(
self,
options: <<Self::Database as crate::database::Database>::TransactionManager as crate::transaction::TransactionManager>::Options,
) -> futures_core::future::BoxFuture<
'c,
Result<crate::transaction::Transaction<'c, $DB>, crate::error::Error>,
> {
crate::transaction::Transaction::begin_with(self, options)
}
}

Expand All @@ -144,7 +175,18 @@ macro_rules! impl_acquire {
'c,
Result<crate::transaction::Transaction<'c, $DB>, crate::error::Error>,
> {
crate::transaction::Transaction::begin(&mut **self)
crate::transaction::Transaction::begin_with(&mut **self, Default::default())
}

#[inline]
fn begin_with(
self,
options: <<Self::Database as crate::database::Database>::TransactionManager as crate::transaction::TransactionManager>::Options,
) -> futures_core::future::BoxFuture<
'c,
Result<crate::transaction::Transaction<'c, $DB>, crate::error::Error>,
> {
crate::transaction::Transaction::begin_with(&mut **self, options)
}
}

Expand All @@ -170,7 +212,18 @@ macro_rules! impl_acquire {
't,
Result<crate::transaction::Transaction<'t, $DB>, crate::error::Error>,
> {
crate::transaction::Transaction::begin(&mut **self)
crate::transaction::Transaction::begin_with(&mut **self, Default::default())
}

#[inline]
fn begin_with(
self,
options: <<Self::Database as crate::database::Database>::TransactionManager as crate::transaction::TransactionManager>::Options,
) -> futures_core::future::BoxFuture<
't,
Result<crate::transaction::Transaction<'t, $DB>, crate::error::Error>,
> {
crate::transaction::Transaction::begin_with(&mut **self, options)
}
}
};
Expand Down
7 changes: 5 additions & 2 deletions sqlx-core/src/any/connection/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,14 @@ impl Connection for AnyConnection {
delegate_to_mut!(self.ping())
}

fn begin(&mut self) -> BoxFuture<'_, Result<Transaction<'_, Self::Database>, Error>>
fn begin_with(
&mut self,
options: (),
) -> BoxFuture<'_, Result<Transaction<'_, Self::Database>, Error>>
where
Self: Sized,
{
Transaction::begin(self)
Transaction::begin_with(self, options)
}

fn cached_statements_size(&self) -> usize {
Expand Down
23 changes: 18 additions & 5 deletions sqlx-core/src/any/transaction.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,27 +10,40 @@ pub struct AnyTransactionManager;

impl TransactionManager for AnyTransactionManager {
type Database = Any;
type Options = ();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, I think this could be a struct containing a TransactionOptions for each database, and then the implementation picks the corresponding one:

#[derive(Default)]
pub struct AnyTransactionOptions {
    #[cfg(feature = "postgres")]
    postgres: PgTransactionOptions

    #[cfg(feature = "mysql")]
    mysql: MySqlTransactionOptions,

    // etc.
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

This could then have setters to replace each type wholesale:

impl AnyTransactionOptions {
    #[cfg(feature = "postgres")]
    pub fn postgres(self, postgres: PgTransactionOptions) -> Self {
        Self { postgres, ..self }
    }

    // etc.
}

Or even have setters for "best effort" options:

impl AnyTransactionOptions {
    pub fn isolation_level_if_supported(self, ilvl: IsolationLevel) -> Self {
        #[cfg(feature = "postgres")]
        self.postgres = self.postgres.isolation_level(ilvl);

        #[cfg(feature = "mysql")]
        self.mysql = self.mysql.isolation_level(ilvl);

        self
    }
}

(Since PgIsolationLevel and MySqlIsolationLevel are identical, they don't need to be separate enums.)


fn begin(conn: &mut AnyConnection) -> BoxFuture<'_, Result<(), Error>> {
fn begin_with(conn: &mut AnyConnection, _options: ()) -> BoxFuture<'_, Result<(), Error>> {
match &mut conn.0 {
#[cfg(feature = "postgres")]
AnyConnectionKind::Postgres(conn) => {
<crate::postgres::Postgres as Database>::TransactionManager::begin(conn)
<crate::postgres::Postgres as Database>::TransactionManager::begin_with(
conn,
Default::default(),
)
}

#[cfg(feature = "mysql")]
AnyConnectionKind::MySql(conn) => {
<crate::mysql::MySql as Database>::TransactionManager::begin(conn)
<crate::mysql::MySql as Database>::TransactionManager::begin_with(
conn,
Default::default(),
)
}

#[cfg(feature = "sqlite")]
AnyConnectionKind::Sqlite(conn) => {
<crate::sqlite::Sqlite as Database>::TransactionManager::begin(conn)
<crate::sqlite::Sqlite as Database>::TransactionManager::begin_with(
conn,
Default::default(),
)
}

#[cfg(feature = "mssql")]
AnyConnectionKind::Mssql(conn) => {
<crate::mssql::Mssql as Database>::TransactionManager::begin(conn)
<crate::mssql::Mssql as Database>::TransactionManager::begin_with(
conn,
Default::default(),
)
}
}
}
Expand Down
15 changes: 14 additions & 1 deletion sqlx-core/src/connection.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::database::{Database, HasStatementCache};
use crate::error::Error;
use crate::transaction::Transaction;
use crate::transaction::{Transaction, TransactionManager};
use futures_core::future::BoxFuture;
use log::LevelFilter;
use std::fmt::Debug;
Expand All @@ -27,6 +27,19 @@ pub trait Connection: Send {
///
/// Returns a [`Transaction`] for controlling and tracking the new transaction.
fn begin(&mut self) -> BoxFuture<'_, Result<Transaction<'_, Self::Database>, Error>>
where
Self: Sized,
{
Self::begin_with(self, Default::default())
}

/// Begin a new transaction or establish a savepoint within the active transaction.
///
/// Returns a [`Transaction`] for controlling and tracking the new transaction.
fn begin_with(
&mut self,
options: <<Self::Database as Database>::TransactionManager as TransactionManager>::Options,
) -> BoxFuture<'_, Result<Transaction<'_, Self::Database>, Error>>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm sorry, I should have looked at this PR before releasing 0.6.0 because this is a breaking change.

While it's unlikely that any implementation of this trait exists outside SQLx, since we haven't sealed any of them it's not impossible.

where
Self: Sized;

Expand Down
7 changes: 5 additions & 2 deletions sqlx-core/src/mssql/connection/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,14 @@ impl Connection for MssqlConnection {
self.execute("/* SQLx ping */").map_ok(|_| ()).boxed()
}

fn begin(&mut self) -> BoxFuture<'_, Result<Transaction<'_, Self::Database>, Error>>
fn begin_with(
&mut self,
options: (),
) -> BoxFuture<'_, Result<Transaction<'_, Self::Database>, Error>>
where
Self: Sized,
{
Transaction::begin(self)
Transaction::begin_with(self, options)
}

#[doc(hidden)]
Expand Down
3 changes: 2 additions & 1 deletion sqlx-core/src/mssql/transaction.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ pub struct MssqlTransactionManager;

impl TransactionManager for MssqlTransactionManager {
type Database = Mssql;
type Options = ();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think I would prefer to see at least an empty MssqlTransactionOptions type so that we can safely add things to it later without it being a breaking change.


fn begin(conn: &mut MssqlConnection) -> BoxFuture<'_, Result<(), Error>> {
fn begin_with(conn: &mut MssqlConnection, _options: ()) -> BoxFuture<'_, Result<(), Error>> {
Box::pin(async move {
let depth = conn.stream.transaction_depth;

Expand Down
8 changes: 6 additions & 2 deletions sqlx-core/src/mysql/connection/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::error::Error;
use crate::mysql::protocol::statement::StmtClose;
use crate::mysql::protocol::text::{Ping, Quit};
use crate::mysql::statement::MySqlStatementMetadata;
use crate::mysql::transaction::MySqlTransactionOptions;
use crate::mysql::{MySql, MySqlConnectOptions};
use crate::transaction::Transaction;
use futures_core::future::BoxFuture;
Expand Down Expand Up @@ -94,10 +95,13 @@ impl Connection for MySqlConnection {
!self.stream.wbuf.is_empty()
}

fn begin(&mut self) -> BoxFuture<'_, Result<Transaction<'_, Self::Database>, Error>>
fn begin_with(
&mut self,
options: MySqlTransactionOptions,
) -> BoxFuture<'_, Result<Transaction<'_, Self::Database>, Error>>
where
Self: Sized,
{
Transaction::begin(self)
Transaction::begin_with(self, options)
}
}
2 changes: 1 addition & 1 deletion sqlx-core/src/mysql/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ pub use options::{MySqlConnectOptions, MySqlSslMode};
pub use query_result::MySqlQueryResult;
pub use row::MySqlRow;
pub use statement::MySqlStatement;
pub use transaction::MySqlTransactionManager;
pub use transaction::{MySqlIsolationLevel, MySqlTransactionManager, MySqlTransactionOptions};
pub use type_info::MySqlTypeInfo;
pub use value::{MySqlValue, MySqlValueFormat, MySqlValueRef};

Expand Down
Loading