From a165615f531f19e2e0e92ad22bc6c044bc311f39 Mon Sep 17 00:00:00 2001 From: Ryan Leckey Date: Wed, 11 Mar 2020 03:27:00 -0700 Subject: [PATCH] row: Row::get panics, Row::try_get is fallible, Query::map panics, Query::try_map is fallible --- sqlx-core/src/mysql/row.rs | 2 +- sqlx-core/src/postgres/executor.rs | 8 +-- sqlx-core/src/postgres/row.rs | 2 +- sqlx-core/src/query.rs | 92 ++++++++++++++------------ sqlx-core/src/row.rs | 23 +++++-- sqlx-macros/src/query_macros/output.rs | 2 +- sqlx-test/src/lib.rs | 2 +- tests/mysql-raw.rs | 8 +-- tests/postgres-raw.rs | 8 +-- tests/postgres.rs | 20 +++--- 10 files changed, 93 insertions(+), 74 deletions(-) diff --git a/sqlx-core/src/mysql/row.rs b/sqlx-core/src/mysql/row.rs index 7996776f..72338c60 100644 --- a/sqlx-core/src/mysql/row.rs +++ b/sqlx-core/src/mysql/row.rs @@ -38,7 +38,7 @@ impl<'c> Row<'c> for MySqlRow<'c> { self.row.len() } - fn get_raw<'r, I>(&'r self, index: I) -> crate::Result>> + fn try_get_raw<'r, I>(&'r self, index: I) -> crate::Result>> where I: ColumnIndex, { diff --git a/sqlx-core/src/postgres/executor.rs b/sqlx-core/src/postgres/executor.rs index 70d30c99..c2d3fc60 100644 --- a/sqlx-core/src/postgres/executor.rs +++ b/sqlx-core/src/postgres/executor.rs @@ -246,8 +246,8 @@ impl PgConnection { .bind_all(args) .try_map(|row: PgRow| -> crate::Result<(u32, SharedStr)> { Ok(( - row.get::(0)? as u32, - row.get::(1)?.into(), + row.try_get::(0)? as u32, + row.try_get::(1)?.into(), )) }) .fetch(self) @@ -296,8 +296,8 @@ impl PgConnection { crate::query::query(&query) .bind_all(args) .try_map(|row: PgRow| { - let idx = row.get::(0)?; - let non_null = row.get::, _>(1)?; + let idx = row.try_get::(0)?; + let non_null = row.try_get::, _>(1)?; Ok((idx, non_null)) }) diff --git a/sqlx-core/src/postgres/row.rs b/sqlx-core/src/postgres/row.rs index 26ce02fd..37c801ca 100644 --- a/sqlx-core/src/postgres/row.rs +++ b/sqlx-core/src/postgres/row.rs @@ -41,7 +41,7 @@ impl<'c> Row<'c> for PgRow<'c> { self.data.len() } - fn get_raw<'r, I>(&'r self, index: I) -> crate::Result>> + fn try_get_raw<'r, I>(&'r self, index: I) -> crate::Result>> where I: ColumnIndex, { diff --git a/sqlx-core/src/query.rs b/sqlx-core/src/query.rs index 955ace3a..42bc3ad4 100644 --- a/sqlx-core/src/query.rs +++ b/sqlx-core/src/query.rs @@ -30,7 +30,7 @@ where /// /// [Query::bind] is also omitted; stylistically we recommend placing your `.bind()` calls /// before `.try_map()` anyway. -pub struct TryMap<'q, DB, F> +pub struct Map<'q, DB, F> where DB: Database, { @@ -85,31 +85,28 @@ impl<'q, DB> Query<'q, DB> where DB: Database, { - /* - FIXME: how do we implement this without another adapter type (and trait) - probably requires lazy normalization: https://github.com/rust-lang/rust/issues/60471 - pub fn map(self, mapper: F) -> Map<'q, DB, F, A> - where - F: for<'c> FnMut(>::Row) -> T - { - ... - } - */ - /// Map each row in the result to another type. /// /// The returned type has most of the same methods but does not have /// [`.execute()`][Query::execute] or [`.bind()][Query::bind]. /// - /// Stylistically, we recommend placing this call after any [`.bind()`][Query::bind] - /// calls, just before [`.fetch()`][Query::fetch], etc. + /// See also: [query_as][crate::query_as::query_as]. + pub fn map(self, mapper: F) -> Map<'q, DB, impl TryMapRow> + where + O: Unpin, + F: MapRow + { + self.try_map(MapRowAdapter(mapper)) + } + + /// Map each row in the result to another type. /// /// See also: [query_as][crate::query_as::query_as]. - pub fn try_map(self, mapper: F) -> TryMap<'q, DB, F> + pub fn try_map(self, mapper: F) -> Map<'q, DB, F> where - F: MapRow, + F: TryMapRow, { - TryMap { + Map { query: self, mapper, } @@ -136,34 +133,34 @@ where } } -impl<'q, DB, F> TryMap<'q, DB, F> +impl<'q, DB, F> Map<'q, DB, F> where DB: Database, Query<'q, DB>: Execute<'q, DB>, - F: MapRow, + F: TryMapRow, { /// Execute the query and get a [Stream] of the results, returning our mapped type. pub fn fetch<'e: 'q, E>( mut self, executor: E, - ) -> impl Stream> + 'e + ) -> impl Stream> + 'e where 'q: 'e, E: RefExecutor<'e, Database = DB> + 'e, F: 'e, - F::Mapped: 'e, + F::Output: 'e, { try_stream! { let mut cursor = executor.fetch_by_ref(self.query); while let Some(next) = cursor.next().await? { - let mapped = self.mapper.map_row(next)?; + let mapped = self.mapper.try_map_row(next)?; yield mapped; } } } /// Get the first row in the result - pub async fn fetch_optional<'e, E>(self, executor: E) -> crate::Result> + pub async fn fetch_optional<'e, E>(self, executor: E) -> crate::Result> where E: RefExecutor<'e, Database = DB>, 'q: 'e, @@ -172,10 +169,10 @@ where let mut cursor = executor.fetch_by_ref(self.query); let mut mapper = self.mapper; let val = cursor.next().await?; - val.map(|row| mapper.map_row(row)).transpose() + val.map(|row| mapper.try_map_row(row)).transpose() } - pub async fn fetch_one<'e, E>(self, executor: E) -> crate::Result + pub async fn fetch_one<'e, E>(self, executor: E) -> crate::Result where E: RefExecutor<'e, Database = DB>, 'q: 'e, @@ -188,7 +185,7 @@ where .await } - pub async fn fetch_all<'e, E>(mut self, executor: E) -> crate::Result> + pub async fn fetch_all<'e, E>(mut self, executor: E) -> crate::Result> where E: RefExecutor<'e, Database = DB>, 'q: 'e, @@ -197,31 +194,44 @@ where let mut out = vec![]; while let Some(row) = cursor.next().await? { - out.push(self.mapper.map_row(row)?); + out.push(self.mapper.try_map_row(row)?); } Ok(out) } } -/// A (hopefully) temporary workaround for an internal compiler error (ICE) involving higher-ranked -/// trait bounds (HRTBs), associated types and closures. -/// -/// See https://github.com/rust-lang/rust/issues/62529 -pub trait MapRow { - type Mapped: Unpin; +// A (hopefully) temporary workaround for an internal compiler error (ICE) involving higher-ranked +// trait bounds (HRTBs), associated types and closures. +// +// See https://github.com/rust-lang/rust/issues/62529 - fn map_row(&mut self, row: ::Row) -> crate::Result; +pub trait TryMapRow { + type Output: Unpin; + + fn try_map_row(&mut self, row: ::Row) -> crate::Result; } -impl MapRow for for<'c> fn(>::Row) -> crate::Result -where - DB: Database, -{ - type Mapped = O; +pub trait MapRow { + type Output: Unpin; - fn map_row(&mut self, row: ::Row) -> crate::Result { - (self)(row) + fn map_row(&mut self, row: ::Row) -> Self::Output; +} + +// An adapter that implements [MapRow] in terms of [TryMapRow] +// Just ends up Ok wrapping it + +struct MapRowAdapter(F); + +impl TryMapRow for MapRowAdapter + where + O: Unpin, + F: MapRow, +{ + type Output = O; + + fn try_map_row(&mut self, row: ::Row) -> crate::Result { + Ok(self.0.map_row(row)) } } diff --git a/sqlx-core/src/row.rs b/sqlx-core/src/row.rs index 26d1eca5..93625ac3 100644 --- a/sqlx-core/src/row.rs +++ b/sqlx-core/src/row.rs @@ -24,16 +24,25 @@ pub trait Row<'c>: Unpin + Send { /// Returns the number of values in the row. fn len(&self) -> usize; - fn get<'r, T, I>(&'r self, index: I) -> crate::Result + fn get<'r, T, I>(&'r self, index: I) -> T + where + T: Type, + I: ColumnIndex, + T: Decode<'c, Self::Database>, + { + self.try_get::(index).unwrap() + } + + fn try_get<'r, T, I>(&'r self, index: I) -> crate::Result where T: Type, I: ColumnIndex, T: Decode<'c, Self::Database>, { - Ok(Decode::decode(self.get_raw(index)?)?) + Ok(Decode::decode(self.try_get_raw(index)?)?) } - fn get_raw<'r, I>( + fn try_get_raw<'r, I>( &self, index: I, ) -> crate::Result<>::RawValue> @@ -65,7 +74,7 @@ macro_rules! impl_from_row_for_tuple { fn from_row(row: $r<'c>) -> crate::Result { use crate::row::Row; - Ok(($(row.get($idx as usize)?,)+)) + Ok(($(row.try_get($idx as usize)?,)+)) } } }; @@ -151,13 +160,13 @@ macro_rules! impl_from_row_for_tuples { #[allow(unused_macros)] macro_rules! impl_map_row_for_row { ($DB:ident, $R:ident) => { - impl crate::query::MapRow<$DB> for F + impl crate::query::TryMapRow<$DB> for F where F: for<'c> FnMut($R<'c>) -> crate::Result, { - type Mapped = O; + type Output = O; - fn map_row(&mut self, row: $R) -> crate::Result { + fn try_map_row(&mut self, row: $R) -> crate::Result { (self)(row) } } diff --git a/sqlx-macros/src/query_macros/output.rs b/sqlx-macros/src/query_macros/output.rs index 8e6d3323..5a1d17fb 100644 --- a/sqlx-macros/src/query_macros/output.rs +++ b/sqlx-macros/src/query_macros/output.rs @@ -91,7 +91,7 @@ pub fn quote_query_as( ref type_, .. }, - )| { quote!( #ident: row.get::<#type_, _>(#i).try_unwrap_optional()? ) }, + )| { quote!( #ident: row.try_get::<#type_, _>(#i).try_unwrap_optional()? ) }, ); let db_path = DB::db_path(); diff --git a/sqlx-test/src/lib.rs b/sqlx-test/src/lib.rs index e44b83c3..d3bbad69 100644 --- a/sqlx-test/src/lib.rs +++ b/sqlx-test/src/lib.rs @@ -41,7 +41,7 @@ macro_rules! test_unprepared_type { let query = format!("SELECT {} as _1", $text); let mut cursor = conn.fetch(&*query); let row = cursor.next().await?.unwrap(); - let rec = row.get::<$ty, _>("_1")?; + let rec = row.try_get::<$ty, _>("_1")?; assert!($value == rec); )+ diff --git a/tests/mysql-raw.rs b/tests/mysql-raw.rs index 1138714f..c264e810 100644 --- a/tests/mysql-raw.rs +++ b/tests/mysql-raw.rs @@ -12,7 +12,7 @@ async fn test_select_expression() -> anyhow::Result<()> { let mut cursor = conn.fetch("SELECT 5"); let row = cursor.next().await?.unwrap(); - assert!(5i32 == row.get::(0)?); + assert!(5i32 == row.try_get::(0)?); Ok(()) } @@ -42,12 +42,12 @@ SELECT id, text FROM messages; let row = cursor.next().await?.unwrap(); - assert!("Hello World" == row.get::<&str, _>("_1")?); + assert!("Hello World" == row.try_get::<&str, _>("_1")?); let row = cursor.next().await?.unwrap(); - let id: i64 = row.get("id")?; - let text: &str = row.get("text")?; + let id: i64 = row.try_get("id")?; + let text: &str = row.try_get("text")?; assert_eq!(1_i64, id); assert_eq!("this is a test", text); diff --git a/tests/postgres-raw.rs b/tests/postgres-raw.rs index 0492609a..f8b98853 100644 --- a/tests/postgres-raw.rs +++ b/tests/postgres-raw.rs @@ -27,7 +27,7 @@ async fn test_select_expression() -> anyhow::Result<()> { let mut cursor = conn.fetch("SELECT 5"); let row = cursor.next().await?.unwrap(); - assert!(5i32 == row.get::(0)?); + assert!(5i32 == row.try_get::(0)?); Ok(()) } @@ -57,12 +57,12 @@ SELECT id, text FROM _sqlx_test_postgres_5112; let row = cursor.next().await?.unwrap(); - assert!("Hello World" == row.get::<&str, _>("_1")?); + assert!("Hello World" == row.try_get::<&str, _>("_1")?); let row = cursor.next().await?.unwrap(); - let id: i64 = row.get("id")?; - let text: &str = row.get("text")?; + let id: i64 = row.try_get("id")?; + let text: &str = row.try_get("text")?; assert_eq!(1_i64, id); assert_eq!("this is a test", text); diff --git a/tests/postgres.rs b/tests/postgres.rs index f9c1f2c2..f7847aa9 100644 --- a/tests/postgres.rs +++ b/tests/postgres.rs @@ -9,7 +9,7 @@ async fn it_connects() -> anyhow::Result<()> { let mut conn = connect().await?; let value = sqlx::query("select 1 + 1") - .try_map(|row: PgRow| row.get::(0)) + .try_map(|row: PgRow| row.try_get::(0)) .fetch_one(&mut conn) .await?; @@ -41,7 +41,7 @@ CREATE TEMPORARY TABLE users (id INTEGER PRIMARY KEY); } let sum: i32 = sqlx::query("SELECT id FROM users") - .try_map(|row: PgRow| row.get::(0)) + .try_map(|row: PgRow| row.try_get::(0)) .fetch(&mut conn) .try_fold(0_i32, |acc, x| async move { Ok(acc + x) }) .await?; @@ -61,14 +61,14 @@ async fn it_can_return_interleaved_nulls_issue_104() -> anyhow::Result<()> { sqlx::query("SELECT NULL::INT, 10::INT, NULL, 20::INT, NULL, 40::INT, NULL, 80::INT") .try_map(|row: PgRow| { Ok(( - row.get::, _>(0)?, - row.get::, _>(1)?, - row.get::, _>(2)?, - row.get::, _>(3)?, - row.get::, _>(4)?, - row.get::, _>(5)?, - row.get::, _>(6)?, - row.get::, _>(7)?, + row.get::, _>(0), + row.get::, _>(1), + row.get::, _>(2), + row.get::, _>(3), + row.get::, _>(4), + row.get::, _>(5), + row.get::, _>(6), + row.get::, _>(7), )) }) .fetch_one(&mut conn)