Skip to content

Add savepoint method to Transaction #184

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

Merged
merged 2 commits into from
May 27, 2016

Conversation

nvb
Copy link
Contributor

@nvb nvb commented May 25, 2016

Fixes #179

This change creates a Transaction.savepoint method, which is equivalent
to Transaction.transaction, but takes a custom name for the nested
transaction's savepoint name.

This change creates a `Transaction.savepoint` method, which is equivalent
to `Transaction.transaction`, but takes a custom name for the nested
transaction's savepoint name.
@@ -151,6 +151,7 @@ impl Config {
pub struct Transaction<'conn> {
conn: &'conn Connection,
depth: u32,
savepoint_name: Option<&'conn str>,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd just make this an Option<String> - the runtime cost of the allocation is ~0 compared to talking over the network.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me. How's that?

I'll squash the commits down to one if/when this PR gets the thumbs up by the way.

@sfackler
Copy link
Owner

👍 Thanks!

@sfackler sfackler merged commit 616b725 into sfackler:master May 27, 2016
@nvb nvb deleted the nvanbenschoten/savepoints branch May 27, 2016 04:21
@nvb
Copy link
Contributor Author

nvb commented May 1, 2020

Hi @sfackler! I noticed that this functionality was lost in the newest release (0.17). Was that intentional? E.g. did you decide that this was bloating the API? Or was it just lost in the Tokio rewrite and missed in 0d3e18b? If the latter, do you mind if I send a patch to add it back in?

For context, CockroachDB is still very interested in this functionality for the same reason as we originally discussed in #179. This client-side retry pattern is documented in the CRDB docs and we'd love to be able to get people onto the newest version of this library. This is especially true because we've seen an uptick in people using Rust with CRDB recently (including me on a few side projects 😃) and also an uptick in people interested in Tokio + async/await.

@sfackler
Copy link
Owner

sfackler commented May 1, 2020

Oops - the removal was an oversight during the big rewrite. Happy to take a PR adding it back, or I can put one up at some point alternatively.

@nvb
Copy link
Contributor Author

nvb commented May 1, 2020

Great! I'll try to get a PR out for this today.

nvb added a commit to nvb/rust-postgres that referenced this pull request May 1, 2020
Revives sfackler#184.

The rewrite for async/await and Tokio accidentally lost functionality
that allowed users to assign specific names to savepoints when using
nested transactions. This functionality had originally been added
in sfackler#184 and had been updated in sfackler#374.

This commit revives this functionality using a similar scheme to the
one that existed before. This should allow CockroachDB users to update
to the next patch release of version `0.17`.
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

Successfully merging this pull request may close these issues.

Custom savepoint names for subtransactions
2 participants