Sqlx Executors and Transactions

help
1

Why this code doesn't compile?

pub async fn f1(
    mut txn: Transaction<'_, MySql>,
)  {
    sqlx::query("").execute(&mut *txn).await;
    for i in 0..5 {
	    f2(&mut *txn).await;   // <-- Mismatched types
    // ^^^^^^^^^ expected `Transaction<'_, MySql>`, found `&mut MySqlConnection`
    }
}

pub async fn f2(
    mut txn: Transaction<'_, MySql>,
)  {

    for i in 0..5 {
	   sqlx::query("").execute(&mut *txn).await;
    }
}

I'm quite puzzled by this compiler error, how should I invoke f2 then?

Thanks!

2

txn is already of the type accepted by f2(); you don't need the &mut *. Just do:

f2(txn).await;
3

This will not work in a cycle unless Transaction is Copy which it is not.

@dani.pardo f2 needs to accept &mut Transaction and not Transaction so that you can call it with &mut txn (also without *). If it actually cannot as it is finishing transaction you will have to start new transaction each time you call f2.

Also please format code properly, with f2 call indented on the same level as for it is harder to see that it is in a cycle.

1 Like
5

I didn't consider nested transactions. But it seems to work though:

    for i in 0..5 {
    	let txn2 = txn.begin().await.unwrap();
    	f2(txn2).await;
    }

I think this will do it, thanks!

6

I would only use nested transactions if your application actually uses nested transaction semantics in some way, such as (conditionally) rolling back steps in a larger transaction without aborting the whole transaction. Savepoints aren't entirely free, though the actual overhead of creating one does depend on the database you're using. (Some databases don't support them at all, or only support them for some storage engines and not others, which also adds to the complexity.)

Your code as written doesn't appear to need nested transactions, so I would encourage you to pass the transaction around by reference when you need to share it; the code given in your original post passes it around by value, which is at the heart of your type-related surprise.

Edit: if you don't make other changes, trying to use nested transactions the way you are doing may also cause some surprising outcomes. fn2 would need to commit the nested transaction before dropping txn, or arrange for sqlx to automatically commit the nested transaction when dropped. The default sqlx behaviour when a transaction - nested or otherwise - is dropped is to roll it back.

3 Likes
7

Note that this also applies to f1 which in the first post does not commit either. It might be better idea to always use &mut Transaction with transaction method which makes it impossible to forget to commit.

I am also now wondering what sqlx does exactly when transaction is dropped, especially if rollback is not successfull: I expected it to panic, but, probably due to non-existence of async drop actual drop implementation just calls start_rollback and has a comment which I understand to mean that dropping transaction basically makes rollback happen later and any error from it be returned from seemingly unrelated function call, which if true will make debugging it harder. No wonder diesel simply has transaction method similar to what I referenced and no way to avoid using closures.

8

In the original code I posted, I ommited the commits for clarity, and in order to focus in the real problem which has to do with ownership.

Although I can live opening nested transactions, as @derspiny says, ideally, I'd like to use the original code, thus sending a single &mut Transaction across methods. But I the following won't compile:

pub async fn f2(
      txn: &mut Transaction<'_, MySql>,
)  {

    for i in 0..5 {
	   sqlx::query("").execute(txn).await;
    }
}

So we might be at the starting point.. is there any way to make the original code work?

9

Try:

sqlx::query("").execute(txn.as_mut()).await;

The execute() method takes E: Executor<'_, _>, not Transaction<'_, _>. The transaction has a DerefMut impl which is why &mut *txn works for owned Transaction.

10

Sure. I do something much like this a fair bit.

pub async fn f1(txn: &mut Transaction<'_, MySql>)  {
    sqlx::query("").execute(&mut *txn).await;
    for i in 0..5 {
        f2(&mut *txn).await;
    }
}

pub async fn f2(txn: &mut Transaction<'_, MySql>) {
    for i in 0..5 {
        sqlx::query("-- …").execute(&mut *txn).await;
    }
}

I didn't test this as I don't have my tools handy at this exact moment, but you were on the right track with reborrowing the transaction in order to reuse it. Some of the reborrows in the code above may also be redundant, though I'm pretty sure that none of them are incorrect.

The only thing you can't do with this structure is have either f1 or f2 commit the transaction. sqlx's Transaction::commit method requires ownership of the underlying transaction. The usual structure I use is for the same function to contain both the begin() and the commit(), both for this reason and to improve readability in complex transactions, but if you do want f1 to commit a transaction started by the caller, then it'd need to receive txn by value:

pub async fn f1(mut txn: Transaction<'_, MySql>)  {
    sqlx::query("").execute(&mut txn).await; // note the removed *
    for i in 0..5 {
        f2(&mut txn).await; // ditto
    }
    txn.commit().await;
}

pub async fn f2(txn: &mut Transaction<'_, MySql>) {
    for i in 0..5 {
        sqlx::query("-- …").execute(&mut *txn).await;
    }
}

Since f1 reuses the transaction, it can't give up ownership, unless it gets ownership back. Passing the transaction into and back out of f2 would be pointlessly complicated in most systems; passing a reference (as above) is likely a better choice even if f1 takes ownership of the transaction.

2 Likes