Make Rows a fully owned type (sfackler#264)

* Make Rows a fully owned type

This allows Rows to outlive a statement and be sent to 'static threads.

Resolves sfackler#263.

* fixup! Make Rows a fully owned type

* Remove unneeded Debug impl

* Oops, we do actually need this :(

Author: jwilm

postgres/src/lib.rs

@@ -218,6 +218,7 @@ pub enum TlsMode<'a> {
     Require(&'a TlsHandshake),
 }
 
+#[derive(Debug)]
 struct StatementInfo {
     name: String,
     param_types: Vec<Type>,
@@ -1082,7 +1083,7 @@ impl Connection {
     ///     println!("foo: {}", foo);
     /// }
     /// ```
-    pub fn query<'a>(&'a self, query: &str, params: &[&ToSql]) -> Result<Rows<'a>> {
+    pub fn query(&self, query: &str, params: &[&ToSql]) -> Result<Rows<'static>> {
         let (param_types, columns) = self.0.borrow_mut().raw_prepare("", query)?;
         let info = Arc::new(StatementInfo {
                                 name: String::new(),
@@ -1300,7 +1301,7 @@ pub trait GenericConnection {
     fn execute(&self, query: &str, params: &[&ToSql]) -> Result<u64>;
 
     /// Like `Connection::query`.
-    fn query<'a>(&'a self, query: &str, params: &[&ToSql]) -> Result<Rows<'a>>;
+    fn query<'a>(&'a self, query: &str, params: &[&ToSql]) -> Result<Rows<'static>>;
 
     /// Like `Connection::prepare`.
     fn prepare<'a>(&'a self, query: &str) -> Result<Statement<'a>>;
@@ -1323,7 +1324,7 @@ impl GenericConnection for Connection {
         self.execute(query, params)
     }
 
-    fn query<'a>(&'a self, query: &str, params: &[&ToSql]) -> Result<Rows<'a>> {
+    fn query<'a>(&'a self, query: &str, params: &[&ToSql]) -> Result<Rows<'static>> {
         self.query(query, params)
     }
 
@@ -1353,7 +1354,7 @@ impl<'a> GenericConnection for Transaction<'a> {
         self.execute(query, params)
     }
 
-    fn query<'b>(&'b self, query: &str, params: &[&ToSql]) -> Result<Rows<'b>> {
+    fn query<'b>(&'b self, query: &str, params: &[&ToSql]) -> Result<Rows<'static>> {
         self.query(query, params)
     }
 
@@ -1396,9 +1397,8 @@ trait OtherNew {
     fn new(name: String, oid: Oid, kind: Kind, schema: String) -> Other;
 }
 
-trait RowsNew<'a> {
-    fn new(stmt: &'a Statement<'a>, data: Vec<RowData>) -> Rows<'a>;
-    fn new_owned(stmt: Statement<'a>, data: Vec<RowData>) -> Rows<'a>;
+trait RowsNew {
+    fn new(stmt: &Statement, data: Vec<RowData>) -> Rows<'static>;
 }
 
 trait LazyRowsNew<'trans, 'stmt> {
@@ -1421,7 +1421,9 @@ trait StatementInternals<'conn> {
 
     fn conn(&self) -> &'conn Connection;
 
-    fn into_query(self, params: &[&ToSql]) -> Result<Rows<'conn>>;
+    fn info(&self) -> &Arc<StatementInfo>;
+
+    fn into_query(self, params: &[&ToSql]) -> Result<Rows<'static>>;
 }
 
 trait ColumnNew {

postgres/src/rows.rs

@@ -9,8 +9,10 @@ use std::fmt;
 use std::io;
 use std::ops::Deref;
 use std::slice;
+use std::sync::Arc;
+use std::marker::PhantomData;
 
-use {Result, RowsNew, LazyRowsNew, StatementInternals};
+use {Result, RowsNew, LazyRowsNew, StatementInternals, StatementInfo};
 use transaction::Transaction;
 use types::{FromSql, WrongType};
 use stmt::{Statement, Column};
@@ -33,23 +35,18 @@ impl<'a, T> Deref for MaybeOwned<'a, T> {
 }
 
 /// The resulting rows of a query.
-pub struct Rows<'stmt> {
-    stmt: MaybeOwned<'stmt, Statement<'stmt>>,
+pub struct Rows<'compat> {
+    stmt_info: Arc<StatementInfo>,
     data: Vec<RowData>,
+    _marker: PhantomData<&'compat u8>
 }
 
-impl<'a> RowsNew<'a> for Rows<'a> {
-    fn new(stmt: &'a Statement<'a>, data: Vec<RowData>) -> Rows<'a> {
+impl RowsNew for Rows<'static> {
+    fn new(stmt: &Statement, data: Vec<RowData>) -> Rows<'static> {
         Rows {
-            stmt: MaybeOwned::Borrowed(stmt),
-            data: data,
-        }
-    }
-
-    fn new_owned(stmt: Statement<'a>, data: Vec<RowData>) -> Rows<'a> {
-        Rows {
-            stmt: MaybeOwned::Owned(stmt),
+            stmt_info: stmt.info().clone(),
             data: data,
+            _marker: PhantomData,
         }
     }
 }
