Changed args of prepare methods to be slices (#1933)

Since there's an opportunity to break more things for `0.13` I decided to improve the `prepare` and `prepare_mut` methods as well. The motivation for that is the same as for #1858, `&[]` requires no heap allocations on the heap if there's a fixed set of arguments.
pgcentralfoundation · Nov 20, 2024 · c1e5dd9 · c1e5dd9
1 parent 7ca80af
commit c1e5dd9
Show file tree

Hide file tree

Showing 5 changed files with 18 additions and 22 deletions.
diff --git a/pgrx-tests/src/tests/spi_tests.rs b/pgrx-tests/src/tests/spi_tests.rs
@@ -216,7 +216,7 @@ mod tests {
                 None,
                 &[],
             )?;
-            let prepared = client.prepare("SELECT * FROM tests.cursor_table", None)?;
+            let prepared = client.prepare("SELECT * FROM tests.cursor_table", &[])?;
             let mut portal = client.open_cursor(&prepared, &[]);
 
             assert_eq!(sum_all(portal.fetch(3)?), 1 + 2 + 3);
@@ -255,7 +255,7 @@ mod tests {
             )?;
             let prepared = client.prepare(
                 "SELECT * FROM tests.cursor_table WHERE id = $1",
-                Some([PgBuiltInOids::INT4OID.oid()].to_vec()),
+                &[PgBuiltInOids::INT4OID.oid()],
             )?;
             client.open_cursor(&prepared, args);
             unreachable!();
@@ -377,7 +377,7 @@ mod tests {
     fn test_prepared_statement() -> Result<(), spi::Error> {
         let rc = Spi::connect(|client| {
             let prepared =
-                client.prepare("SELECT $1", Some(vec![PgOid::BuiltIn(PgBuiltInOids::INT4OID)]))?;
+                client.prepare("SELECT $1", &[PgOid::BuiltIn(PgBuiltInOids::INT4OID)])?;
             client.select(&prepared, None, &[42.into()])?.first().get::<i32>(1)
         })?;
 
@@ -389,7 +389,7 @@ mod tests {
     fn test_prepared_statement_argument_mismatch() {
         let err = Spi::connect(|client| {
             let prepared =
-                client.prepare("SELECT $1", Some(vec![PgOid::BuiltIn(PgBuiltInOids::INT4OID)]))?;
+                client.prepare("SELECT $1", &[PgOid::BuiltIn(PgBuiltInOids::INT4OID)])?;
             client.select(&prepared, None, &[]).map(|_| ())
         })
         .unwrap_err();
@@ -404,9 +404,7 @@ mod tests {
     fn test_owned_prepared_statement() -> Result<(), spi::Error> {
         let prepared = Spi::connect(|client| {
             Ok::<_, spi::Error>(
-                client
-                    .prepare("SELECT $1", Some(vec![PgOid::BuiltIn(PgBuiltInOids::INT4OID)]))?
-                    .keep(),
+                client.prepare("SELECT $1", &[PgOid::BuiltIn(PgBuiltInOids::INT4OID)])?.keep(),
             )
         })?;
         let rc = Spi::connect(|client| {
@@ -442,7 +440,7 @@ mod tests {
     #[pg_test(error = "CREATE TABLE is not allowed in a non-volatile function")]
     fn test_execute_prepared_statement_in_readonly() -> Result<(), spi::Error> {
         Spi::connect(|client| {
-            let stmt = client.prepare("CREATE TABLE a ()", None)?;
+            let stmt = client.prepare("CREATE TABLE a ()", &[])?;
             // This is supposed to run in read-only
             stmt.execute(&client, Some(1), &[])?;
             Ok(())
@@ -452,7 +450,7 @@ mod tests {
     #[pg_test]
     fn test_execute_prepared_statement_in_readwrite() -> Result<(), spi::Error> {
         Spi::connect(|client| {
-            let stmt = client.prepare_mut("CREATE TABLE a ()", None)?;
+            let stmt = client.prepare_mut("CREATE TABLE a ()", &[])?;
             // This is supposed to run in read-write
             stmt.execute(&client, Some(1), &[])?;
             Ok(())

diff --git a/pgrx-tests/tests/compile-fail/escaping-spiclient-1209-prep-stmt.rs b/pgrx-tests/tests/compile-fail/escaping-spiclient-1209-prep-stmt.rs
@@ -5,7 +5,7 @@ use std::error::Error;
 fn issue1209_prepared_stmt(q: &str) -> Result<Option<String>, Box<dyn Error>> {
     use pgrx::spi::Query;
 
-    let prepared = { Spi::connect(|c| c.prepare(q, None))? };
+    let prepared = { Spi::connect(|c| c.prepare(q, &[]))? };
 
     Ok(Spi::connect(|c| prepared.execute(&c, Some(1), &[])?.first().get(1))?)
 }

diff --git a/pgrx-tests/tests/compile-fail/escaping-spiclient-1209-prep-stmt.stderr b/pgrx-tests/tests/compile-fail/escaping-spiclient-1209-prep-stmt.stderr
@@ -1,8 +1,8 @@
 error: lifetime may not live long enough
  --> tests/compile-fail/escaping-spiclient-1209-prep-stmt.rs:8:39
   |
-8 |     let prepared = { Spi::connect(|c| c.prepare(q, None))? };
-  |                                    -- ^^^^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'2`
+8 |     let prepared = { Spi::connect(|c| c.prepare(q, &[]))? };
+  |                                    -- ^^^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'2`
   |                                    ||
   |                                    |return type of closure is std::result::Result<pgrx::spi::PreparedStatement<'2>, pgrx::spi::SpiError>
   |                                    has type `SpiClient<'1>`
diff --git a/pgrx/src/spi/client.rs b/pgrx/src/spi/client.rs
@@ -17,7 +17,7 @@ impl<'conn> SpiClient<'conn> {
     pub fn prepare<Q: PreparableQuery<'conn>>(
         &self,
         query: Q,
-        args: Option<Vec<PgOid>>,
+        args: &[PgOid],
     ) -> SpiResult<PreparedStatement<'conn>> {
         query.prepare(self, args)
     }
@@ -26,7 +26,7 @@ impl<'conn> SpiClient<'conn> {
     pub fn prepare_mut<Q: PreparableQuery<'conn>>(
         &self,
         query: Q,
-        args: Option<Vec<PgOid>>,
+        args: &[PgOid],
     ) -> SpiResult<PreparedStatement<'conn>> {
         query.prepare_mut(self, args)
     }

diff --git a/pgrx/src/spi/query.rs b/pgrx/src/spi/query.rs
@@ -53,14 +53,14 @@ pub trait PreparableQuery<'conn>: Query<'conn> {
     fn prepare(
         self,
         client: &SpiClient<'conn>,
-        args: Option<Vec<PgOid>>,
+        args: &[PgOid],
     ) -> SpiResult<PreparedStatement<'conn>>;
 
     /// Prepares a query allowed to change data
     fn prepare_mut(
         self,
         client: &SpiClient<'conn>,
-        args: Option<Vec<PgOid>>,
+        args: &[PgOid],
     ) -> SpiResult<PreparedStatement<'conn>>;
 }
 
@@ -152,17 +152,15 @@ fn prepare_datum<'mcx>(datum: &DatumWithOid<'mcx>) -> (pg_sys::Datum, std::os::r
 
 fn prepare<'conn>(
     cmd: &CStr,
-    args: Option<Vec<PgOid>>,
+    args: &[PgOid],
     mutating: bool,
 ) -> SpiResult<PreparedStatement<'conn>> {
-    let args = args.unwrap_or_default();
-
     // SAFETY: all arguments are prepared above
     let plan = unsafe {
         pg_sys::SPI_prepare(
             cmd.as_ptr(),
             args.len() as i32,
-            args.into_iter().map(PgOid::value).collect::<Vec<_>>().as_mut_ptr(),
+            args.into_iter().map(|arg| arg.value()).collect::<Vec<_>>().as_mut_ptr(),
         )
     };
     Ok(PreparedStatement {
@@ -206,15 +204,15 @@ macro_rules! impl_prepared_query {
             fn prepare(
                 self,
                 _client: &SpiClient<'conn>,
-                args: Option<Vec<PgOid>>,
+                args: &[PgOid],
             ) -> SpiResult<PreparedStatement<'conn>> {
                 prepare($s(self).as_ref(), args, false)
             }
 
             fn prepare_mut(
                 self,
                 _client: &SpiClient<'conn>,
-                args: Option<Vec<PgOid>>,
+                args: &[PgOid],
             ) -> SpiResult<PreparedStatement<'conn>> {
                 prepare($s(self).as_ref(), args, true)
             }