- Notifications
You must be signed in to change notification settings - Fork515
Fix transction rollback onFuture drop#1121
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Uh oh!
There was an error while loading.Please reload this page.
Looks good to me other than the one nit! |
Looks like it needs a diff --git a/postgres-types/src/chrono_04.rs b/postgres-types/src/chrono_04.rsindex d599bde0..64166309 100644--- a/postgres-types/src/chrono_04.rs+++ b/postgres-types/src/chrono_04.rs@@ -111,9 +111,10 @@ impl ToSql for DateTime<FixedOffset> { impl<'a> FromSql<'a> for NaiveDate { fn from_sql(_: &Type, raw: &[u8]) -> Result<NaiveDate, Box<dyn Error + Sync + Send>> { let jd = types::date_from_sql(raw)?;+ let jd = Duration::try_days(jd)?; base() .date()- .checked_add_signed(Duration::days(i64::from(jd)))+ .checked_add_signed(jd) .ok_or_else(|| "value too large to decode".into()) } |
- fixes drop of `Future` after synchronous message send but before `Transaction` object creation- follow-on torust-postgres@f6189a9 for `TransactionBuilder` and savepoint `Transaction::transaction` creation
- use non-deprecated `Duration::try_days` method- remove redundant top-level `rand` import
Ran |
Hey@sfackler, would be great to get this pushed forward so we can remove our patched dependencies. Let me know if there are other blockers, thanks. |
@sfackler another bump on merging this PR. Any other blockers? |
-rust-postgres#626 with feature gate additions
PR for#1120. Also rollsback the savepoint (found while implementing this).