Bug: Exception "A transaction is already in progress; nested/concurrent transactions aren't supported." when using TransactionScope with PostgreSQL bulk operations

[RodolfoFerreira]

Bug Description

Hi!

I was trying to use the BulkOperations extensions with a NpgsqlConnection within a TransactionScope but was faced the exception below:

Exception Message:

System.InvalidOperationException: 'A transaction is already in progress; nested/concurrent transactions aren't supported.'
   at Npgsql.NpgsqlConnection.BeginTransaction(IsolationLevel level, Boolean async, CancellationToken cancellationToken)
   at RepoDb.NpgsqlConnectionExtension.TransactionalExecuteAsync[TResult](NpgsqlConnection connection, Func`1 executeAsync, NpgsqlTransaction transaction, CancellationToken cancellationToken)
   at RepoDb.NpgsqlConnectionExtension.BinaryImportAsync(NpgsqlConnection connection, String tableName, DataTable table, Nullable`1 rowState, IEnumerable`1 mappings, DbFieldCollection dbFields, Nullable`1 bulkCopyTimeout, Nullable`1 batchSize, BulkImportIdentityBehavior identityBehavior, IDbSetting dbSetting, NpgsqlTransaction transaction, CancellationToken cancellationToken)
   at RepoDb.NpgsqlConnectionExtension.<>c__DisplayClass24_0.<<BinaryBulkUpdateBaseAsync>b__2>d.MoveNext()
--- End of stack trace from previous location ---
   at RepoDb.NpgsqlConnectionExtension.PseudoBasedBinaryImportAsync(NpgsqlConnection connection, String tableName, Nullable`1 bulkCopyTimeout, DbFieldCollection dbFields, Func`1 getPseudoTableName, Func`1 getMappings, Func`2 binaryImportAsync, Func`1 getMergeToPseudoCommandText, Action`1 setIdentities, IEnumerable`1 qualifiers, Boolean isBinaryBulkInsert, BulkImportIdentityBehavior identityBehavior, BulkImportPseudoTableType pseudoTableType, IDbSetting dbSetting, NpgsqlTransaction transaction, CancellationToken cancellationToken)
   at RepoDb.NpgsqlConnectionExtension.PseudoBasedBinaryImportAsync(NpgsqlConnection connection, String tableName, Nullable`1 bulkCopyTimeout, DbFieldCollection dbFields, Func`1 getPseudoTableName, Func`1 getMappings, Func`2 binaryImportAsync, Func`1 getMergeToPseudoCommandText, Action`1 setIdentities, IEnumerable`1 qualifiers, Boolean isBinaryBulkInsert, BulkImportIdentityBehavior identityBehavior, BulkImportPseudoTableType pseudoTableType, IDbSetting dbSetting, NpgsqlTransaction transaction, CancellationToken cancellationToken)
   at RepoDb.NpgsqlConnectionExtension.BinaryBulkUpdateBaseAsync(NpgsqlConnection connection, String tableName, DataTable table, Nullable`1 rowState, IEnumerable`1 qualifiers, IEnumerable`1 mappings, Nullable`1 bulkCopyTimeout, Nullable`1 batchSize, Boolean keepIdentity, BulkImportPseudoTableType pseudoTableType, NpgsqlTransaction transaction, CancellationToken cancellationToken)
   at RepoDb.NpgsqlConnectionExtension.BinaryBulkUpdateAsync(NpgsqlConnection connection, String tableName, DataTable table, Nullable`1 rowState, IEnumerable`1 qualifiers, IEnumerable`1 mappings, Nullable`1 bulkCopyTimeout, Nullable`1 batchSize, Boolean keepIdentity, BulkImportPseudoTableType pseudoTableType, NpgsqlTransaction transaction, CancellationToken cancellationToken)

I noticed that the exception was thrown only when using async methods, like BinaryBulkInsertAsync, and in the same scenario if used sync methods nothing wrong happened.

I took the liberty to investigate further and reached into the NpgsqlWrapper in the RepoDb.PostgreSQL.BulkOperations extensions. Inside that, the methods TransactionalExecute and TransactionalExecuteAsync are used in the functions that execute bulk operations against the database.

They can receive a NpgsqlTransaction as a parameter, and if not, create a new transaction to execute the command. However, the condition used to check wether to use or not an existing transaction differs in each functions.

In the sync TransactionalExecute, the flag is checked as below:

var hasTransaction = (transaction == null || Transaction.Current != null);

But in the async TransactionalExecuteAsync, we got:

var hasTransaction = transaction != null;

So, when using a TransactionScope from System.Transactions, the async function tries to open another transaction and throws the exception, but the sync method don't.

I guess it is something that was already seen before, but is missing in the async method.

I would love to contribute with a PR to fix this issue!

Library Version:

Example: RepoDb v1.13.1 and RepoDb.PostgreSql v1.13.1 and RepoDb.PostgreSql.BulkOperations v1.13.1

[RodolfoFerreira]

Hello again!!!

I was faced another problem related with this.

In some database operations in my code, I use a explict transaction, passing it to the bulk insert operation inside RepoDb.

The same error was thrown, so I looked inside NpsqlWrapper and found and mislogic in the condition to check if we already have a transaction.

var hasTransaction = (transaction == null || Transaction.Current != null);

If we use a explicit transaction, the logic is falsy, because the variable must NOT be null so that hasTransaction get to be true. This cause the code to start another transaction with one already existing in the connection throwing the error.

I would love to contribute with another PR to fix this issue!

Library Version:

Example: RepoDb v1.13.1 and RepoDb.PostgreSql v1.13.1 and RepoDb.PostgreSql.BulkOperations v1.13.1

[mikependon]

Closing this issue. Currently, RepoDB has some pending changes on its extension library which still limit me to push a new beta release on main package. When do you think you need a new pack for this?

[RodolfoFerreira]

When do you think it would be possible to release a beta version? It would be great to have a new version as soon as possible, but I can try to use it building locally for now...

If I build and use the release from the master branch, would I have any problems with new code that it's not ready for a beta version, like you said?

Thanks so much for the reply!

[RodolfoFerreira]

When do you think it would be possible to release a beta version? It would be great to have a new version as soon as possible, but I can try to use it building locally for now...

If I build and use the release from the master branch, would I have any problems with new code that it's not ready for a beta version, like you said?

Thanks so much for the reply!

Hey @mikependon ,

Just wanted to drop a quick message to see if there are any updates on this issue. I'm sorry if I'm being annoying! Just wanted a tip to see what can I do with my project :)

Cheers

[mikependon]

You are not annoying us, FYI. We are happy and glad that you keep pushing for follow-ups, and please continue to do so. 💯

We can release a alpha (not beta) of the next version since I know there is still a pending bug on the MySql extension after some refactoring. We can do it before EOD of CET.

Would that work for you?

[RodolfoFerreira]

You are not annoying us, FYI. We are happy and glad that you keep pushing for follow-ups, and please continue to do so. 💯

We can release a alpha (not beta) of the next version since I know there is still a pending bug on the MySql extension after some refactoring. We can do it before EOD of CET.

Would that work for you?

It would be very nice!

Will I be able to download it from nuget itself or should I get it from the website instead?

Thank you!

[mikependon]

The alpha versions is now available including the extended libraries. It has all the latest changes/PRs, including yours. They are not fully tested yet.

Install-Package RepoDb.SqlServer -Version 1.13.1-alpha1

Please feel free to revert if you have questions and thanks!

[mikependon]

By the way, it does not mean it is not fully tested, it is then on low-quality. Our >10K Unit and Integration Tests are passing on that latest release, so the quality are there. But, we are not giving a guarantee as it is not yet a full official release of that version.

[RodolfoFerreira]

Thank you very much!

Just got it here! Gonna test my use cases here!

Closing the issue!