`transaction` API is problematic, can we just have a `getConnection` style API?

[mberndt123]

Hi,

the current transaction API is difficult to integrate with libraries like cats-effect or ZIO. The problem is that you're expected to pass a block to it that will perform side effects, and when the block finishes, the transaction is committed. IOW, the lifecycle of the transaction object is tied to the control flow of the application. But side effects and imperative control flow is not how these libraries normally work. While it's possible to perform side effects using things like ZIO's Runtime API, this is problematic because it interferes with fiber interruption.
There are also other scenarios where this API is problematic, for instance when you want to interact with any kind of asynchronous API (e. g. Future-based) within your transaction or when you're already using some other API for resource management, e. g. a scala.util.Using.Manager . While I can see the advantage of not being able to forget calling close, the conflation of lifecycle management and control flow makes these situations difficult to deal with.

The getAutoCommitClientConnection method is better in this regard, but just because you require more control over the transaction lifecycle doesn't mean you want auto-commit. A simple solution to deal with all this is to just add some method like startTransaction that gives you a DbApi.Txn object and that you need to call close() on yourself. This should be easy to add as DbApi is already a subtype of AutoCloseable.

[lihaoyi]

Adding a lower-level API that you can use in asynchronous workflows seems fine to me, it can live alongside the high-level blocking API without issue. Feel free to send a PR with a proposed design

[mberndt123]

OK, this is slightly trickier than I first thought.

The way I think it should work is that you have a startTransaction method that returns a DbApi.Txn object, and you are responsible for calling .close() on it when you're done. The problem is: when .close() is called, we don't know if whatever the user was doing with the transaction was successful or not, and hence we don't know which TransactionListener hooks need to be called.

I see three possible solutions to this.

1. close() commits the transaction. If an error occurs, it's the user's responsibility to call rollback, thus turning the subsequent commit into a no-op. TransactionListener hooks are ignored as we don't know which ones to call
2. remove close(), don't extend AutoCloseable, add def close(err: Option[Throwable]). If there's an error, roll back and call the rollback hooks, adding any exceptions thrown by the hooks to the original error using addSuppressed. If there's no error, commit and call the commit hooks.
3. simplify the TransactionListener interface to only have two hooks: before start of transaction and after end of transaction (however it ends). I actually think that that API is problematic in its current form anyway because it's meant to be used for resource cleanup, but these hooks can be called more than once (when you call rollback on the transaction object) and freeing the same resource twice is a bug.

I personally would prefer the first option. I am not a fan of the TransactionListener API anyway, the second option would probably be awkward to use in practice, and both the second and third option would open a whole other can of worms that I consider to be beyond the scope of this particular issue. But I'd like to hear your thoughts on this.

[lihaoyi]

Not that familiar with the transaction listener API, maybe @aboisvert has some opinions here?

[lihaoyi]

In the absence of any opinion from @aboisvert, I'd be happy for the low-level startTransaction/close API to not call TransactionListener hooks at all. After all, people using a low level API would want low-level control over what exactly happens, and they can always just run whatever code is necessary themselves at various points in the transaction lifecycle.