sqlx icon indicating copy to clipboard operation
sqlx copied to clipboard

sqlx lost transaction support starting version 0.7.0

Open goodjius opened this issue 1 year ago • 6 comments

Bug Description

Get error: .fetch_one(&mut tx) | --------- ^^^^^^^ the trait Executor<'_> is not implemented for &mut Transaction<'_, Sqlite>. This is no problem with sqlx 0.6.x. Compiler start to complain starting 0.7.0

Minimal Reproduction

        let mut tx = pool.begin().await?;
        let res = sqlx::query("DELETE FROM \"testcases\" WHERE id = $1")
            .bind(id)
            .execute(&mut tx)
            .await?
            .rows_affected();

        tx.commit().await?;

Info

  • SQLx version: 0.7.x
  • SQLx features enabled: sqlite
  • Database server and version: SQLite 3.39.5
  • Operating system: Mac OS
  • rustc --version: 1.71.0

goodjius avatar Aug 19 '23 01:08 goodjius

Please read the 0.7 changelog and look at the current examples. This is not a bug, just a breaking change how transactions are used

dragonnn avatar Aug 19 '23 07:08 dragonnn

@goodjius . I was struggling with the same issue but exact advice from the changelog did not help, also I did not find example of the transaction for sqlite.

Here is the relevant part of the changelog mentioned by @dragonnn

The Executor impls for Transaction and PoolConnection have been deleted because they cannot exist in the new crate architecture without rewriting the Executor trait entirely.
To fix this breakage, simply add a dereference where an impl Executor is expected, as they both dereference to the inner connection type which will still implement it:
&mut transaction -> &mut *transaction
&mut connection -> &mut *connection

What helped was using as_mut. (although I am using connection.transaction)

My example

        self.conn.transaction(|tx| Box::pin(async move {
            sqlx::query(
                r#"...
            "#,
            ).bind(...)
             .execute(tx.as_mut())
             .await?;


            Ok::<_, eyre::Error>(())
        })).await?;

In your example this would be:

        let mut tx = pool.begin().await?;
        let res = sqlx::query("DELETE FROM \"testcases\" WHERE id = $1")
            .bind(id)
            .execute(tx.as_mut()) // <-- here
            .await?
            .rows_affected();

        tx.commit().await?;

dvush avatar Sep 12 '23 14:09 dvush

Hi, sorry for bumping this old issue. I stumbled across the same problem, I wanted to use a transaction in a personal project and I wasn't able to get it to work at first. The examples in this repo were able to help me and I got it working, but I think it would be nice to add a short example on how to use Transaction as an executor to the docs.

Is that a good idea? If yes, I would submit a PR 😄.

Lachstec avatar Mar 18 '24 19:03 Lachstec

@Lachstec Yes please do it.

fosskers avatar Jun 25 '24 01:06 fosskers

@fosskers Okay, I will get to it if I find some time today.

Lachstec avatar Jun 25 '24 06:06 Lachstec

@fosskers I've submitted a PR where I added a Snippet inspired by the one in this issue, hope that it is fine like that :)

Lachstec avatar Jun 25 '24 07:06 Lachstec

Currently, there are only these scenarios available:

/// Scenario 1: pool/tx interface + multiple queries
///
/// TLDR we should never want this ambiguity
///
/// for a single query this one will work just fine
pub async fn run(executor: impl Executor<'_, Database = Postgres>) {
    sqlx::query("SELECT 1").execute(executor).await.unwrap();
    sqlx::query("SELECT 1").execute(executor).await.unwrap();
    // error:                       ^^^^^^^^ value used after move
}

/// Scenario 2: we want to use the same executor for multiple queries
///
/// we can demand Executor: Copy
pub async fn run_copy(executor: impl Executor<'_, Database = Postgres> + std::marker::Copy) {
    sqlx::query("SELECT 1").execute(executor).await.unwrap();
    run_copy(executor).await;
    sqlx::query("SELECT 1").execute(executor).await.unwrap();
}

/// Scenario 3: we want to use the same executor for multiple queries
///
/// We can specify an executor type as a Transaction explicitly
/// This allows us to use transactions as normal with multiple queries
#[async_recursion::async_recursion]
pub async fn run_atomic(executor: &mut Transaction<'_, Postgres>) {
    sqlx::query("SELECT 1").execute(&mut **executor).await.unwrap();
    run_atomic(executor).await;
    sqlx::query("SELECT 1").execute(&mut **executor).await.unwrap();
}

pub async fn outer_logic(pool: sqlx::Pool<Postgres>) {
    {
        // Scenario 2 use
        // w/o transaction -- works
        run_copy(&pool).await;
        run_copy(&pool).await;
        // with transaction won't work
    }

    {
        // Scenario 3 use
        // w/o transaction
        // run_atomic(&pool).await; // error: cannot use pool

        // with transaction -- works
        let mut tx = pool.begin().await.unwrap();
        sqlx::query("SELECT 1").execute(&mut *tx).await.unwrap();
        run_atomic(&mut tx).await;
        sqlx::query("SELECT 1").execute(&mut *tx).await.unwrap();
        tx.commit().await.unwrap();
    }
}

I prefer Scenario 3 as universal. And the cost is small: you should make every query in transactions, which is not a big deal most of the time.

awnion avatar Jul 21 '24 19:07 awnion