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

Feature Request: Savepoints #20211

Closed
Lobosque opened this issue Mar 7, 2020 · 9 comments
Closed

Feature Request: Savepoints #20211

Lobosque opened this issue Mar 7, 2020 · 9 comments

Comments

@Lobosque
Copy link

Lobosque commented Mar 7, 2020

Rollback a transaction up to a specific savepoint without ExecuteSqlRaw and without mannually re-arranging the ChangeTracker State

transaction.Savepoint("step2") // --> SAVEPOINT step2
//....
transaction.Rollback("step2") // --> ROLLBACK TO SAVEPOINT step2
// ...
transaction.Rollback() // --> ROLLBACK
@roji
Copy link
Member

roji commented Mar 7, 2020

@Lobosque savepoints are a PostgreSQL-specific concept, or at least not a universal thing supported across database. For this reason, it isn't present in the ADO.NET API, and so we can't really add anything to EF Core generically. This would also be quite an effort, as the EF Core infrastructure currently knows about transactions succeeding or failing, and does not have a concept of partially rolling back a transaction (which is what savepoints allow).

Note that you can extract the ADO DbTransaction from the RelationalTransaction instance, down-cast that to NpgsqlTransaction, and call those methods yourself - so you don't need to do ExecuteSqlRaw.

@roji
Copy link
Member

roji commented Mar 7, 2020

I took a quick look and apparently most DBs do support some form of savepoints (PostgreSQL, SQL Server, Sqlite, MySQL). So we could expose something in ADO.NET for managing them (and after that possibly do something with that in EF), though we'd need to first investigate that semantics are consistent.

@Lobosque
Copy link
Author

Lobosque commented Mar 7, 2020

@roji it is implemented accross all relevant databases and it looks like it is in the SQL standard.
Without a deep analysis, semantics seems to be pretty consistent accross implementations. (If you look too deep, then what is?).

Shouldn't the issue be reopened so others could also show their interest and discuss the feature?

@roji
Copy link
Member

roji commented Mar 7, 2020

I do agree there is potential here for a cross-database transaction savepoint API - but we'd add it to ADO.NET first in any case (so https://github.com/dotnet/runtime). We haven't seen any request for it up to now, but that doesn't mean we shouldn't have an issue for it.

At the EF level, assuming support is added in ADO.NET we'd have to understand what it means to have savepoint support exactly. I agree it may make sense to use a savepoint when SaveUpdates is called and a transaction is already open (so that the entire transaction doesn't have to be rolled back in case of an error), but that's something that can only come significantly later.

Reopening to discuss in triage.

@roji roji reopened this Mar 7, 2020
@roji
Copy link
Member

roji commented Mar 9, 2020

Opened dotnet/runtime#33397 to track adding savepoint APIs to System.Data.Common.

Using savepoints as a fix for #20176 (rolling back on optimistic concurrency exceptions) can be tracked in that issue, so I don't think this issue can track anything else (except maybe adding sugar on RelationalTransaction to call the new savepoint APIs on DbTransaction, which seems pretty low value).

@roji
Copy link
Member

roji commented Apr 27, 2020

Note that the proposal for System.Data support (dotnet/runtime#33397) is waiting to be reviewed and will probably make it into .NET 5.0. At the same time, full support for savepoints has already been merged for EF Core as well in #20176.

Thanks for raising this issue!

@Lobosque
Copy link
Author

@roji thank you and the team for making it happen!

@roji
Copy link
Member

roji commented Apr 30, 2020

Thank you for raising it and making the original suggestion! This probably wouldn't have happened otherwise...!

@roji
Copy link
Member

roji commented Jun 4, 2020

Duplicate of #20176

@roji roji marked this as a duplicate of #20176 Jun 4, 2020
@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants