-
Notifications
You must be signed in to change notification settings - Fork 249
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
feat: add support for nested transaction rollbacks via savepoints in sql #4405
Conversation
CodSpeed Performance ReportMerging #4405 will improve performances by 5.78%Comparing Summary
Benchmarks breakdown
|
This is my first OSS contribution for a Rust project, so I'm sure I've made some stupid mistakes, but I think it should mostly work :) This change adds a mutable depth counter, that can track how many levels deep a transaction is, and uses savepoints to implement correct rollback behaviour. Previously, once a nested transaction was complete, it would be saved with `COMMIT`, meaning that even if the outer transaction was rolled back, the operations in the inner transaction would persist. With this change, if the outer transaction gets rolled back, then all inner transactions will also be rolled back. Different flavours of SQL servers have different syntax for handling savepoints, so I've had to add new methods to the `Queryable` trait for getting the commit and rollback statements. These are both parameterized by the current depth. I've additionally had to modify the `begin_statement` method to accept a depth parameter, as it will need to conditionally create a savepoint. Signed-off-by: Lucian Buzzo <[email protected]>
Signed-off-by: Lucian Buzzo <[email protected]>
Signed-off-by: Lucian Buzzo <[email protected]>
Signed-off-by: Lucian Buzzo <[email protected]>
This allows an existing transaction ID to be used when starting a new transaction, useful for nested transactions. Signed-off-by: Lucian Buzzo <[email protected]>
5a0f60f
to
4c24703
Compare
@Jolg42 Dumb question: how do we now update prisma/prisma#21678 to use this buildkite release? |
@LucianBuzzo |
Indeed, like Jan said, I can see it popped up as an automated PR here prisma/prisma#21852 |
Same as #4375 but in our repo to trigger Buildkite