Fix issue #1209

pgrx-examples/spi/src/lib.rs

@@ -54,7 +54,10 @@ fn spi_return_query() -> Result<
     let query = "SELECT oid, relname::text || '-pg16' FROM pg_class";
 
     Spi::connect(|client| {
-        Ok(client.select(query, None, None)?.map(|row| (row["oid"].value(), row[2].value())))
+        Ok(client
+            .select(query, None, None)?
+            .map(|row| (row["oid"].value(), row[2].value()))
+            .collect::<Vec<_>>())
     })
     .map(|results| TableIterator::new(results))
 }
@@ -111,6 +114,17 @@ fn spi_insert_title2(
     TableIterator::once(tuple)
 }
 
+#[pg_extern]
+fn issue1209_fixed() -> Result<Option<String>, Box<dyn std::error::Error>> {
+    let res = Spi::connect(|c| {
+        let mut cursor = c.open_cursor("SELECT 'hello' FROM generate_series(1, 10000)", None);
+        let table = cursor.fetch(10000)?;
+        table.into_iter().map(|row| row.get::<&str>(1)).collect::<Result<Vec<_>, _>>()
+    })?;
+
+    Ok(res.first().cloned().flatten().map(|s| s.to_string()))
+}
+
 extension_sql!(
     r#"
 

pgrx-tests/src/tests/bgworker_tests.rs

@@ -27,11 +27,13 @@ pub extern "C" fn bgworker(arg: pg_sys::Datum) {
         BackgroundWorker::transaction(|| {
             Spi::run("CREATE TABLE tests.bgworker_test (v INTEGER);")?;
             Spi::connect(|mut client| {
-                client.update(
-                    "INSERT INTO tests.bgworker_test VALUES ($1);",
-                    None,
-                    Some(vec![(PgOid::BuiltIn(PgBuiltInOids::INT4OID), arg.into_datum())]),
-                )
+                client
+                    .update(
+                        "INSERT INTO tests.bgworker_test VALUES ($1);",
+                        None,
+                        Some(vec![(PgOid::BuiltIn(PgBuiltInOids::INT4OID), arg.into_datum())]),
+                    )
+                    .map(|_| ())
             })
         })
         .expect("bgworker transaction failed");
@@ -78,6 +80,7 @@ pub extern "C" fn bgworker_return_value(arg: pg_sys::Datum) {
                 None,
                 Some(vec![(PgOid::BuiltIn(PgBuiltInOids::INT4OID), val.into_datum())]),
             )
+            .map(|_| ())
         })
     })
     .expect("bgworker transaction failed");

pgrx-tests/src/tests/heap_tuple.rs

@@ -927,12 +927,12 @@ mod tests {
     }
 
     #[pg_test]
-    fn test_tuple_desc_clone() {
+    fn test_tuple_desc_clone() -> Result<(), spi::Error> {
         let result = Spi::connect(|client| {
             let query = "select * from generate_lots_of_dogs()";
-            client.select(query, None, None)
-        });
-        let table = result.expect("unable to select table result");
-        assert_eq!(table.len(), 10_000);
+            client.select(query, None, None).map(|table| table.len())
+        })?;
+        assert_eq!(result, 10_000);
+        Ok(())
     }
 }

pgrx-tests/src/tests/spi_tests.rs

@@ -13,13 +13,14 @@ mod tests {
     #[allow(unused_imports)]
     use crate as pgrx_tests;
     use pgrx::IntoDatum;
+    use std::error::Error;
 
     use pgrx::prelude::*;
     use pgrx::spi;
 
     #[pg_test(error = "syntax error at or near \"THIS\"")]
     fn test_spi_failure() -> Result<(), spi::Error> {
-        Spi::connect(|client| client.select("THIS IS NOT A VALID QUERY", None, None)).map(|_| ())
+        Spi::connect(|client| client.select("THIS IS NOT A VALID QUERY", None, None).map(|_| ()))
     }
 
     #[pg_test]
@@ -180,7 +181,7 @@ mod tests {
     #[pg_test]
     fn test_inserting_null() -> Result<(), pgrx::spi::Error> {
         Spi::connect(|mut client| {
-            client.update("CREATE TABLE tests.null_test (id uuid)", None, None)
+            client.update("CREATE TABLE tests.null_test (id uuid)", None, None).map(|_| ())
         })?;
         assert_eq!(
             Spi::get_one_with_args::<i32>(
@@ -369,7 +370,7 @@ mod tests {
         let err = Spi::connect(|client| {
             let prepared =
                 client.prepare("SELECT $1", Some(vec![PgOid::BuiltIn(PgBuiltInOids::INT4OID)]))?;
-            client.select(&prepared, None, None)
+            client.select(&prepared, None, None).map(|_| ())
         })
         .unwrap_err();
 
@@ -404,7 +405,7 @@ mod tests {
     #[pg_test(error = "CREATE TABLE is not allowed in a non-volatile function")]
     fn test_readwrite_in_readonly() -> Result<(), spi::Error> {
         // This is supposed to run in read-only
-        Spi::connect(|client| client.select("CREATE TABLE a ()", None, None)).map(|_| ())
+        Spi::connect(|client| client.select("CREATE TABLE a ()", None, None).map(|_| ()))
     }
 
     #[pg_test]
@@ -508,4 +509,50 @@ mod tests {
         assert_eq!("'quoted-with-''quotes'''", spi::quote_literal("quoted-with-'quotes'"));
         assert_eq!("'quoted-string'", spi::quote_literal(String::from("quoted-string")));
     }
+
+    #[pg_test]
+    fn can_return_borrowed_str() -> Result<(), Box<dyn Error>> {
+        let res = Spi::connect(|c| {
+            let mut cursor = c.open_cursor("SELECT 'hello' FROM generate_series(1, 10000)", None);
+            let table = cursor.fetch(10000)?;
+            table.into_iter().map(|row| row.get::<&str>(1)).collect::<Result<Vec<_>, _>>()
+        })?;
+
+        let value = res.first().cloned().flatten().map(|s| s.to_string());
+        assert_eq!(Some("hello".to_string()), value);
+        Ok(())
+    }
+
+    // TODO:  The point of this test is to **not** compile.  How to write a test for that?
+    // #[pg_test]
+    // fn issue1209() -> Result<Option<String>, Box<dyn Error>> {
+    //     // create the cursor we actually care about
+    //     let mut res = Spi::connect(|c| {
+    //         c.open_cursor("select 'hello' from generate_series(1, 10000)", None)
+    //             .fetch(10000)
+    //             .unwrap()
+    //     });
+    //
+    //     // here we just perform some allocations to make sure that the previous cursor gets invalidated
+    //     for _ in 0..1000 {
+    //         Spi::connect(|c| c.open_cursor("select 1", None).fetch(1).unwrap());
+    //     }
+    //
+    //     // later elements are probably more likely to point to deallocated memory
+    //     for _ in 0..1000 {
+    //         res.next();
+    //     }
+    //
+    //     // segfault
+    //     Ok(res.next().unwrap().get::<String>(1)?)
+    // }
+    //
+    // #[pg_extern]
+    // fn issue1209_prepared_stmt(q: &str) -> Result<Option<String>, Box<dyn std::error::Error>> {
+    //     use pgrx::spi::Query;
+    //
+    //     let prepared = { Spi::connect(|c| c.prepare(q, None))? };
+    //
+    //     Ok(Spi::connect(|c| prepared.execute(&c, Some(1), None)?.first().get(1))?)
+    // }
 }