@@ -63,10 +60,10 @@ impl<'a> fmt::Debug for Rows<'a> {
     }
 }
 
-impl<'stmt> Rows<'stmt> {
+impl<'rows> Rows<'rows> {
     /// Returns a slice describing the columns of the `Rows`.
     pub fn columns(&self) -> &[Column] {
-        self.stmt.columns()
+        &self.stmt_info.columns[..]
     }
 
     /// Returns the number of rows present.
@@ -86,15 +83,15 @@ impl<'stmt> Rows<'stmt> {
     /// Panics if `idx` is out of bounds.
     pub fn get<'a>(&'a self, idx: usize) -> Row<'a> {
         Row {
-            stmt: &*self.stmt,
+            stmt_info: &self.stmt_info,
             data: MaybeOwned::Borrowed(&self.data[idx]),
         }
     }
 
     /// Returns an iterator over the `Row`s.
     pub fn iter<'a>(&'a self) -> Iter<'a> {
         Iter {
-            stmt: &*self.stmt,
+            stmt_info: &self.stmt_info,
             iter: self.data.iter(),
         }
     }
@@ -111,7 +108,7 @@ impl<'a> IntoIterator for &'a Rows<'a> {
 
 /// An iterator over `Row`s.
 pub struct Iter<'a> {
-    stmt: &'a Statement<'a>,
+    stmt_info: &'a StatementInfo,
     iter: slice::Iter<'a, RowData>,
 }
 
@@ -121,7 +118,7 @@ impl<'a> Iterator for Iter<'a> {
     fn next(&mut self) -> Option<Row<'a>> {
         self.iter.next().map(|row| {
             Row {
-                stmt: &*self.stmt,
+                stmt_info: self.stmt_info,
                 data: MaybeOwned::Borrowed(row),
             }
         })
@@ -136,7 +133,7 @@ impl<'a> DoubleEndedIterator for Iter<'a> {
     fn next_back(&mut self) -> Option<Row<'a>> {
         self.iter.next_back().map(|row| {
             Row {
-                stmt: &*self.stmt,
+                stmt_info: self.stmt_info,
                 data: MaybeOwned::Borrowed(row),
             }
         })
@@ -147,14 +144,14 @@ impl<'a> ExactSizeIterator for Iter<'a> {}
 
 /// A single result row of a query.
 pub struct Row<'a> {
-    stmt: &'a Statement<'a>,
+    stmt_info: &'a StatementInfo,
     data: MaybeOwned<'a, RowData>,
 }
 
 impl<'a> fmt::Debug for Row<'a> {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
         fmt.debug_struct("Row")
-            .field("statement", self.stmt)
+            .field("statement", self.stmt_info)
             .finish()
     }
 }
