querybuilder icon indicating copy to clipboard operation
querybuilder copied to clipboard

InsertGetIdAsync (or similar) should not require DbTransaction – honor IDbTransaction abstraction

Open manuelcim opened this issue 7 months ago • 0 comments

Hi, I'm running into a limitation when using InsertGetIdAsync (and similar methods) in conjunction with a custom implementation of IDbTransaction.

Although the method signature indicates support for any IDbTransaction

public Task<T> InsertGetIdAsync<T>(object data, IDbTransaction transaction = null, ...)

internally, the method attempts to cast transaction to DbTransaction or a provider-specific concrete class, like SqlTransaction. This results in a runtime InvalidCastException when using a valid, spec-compliant custom wrapper like:

public class MyTransaction : IDbTransaction { ... }

This violates the expected abstraction and makes it impossible to wrap the transaction logic for tracking, scoping, or ownership control purposes.

Why This Matters?

  • IDbTransaction exists to abstract away the implementation detail — this cast defeats that purpose.
  • Libraries like Dapper handle this correctly by accepting IDbTransaction and only downcasting when absolutely necessary (and typically using a safe pattern or provider checks).
  • Custom transaction scopes are common in applications that follow layered or hexagonal architecture.

Proposal

  • Avoid downcasting directly to DbTransaction.
  • If provider-specific logic is necessary, use DbProviderFactory or check the underlying connection type to handle it safely.
  • Or: expose alternative overloads that accept provider-specific types if needed, but allow IDbTransaction for general use.

Reproduction Steps var tx = new MyWrappedTransaction(innerIDbTransaction); await db.Query("MyTable").InsertGetIdAsync<int>(data, transaction: tx);

Expected: works fine Actual: Unable to cast object of type 'MyWrappedTransaction' to 'System.Data.Common.DbTransaction'

Thanks for the great work on SqlKata.

manuelcim avatar May 30 '25 13:05 manuelcim