Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable nested transactions and rollbacks? #56

Open
kindaro opened this issue Mar 7, 2021 · 4 comments
Open

Enable nested transactions and rollbacks? #56

kindaro opened this issue Mar 7, 2021 · 4 comments

Comments

@kindaro
Copy link
Contributor

kindaro commented Mar 7, 2021

So, SQL has two features that essentially do the same thing. First you have to use begin … rollback / commit, and then inside it you can use savepoint … rollback to savepoint / release savepoint, and you can do that any number of times, effectively creating arbitrary nesting of transactions.

This means that in principle transactions compose. But at present, if I wrap a snaplet that uses a transaction into another snaplet that also uses a transaction, it will be a mess: the transaction will start at the first begin, the inner begin will be ignored, then the inner rollback / commit will affect everything back to the first begin and the final rollback / commit will be ignored. Good for me that PostgreSQL emits a warning in such cases!

This is not in principle a problem — we only need to track in the HasPostgres monad what the current nesting level is. Then we can intelligently choose whether to use begin or savepoint. For bonus points, we may also carry around a source of fresh names for save points — this would be required for vanilla SQL, but PostgreSQL allows using the same name for nested save points, so it is not necessary.

Can we have this feature? I am considering writing it for a private project and if it goes well I may be able to contribute a patch.

@kindaro
Copy link
Contributor Author

kindaro commented Mar 7, 2021

While at it, maybe we can also add some locking with an MVar, so that another thread that happens to have the same connexion does not interfere with a transaction… There is some conversation about it up the stream: lpsmith/postgresql-simple#202 — but it is inconclusive and we are in a better position to do this since we are high level and already carry around an environment.

@mightybyte
Copy link
Owner

My rough idea for this library was that it would live at a lower level of abstraction than what you're talking about. Now it is true that this package has a few withTransaction... functions dealing with transactions, but it also has the primitive begin, rollback, and commit functions. My initial thought is that if the included withTransaction... functions don't do what you want out of the box, then you should write your own transaction handling infrastructure outside of this package using the lower level primitives. MVars and separate threads definitely seem out of this package's scope. Are there any obstacles to you implementing this functionality in your app as opposed to in this package?

@kindaro
Copy link
Contributor Author

kindaro commented Mar 7, 2021

Sure, if this does not align with your vision, I can find a way to handle it independently.

@kindaro kindaro closed this as completed Mar 7, 2021
@kindaro
Copy link
Contributor Author

kindaro commented Mar 8, 2021

So, I drafted a proof of concept and it is actually very simple.

There are no MVar s and no threads — this was a red herring. We simply add a Bool field to PostgresConn and use it to track whether we are inside a transaction, in which case we use save points instead. It is strictly more powerful:

  • It should work exactly the same in any code that does not use nested transactions. I have a fairly big application built on top of snaplet-postgresql-simple and it compiles and passes end to end tests.
  • It also allows one to use nested transactions fearlessly. I added some nested transactions and looked at SQL logs — on the examples I have, rollbacks roll back exactly one layer, as expected.

I think it would be problematic to implement this on top of snaplet-postgresql-simple.

  • We need to track one more bit of immutable state and it would require a whole new monadic layer.
  • snaplet-postgresql-simple does not re-export from postgresql-simple the primitives for working with save points.
  • Using these primitives as provided by postgresql-simple requires a Connection, which means that we will have to deconstruct the Postgres abstraction.

That is to say, the abstraction of snaplet-postgresql-simple does not permit the feature of nested transactions to be independently added on top.

This is why I would like you to take a look at this patch.

I completely understand if you do not want to add this feature. But it is small, mostly backwards compatible (only internal interface is changed) and does not add any more imports or dependencies. And most importantly, it removes a foot gun and makes more things possible.

@kindaro kindaro reopened this Mar 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants