From d981262e7e550a14080b668addb14fa23b66964d Mon Sep 17 00:00:00 2001 From: Ryan Leckey Date: Thu, 27 Feb 2020 01:59:37 -0800 Subject: [PATCH] row: RowIndex -> ColumnIndex and de-duplicate logic with macros --- sqlx-core/src/cursor.rs | 5 -- sqlx-core/src/error.rs | 14 ++++- sqlx-core/src/postgres/protocol/data_row.rs | 27 +-------- sqlx-core/src/postgres/row.rs | 48 +++------------- sqlx-core/src/row.rs | 61 ++++++++++++++++++--- 5 files changed, 77 insertions(+), 78 deletions(-) diff --git a/sqlx-core/src/cursor.rs b/sqlx-core/src/cursor.rs index 22f927d1d..1bf880025 100644 --- a/sqlx-core/src/cursor.rs +++ b/sqlx-core/src/cursor.rs @@ -23,9 +23,6 @@ where // `.await`-ing a cursor will return the affected rows from the query Self: Future>, { - // Construct the [Cursor] from a [Pool] - // Meant for internal use only - // TODO: Anyone have any better ideas on how to instantiate cursors generically from a pool? #[doc(hidden)] fn from_pool(pool: &Pool, query: E) -> Self where @@ -37,8 +34,6 @@ where where Self: Sized, DB::Connection: Connect, - // MaybeOwnedConnection<'c, DB::Connection>: - // Connect, C: Into>, E: Execute<'q, DB>; diff --git a/sqlx-core/src/error.rs b/sqlx-core/src/error.rs index 4a1f19394..9269d6a3b 100644 --- a/sqlx-core/src/error.rs +++ b/sqlx-core/src/error.rs @@ -26,9 +26,15 @@ pub enum Error { /// More than one row was returned by a query that expected to return exactly one row. FoundMoreThanOne, - /// Column was not found in Row during [Row::try_get]. + /// Column was not found by name in a Row (during [Row::try_get]). ColumnNotFound(Box), + /// Column index was out of bounds (e.g., asking for column 4 in a 2-column row). + ColumnIndexOutOfBounds { + index: usize, + len: usize, + }, + /// Unexpected or invalid data was encountered. This would indicate that we received /// data that we were not expecting or it was in a format we did not understand. This /// generally means either there is a programming error in a SQLx driver or @@ -89,6 +95,12 @@ impl Display for Error { write!(f, "no column found with the name {:?}", name) } + Error::ColumnIndexOutOfBounds { index, len } => write!( + f, + "column index out of bounds: there are {} columns but the index is {}", + len, index + ), + Error::FoundMoreThanOne => { f.write_str("found more than one row when we expected exactly one") } diff --git a/sqlx-core/src/postgres/protocol/data_row.rs b/sqlx-core/src/postgres/protocol/data_row.rs index a5148ebff..f0bf6c65d 100644 --- a/sqlx-core/src/postgres/protocol/data_row.rs +++ b/sqlx-core/src/postgres/protocol/data_row.rs @@ -27,13 +27,9 @@ impl DataRow { } impl DataRow { - pub(crate) fn read<'a>( - connection: &mut PgConnection, - // buffer: &'a [u8], - // values: &'a mut Vec>>, - ) -> crate::Result { + pub(crate) fn read(connection: &mut PgConnection) -> crate::Result { let buffer = connection.stream.buffer(); - let values = &mut connection.data_row_values_buf; + let values = &mut connection.current_row_values; values.clear(); @@ -64,22 +60,3 @@ impl DataRow { Ok(Self { len }) } } - -#[cfg(test)] -mod tests { - use super::{DataRow, Decode}; - - const DATA_ROW: &[u8] = b"\0\x03\0\0\0\x011\0\0\0\x012\0\0\0\x013"; - - #[test] - fn it_reads_data_row() { - let mut values = Vec::new(); - let m = DataRow::read(DATA_ROW, &mut values).unwrap(); - - assert_eq!(m.len, 3); - - assert_eq!(m.get(DATA_ROW, &values, 0), Some(&b"1"[..])); - assert_eq!(m.get(DATA_ROW, &values, 1), Some(&b"2"[..])); - assert_eq!(m.get(DATA_ROW, &values, 2), Some(&b"3"[..])); - } -} diff --git a/sqlx-core/src/postgres/row.rs b/sqlx-core/src/postgres/row.rs index 58987e88d..a22c8efaa 100644 --- a/sqlx-core/src/postgres/row.rs +++ b/sqlx-core/src/postgres/row.rs @@ -6,7 +6,7 @@ use crate::decode::Decode; use crate::pool::PoolConnection; use crate::postgres::protocol::DataRow; use crate::postgres::{PgConnection, Postgres}; -use crate::row::{Row, RowIndex}; +use crate::row::{ColumnIndex, Row}; use crate::types::Type; pub struct PgRow<'c> { @@ -24,46 +24,16 @@ impl<'c> Row<'c> for PgRow<'c> { fn try_get_raw<'i, I>(&'c self, index: I) -> crate::Result> where - I: RowIndex<'c, Self> + 'i, + I: ColumnIndex<'c, Self> + 'i, { - index.try_get_raw(self) - } -} - -impl<'c> RowIndex<'c, PgRow<'c>> for usize { - fn try_get_raw(self, row: &'c PgRow<'c>) -> crate::Result> { - Ok(row.data.get( - row.connection.stream.buffer(), - &row.connection.data_row_values_buf, - self, + Ok(self.data.get( + self.connection.stream.buffer(), + &self.connection.current_row_values, + index.try_resolve(self)?, )) } } -impl<'c> RowIndex<'c, PgRow<'c>> for &'_ str { - fn try_get_raw(self, row: &'c PgRow<'c>) -> crate::Result> { - let index = row - .columns - .get(self) - .ok_or_else(|| crate::Error::ColumnNotFound((*self).into()))?; - - Ok(row.data.get( - row.connection.stream.buffer(), - &row.connection.data_row_values_buf, - *index, - )) - } -} - -// TODO: impl_from_row_for_row!(PgRow); - -impl crate::query::MapRow for F -where - F: for<'c> FnMut(PgRow<'c>) -> crate::Result, -{ - type Mapped = O; - - fn map_row(&mut self, row: PgRow) -> crate::Result { - (self)(row) - } -} +impl_map_row_for_row!(Postgres, PgRow); +impl_column_index_for_row!(PgRow); +impl_from_row_for_row!(PgRow); diff --git a/sqlx-core/src/row.rs b/sqlx-core/src/row.rs index 780f99b17..fbd9c6717 100644 --- a/sqlx-core/src/row.rs +++ b/sqlx-core/src/row.rs @@ -4,11 +4,11 @@ use crate::database::Database; use crate::decode::Decode; use crate::types::Type; -pub trait RowIndex<'c, R: ?Sized> +pub trait ColumnIndex<'c, R: ?Sized> where R: Row<'c>, { - fn try_get_raw(self, row: &'c R) -> crate::Result>; + fn try_resolve(self, row: &'c R) -> crate::Result; } /// Represents a single row of the result set. @@ -26,7 +26,7 @@ pub trait Row<'c>: Unpin + Send { fn get(&'c self, index: I) -> T where T: Type, - I: RowIndex<'c, Self>, + I: ColumnIndex<'c, Self>, T: Decode, { // todo: use expect with a proper message @@ -36,7 +36,7 @@ pub trait Row<'c>: Unpin + Send { fn try_get(&'c self, index: I) -> crate::Result where T: Type, - I: RowIndex<'c, Self>, + I: ColumnIndex<'c, Self>, T: Decode, { Ok(Decode::decode_nullable(self.try_get_raw(index)?)?) @@ -44,7 +44,7 @@ pub trait Row<'c>: Unpin + Send { fn try_get_raw<'i, I>(&'c self, index: I) -> crate::Result> where - I: RowIndex<'c, Self> + 'i; + I: ColumnIndex<'c, Self> + 'i; } /// A **record** that can be built from a row returned from by the database. @@ -55,12 +55,57 @@ where fn from_row(row: R) -> Self; } +// Macros to help unify the internal implementations as a good chunk is very similar + +#[allow(unused_macros)] +macro_rules! impl_map_row_for_row { + ($DB:ident, $R:ident) => { + impl crate::query::MapRow<$DB> for F + where + F: for<'c> FnMut($R<'c>) -> crate::Result, + { + type Mapped = O; + + fn map_row(&mut self, row: $R) -> crate::Result { + (self)(row) + } + } + }; +} + +#[allow(unused_macros)] +macro_rules! impl_column_index_for_row { + ($R:ident) => { + impl<'c> crate::row::ColumnIndex<'c, $R<'c>> for usize { + fn try_resolve(self, row: &'c $R<'c>) -> crate::Result { + if self >= row.len() { + return Err(crate::Error::ColumnIndexOutOfBounds { + len: row.len(), + index: self, + }); + } + + Ok(self) + } + } + + impl<'c> crate::row::ColumnIndex<'c, $R<'c>> for &'_ str { + fn try_resolve(self, row: &'c $R<'c>) -> crate::Result { + row.columns + .get(self) + .ok_or_else(|| crate::Error::ColumnNotFound((*self).into())) + .map(|&index| index) + } + } + }; +} + #[allow(unused_macros)] macro_rules! impl_from_row_for_row { - ($R:ty) => { - impl crate::row::FromRow<$R> for $R { + ($R:ident) => { + impl<'c> crate::row::FromRow<'c, $R<'c>> for $R<'c> { #[inline] - fn from_row(row: $R) -> Self { + fn from_row(row: $R<'c>) -> Self { row } }