sfackler / rust-postgres

Native PostgreSQL driver for the Rust programming language
Apache License 2.0
3.39k stars 429 forks source link

Fix transction rollback on `Future` drop #1121

Open wquark opened 3 months ago

wquark commented 3 months ago

PR for https://github.com/sfackler/rust-postgres/issues/1120. Also rollsback the savepoint (found while implementing this).

sfackler commented 3 months ago

Looks good to me other than the one nit!

sfackler commented 3 months ago

Looks like it needs a cargo fmt, and this should fix the chrono deprecation warning:

diff --git a/postgres-types/src/chrono_04.rs b/postgres-types/src/chrono_04.rs
index 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())
     }
wquark commented 3 months ago

Ran cargo fmt and fixed warnings. Left the warning fixes in a separate commit since they seemed unrelated.

wquark commented 3 months ago

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.

wquark commented 3 months ago

@sfackler another bump on merging this PR. Any other blockers?