@@ -172,7 +169,7 @@ impl<'a> Row<'a> {
 
     /// Returns a slice describing the columns of the `Row`.
     pub fn columns(&self) -> &[Column] {
-        self.stmt.columns()
+        &self.stmt_info.columns[..]
     }
 
     /// Retrieves the contents of a field of the row.
@@ -227,12 +224,12 @@ impl<'a> Row<'a> {
         where I: RowIndex,
               T: FromSql
     {
-        let idx = match idx.idx(self.stmt) {
+        let idx = match idx.idx(&self.stmt_info.columns) {
             Some(idx) => idx,
             None => return None,
         };
 
-        let ty = self.stmt.columns()[idx].type_();
+        let ty = self.stmt_info.columns[idx].type_();
         if !<T as FromSql>::accepts(ty) {
             return Some(Err(Error::Conversion(Box::new(WrongType::new(ty.clone())))));
         }
@@ -248,7 +245,7 @@ impl<'a> Row<'a> {
     pub fn get_bytes<I>(&self, idx: I) -> Option<&[u8]>
         where I: RowIndex + fmt::Debug
     {
-        match idx.idx(self.stmt) {
+        match idx.idx(&self.stmt_info.columns) {
             Some(idx) => self.data.get(idx),
             None => panic!("invalid index {:?}", idx),
         }
@@ -259,13 +256,13 @@ impl<'a> Row<'a> {
 pub trait RowIndex {
     /// Returns the index of the appropriate column, or `None` if no such
     /// column exists.
-    fn idx(&self, stmt: &Statement) -> Option<usize>;
+    fn idx(&self, _: &[Column]) -> Option<usize>;
 }
 
 impl RowIndex for usize {
     #[inline]
-    fn idx(&self, stmt: &Statement) -> Option<usize> {
-        if *self >= stmt.columns().len() {
+    fn idx(&self, columns: &[Column]) -> Option<usize> {
+        if *self >= columns.len() {
             None
         } else {
             Some(*self)
@@ -275,15 +272,15 @@ impl RowIndex for usize {
 
 impl<'a> RowIndex for &'a str {
     #[inline]
-    fn idx(&self, stmt: &Statement) -> Option<usize> {
-        if let Some(idx) = stmt.columns().iter().position(|d| d.name() == *self) {
+    fn idx(&self, columns: &[Column]) -> Option<usize> {
+        if let Some(idx) = columns.iter().position(|d| d.name() == *self) {
             return Some(idx);
         };
 
         // FIXME ASCII-only case insensitivity isn't really the right thing to
         // do. Postgres itself uses a dubious wrapper around tolower and JDBC
         // uses the US locale.
-        stmt.columns().iter().position(|d| d.name().eq_ignore_ascii_case(*self))
+        columns.iter().position(|d| d.name().eq_ignore_ascii_case(*self))
     }
 }
 
@@ -381,7 +378,7 @@ impl<'trans, 'stmt> FallibleIterator for LazyRows<'trans, 'stmt> {
             .pop_front()
             .map(|r| {
                 Row {
-                    stmt: self.stmt,
+                    stmt_info: &**self.stmt.info(),
                     data: MaybeOwned::Owned(r),
                 }
             });

postgres/src/stmt.rs

@@ -26,11 +26,7 @@ pub struct Statement<'conn> {
 
 impl<'a> fmt::Debug for Statement<'a> {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        fmt.debug_struct("Statement")
-            .field("name", &self.info.name)
-            .field("parameter_types", &self.info.param_types)
-            .field("columns", &self.info.columns)
-            .finish()
+        fmt::Debug::fmt(&*self.info, fmt)
     }
 }
 
@@ -54,15 +50,19 @@ impl<'conn> StatementInternals<'conn> for Statement<'conn> {
         }
     }
 
+    fn info(&self) -> &Arc<StatementInfo> {
+        &self.info
+    }
+
     fn conn(&self) -> &'conn Connection {
         self.conn
     }
 
-    fn into_query(self, params: &[&ToSql]) -> Result<Rows<'conn>> {
+    fn into_query(self, params: &[&ToSql]) -> Result<Rows<'static>> {
         check_desync!(self.conn);
         let mut rows = vec![];
         self.inner_query("", 0, params, |row| rows.push(row))?;
-        Ok(Rows::new_owned(self, rows))
+        Ok(Rows::new(&self, rows))
     }
 }
 
@@ -201,7 +201,7 @@ impl<'conn> Statement<'conn> {
     ///     println!("foo: {}", foo);
     /// }
     /// ```
-    pub fn query<'a>(&'a self, params: &[&ToSql]) -> Result<Rows<'a>> {
+    pub fn query(&self, params: &[&ToSql]) -> Result<Rows<'static>> {
         check_desync!(self.conn);
         let mut rows = vec![];
         self.inner_query("", 0, params, |row| rows.push(row))?;

postgres/src/transaction.rs

@@ -228,7 +228,7 @@ impl<'conn> Transaction<'conn> {
     }
 
     /// Like `Connection::query`.
-    pub fn query<'a>(&'a self, query: &str, params: &[&ToSql]) -> Result<Rows<'a>> {
+    pub fn query<'a>(&'a self, query: &str, params: &[&ToSql]) -> Result<Rows<'static>> {
         self.conn.query(query, params)
     }
 

postgres/tests/test.rs

@@ -1088,11 +1088,11 @@ fn test_row_case_insensitive() {
     conn.batch_execute("CREATE TEMPORARY TABLE foo (foo INT, \"bAr\" INT, \"Bar\" INT);")
         .unwrap();
     let stmt = conn.prepare("SELECT * FROM foo").unwrap();
-    assert_eq!(Some(0), "foo".idx(&stmt));
-    assert_eq!(Some(0), "FOO".idx(&stmt));
-    assert_eq!(Some(1), "bar".idx(&stmt));
-    assert_eq!(Some(1), "bAr".idx(&stmt));
-    assert_eq!(Some(2), "Bar".idx(&stmt));
+    assert_eq!(Some(0), "foo".idx(&stmt.columns()));
+    assert_eq!(Some(0), "FOO".idx(&stmt.columns()));
+    assert_eq!(Some(1), "bar".idx(&stmt.columns()));
+    assert_eq!(Some(1), "bAr".idx(&stmt.columns()));
+    assert_eq!(Some(2), "Bar".idx(&stmt.columns()));
 }
 
 #[test]