jonwagner / Insight.Database

Fast, lightweight .NET micro-ORM
Other
861 stars 145 forks source link

Provider doesn't use the DbConnectionWrapper's inner transaction when none is supplied to the BulkCopyAsync method #397

Closed Greeng0 closed 5 years ago

Greeng0 commented 5 years ago

Describe the bug

If we try to invoke BulkCopyAsync from a DbConnectionWrapper that has an inner transaction already open, that transaction is not used unless we explicitly pass it as a parameter to BulkCopyAsync.

We end up with a System.InvalidOperationException: 'Unexpected existing transaction.' being thrown as a new transaction is created by the bulk copy: https://stackoverflow.com/a/19117313/1036859.

Steps to reproduce

The following code reproduces the exception:

var sqlCon = new SqlConnection(@"<connection string here>");
DbConnectionWrapper wrapper = new DbConnectionWrapper(sqlCon);
wrapper.Open();
using (DbConnectionWrapper connection = wrapper.BeginAutoTransaction())
{
    try
    {
        await connection.BulkCopyAsync("MyTableName", new List<MyClass> { myClass });
        connection.Commit();
    }
    catch
    {
        connection.Rollback();
        throw;
    }
}

Expected behavior

The transaction in the DbConnectionWrapper should be used, as is happening when we use the non-asynchronous BulkCopy method without explicitly sending it a transaction parameter.

Greeng0 commented 5 years ago

I have submitted a pull request to resolve this issue where I added the following override to DbConnectionWrapperInsightDbProvider.

It is basically the same as the BulkCopy override in the same class, where it simply forwards transaction ?? wrapped.InnerTransaction to the base implementation of the method.

public override Task BulkCopyAsync(IDbConnection connection, string tableName, IDataReader reader, Action<InsightBulkCopy> configure, InsightBulkCopyOptions options, IDbTransaction transaction, CancellationToken cancellationToken)
{
    DbConnectionWrapper wrapped = (DbConnectionWrapper)connection;

    return base.BulkCopyAsync(connection, tableName, reader, configure, options, transaction ?? wrapped.InnerTransaction, cancellationToken);
}

Feel free to reject the pull request if I missed something (I've never contributed to this repository before) and you'd rather investigate yourselves.

jonwagner commented 5 years ago

Good fix. This is in 6.2.9.