Suggestion: add ?transaction:bool to Dream.sql

[aantron]

As maybe one of the most convenient "simple" ways to do a transaction with automatic rollback on failure.

cc @thangngoc89

[thangngoc89]

I would suggest ?transaction: true is the default mode and users can opt out. I can't think of any use cases that would need to be performed without a transaction

[aantron]

I would suggest ?transaction: true is the default mode and users can opt out. I can't think of any use cases that would need to be performed without a transaction

Single-query operations, and some cases where the order makes it safe not to use a transaction somehow. Transactions have performance consequences IIRC.

However, ~transaction:true is indeed the right default because it makes safety the default in a concise way. Users can add ~transaction:false if they are optimizing and know what they are doing.

[aantron]

Another use case where you don't want a transaction at Dream.sql is when you want to grab a connection from the pool (what Dream.sql does) and hold on to it for two separate transactions for some reason. That's probably questionable, because it means you are running some big operation in between them, most likely, and you probably don't want to actually hold a connection during it. But, this may be right in some user's code. Anyway, that's probably rare and obscure.

[tcoopman]

I haven't looked at the implementation/API, just following the discussion, so I might be off.

From my experience having default transactions is not something a lot of other frameworks do (there are frameworks that do but I haven't worked with them in a long time). If you want a transaction you have to do it explicitly. Take a look at how ecto does it for example https://hexdocs.pm/ecto/Ecto.Multi.html

It's correct that this is a bit more work, but it makes everything explicit. When and when not to use transactions is something developers should think about IMO. Maybe having 2 different functions, one with and one without transaction would be an option.

[aantron]

Take a look at how ecto does it for example https://hexdocs.pm/ecto/Ecto.Multi.html

As far as I can tell, the transaction function in the Multi docs is both getting a connection and running a transaction, which is what we are proposing Dream.sql should do by default (right now, it is just getting a connection). Am I missing something about these docs? As far as I can tell, an Ecto.Multi is always meant to be run inside a transaction, at least in basic usage (so the default?).

It's correct that this is a bit more work, but it makes everything explicit. When and when not to use transactions is something developers should think about IMO. Maybe having 2 different functions, one with and one without transaction would be an option.

The developer will, or at least can, still think about it, and can opt out with Dream.sql ~transaction:false ..., and has explicit control readily available.

[tcoopman]

Multi is indeed how you would mostly use transactions in Ecto, so it's meant to be run in a transaction.

As far as I can tell, the transaction function in the Multi docs is both getting a connection and running a transaction

Multi doesn't get a transaction, you have to run a Multi in the Repo.transaction

Repo has a lot of other functions like get, insert, one, update,... that all run outside of a transaction.

This was what I meant with doing it explicitly. You either want to do a single query, insert, update,... and use one of those Repo functions, or you want to run a transaction and use Repo.transaction.

So what I'm wondering is if something similar would be useful for dream. For example Dream.sql_get, Dream.sql_insert, Dream.sql_transaction?