impl IntoDatum for Result<T: IntoDatum, E: Display> (pgcentralfoundation#972)

This pulls out some commits from pgcentralfoundation#969 into its own PR to add support for `#[pg_extern]`-style functions returning a `Result` of some kind.

We don't care about the type of the Error, so long as it implements `Display`.  If the returned value is the Err variant, the `IntoDatum` impl for `Result<T, E>` will raise a Postgres ERROR using `ereport!(ERROR, PgSqlErrorCode::ERRCODE_DATA_EXCEPTION, &format!("{}", e))`.

It's tested to also work with the [`eyre::Result`](https://docs.rs/eyre/latest/eyre/type.Result.html) type along with returning `Result<Option<T>, E>`.

The case of returning a `Result<(), E>` uncovered some issues with how pgx converts `()` into a Datum, so those have been fixed along the way.

It also allows `#[pg_test]` functions to return Results which makes writing unit tests a little nicer.

Author: eeeebbbbrrrr

pgx-sql-entity-graph/src/metadata/sql_translatable.rs

@@ -7,6 +7,7 @@ to the `pgx` framework and very subject to change between versions. While you ma
 
 */
 use std::error::Error;
+use std::fmt::Display;
 
 use super::return_variant::ReturnsError;
 use super::{FunctionMetadataTypeEntity, Returns};
@@ -18,6 +19,7 @@ pub enum ArgumentError {
     BareU8,
     SkipInArray,
     Datum,
+    NotValidAsArgument(&'static str),
 }
 
 impl std::fmt::Display for ArgumentError {
@@ -38,6 +40,9 @@ impl std::fmt::Display for ArgumentError {
             ArgumentError::Datum => {
                 write!(f, "A Datum as an argument means that `sql = \"...\"` must be set in the declaration")
             }
+            ArgumentError::NotValidAsArgument(type_name) => {
+                write!(f, "`{}` is not able to be used as a function argument", type_name)
+            }
         }
     }
 }
@@ -102,6 +107,19 @@ pub unsafe trait SqlTranslatable {
     }
 }
 
+unsafe impl<E> SqlTranslatable for Result<(), E>
+where
+    E: Display,
+{
+    fn argument_sql() -> Result<SqlMapping, ArgumentError> {
+        Err(ArgumentError::NotValidAsArgument("()"))
+    }
+
+    fn return_sql() -> Result<Returns, ReturnsError> {
+        Ok(Returns::One(SqlMapping::literal("VOID")))
+    }
+}
+
 unsafe impl<T> SqlTranslatable for Option<T>
 where
     T: SqlTranslatable,
@@ -135,7 +153,7 @@ where
 unsafe impl<T, E> SqlTranslatable for Result<T, E>
 where
     T: SqlTranslatable,
-    E: std::error::Error + 'static,
+    E: Display,
 {
     fn argument_sql() -> Result<SqlMapping, ArgumentError> {
         T::argument_sql()

pgx-sql-entity-graph/src/pg_extern/mod.rs

@@ -419,6 +419,23 @@ impl PgExtern {
                     quote_spanned! { self.func.sig.output.span() =>
                        ::pgx::fcinfo::pg_return_void()
                     }
+                } else if retval_ty.result {
+                    if retval_ty.optional.is_some() {
+                        // returning `Result<Option<T>>`
+                        quote_spanned! {
+                            self.func.sig.output.span() =>
+                                match ::pgx::datum::IntoDatum::into_datum(#result_ident) {
+                                    Some(datum) => datum,
+                                    None => ::pgx::fcinfo::pg_return_null(#fcinfo_ident),
+                                }
+                        }
+                    } else {
+                        // returning Result<T>
+                        quote_spanned! {
+                            self.func.sig.output.span() =>
+                                ::pgx::datum::IntoDatum::into_datum(#result_ident).unwrap_or_else(|| panic!("returned Datum was NULL"))
+                        }
+                    }
                 } else if retval_ty.resolved_ty == syn::parse_quote!(pg_sys::Datum)
                     || retval_ty.resolved_ty == syn::parse_quote!(pgx::pg_sys::Datum)
                     || retval_ty.resolved_ty == syn::parse_quote!(::pgx::pg_sys::Datum)

pgx-sql-entity-graph/src/used_type.rs

@@ -27,8 +27,10 @@ pub struct UsedType {
     /// Set via `VariadicArray` or `variadic!()`
     pub variadic: bool,
     pub default: Option<String>,
-    /// Set via the type being an `Option`.
+    /// Set via the type being an `Option` or a `Result<Option<T>>`.
     pub optional: Option<syn::Type>,
+    /// Set via the type being a `Result<T>`
+    pub result: bool,
 }
 
 impl UsedType {
@@ -131,8 +133,8 @@ impl UsedType {
             original => (original, None),
         };
 
-        // In this  step, we go look at the resolved type and determine if it is a variadic, optional, etc.
-        let (resolved_ty, variadic, optional) = match resolved_ty {
+        // In this  step, we go look at the resolved type and determine if it is a variadic, optional, result, etc.
+        let (resolved_ty, variadic, optional, result) = match resolved_ty {
             syn::Type::Path(type_path) => {
                 let path = &type_path.path;
                 let last_segment = path.segments.last().ok_or(syn::Error::new(
@@ -141,6 +143,68 @@ impl UsedType {
                 ))?;
                 let ident_string = last_segment.ident.to_string();
                 match ident_string.as_str() {
+                    "Result" => {
+                        match &last_segment.arguments {
+                            syn::PathArguments::AngleBracketed(angle_bracketed) => {
+                                match angle_bracketed.args.first().ok_or(syn::Error::new(
+                                    angle_bracketed.span(),
+                                    "No inner arg for Result<T, E> found",
+                                ))? {
+                                    syn::GenericArgument::Type(inner_ty) => {
+                                        match inner_ty {
+                                            // Result<$Type<T>>
+                                            syn::Type::Path(ref inner_type_path) => {
+                                                let path = &inner_type_path.path;
+                                                let last_segment =
+                                                    path.segments.last().ok_or(syn::Error::new(
+                                                        path.span(),
+                                                        "No last segment found while scanning path",
+                                                    ))?;
+                                                let ident_string = last_segment.ident.to_string();
+                                                match ident_string.as_str() {
+                                                    "VariadicArray" => (
+                                                        syn::Type::Path(type_path.clone()),
+                                                        true,
+                                                        Some(inner_ty.clone()),
+                                                        false,
+                                                    ),
+                                                    "Option" => (
+                                                        syn::Type::Path(type_path.clone()),
+                                                        false,
+                                                        Some(inner_ty.clone()),
+                                                        true,
+                                                    ),
+                                                    _ => (
+                                                        syn::Type::Path(type_path.clone()),
+                                                        false,
+                                                        None,
+                                                        true,
+                                                    ),
+                                                }
+                                            }
+                                            // Result<T>
+                                            _ => (
+                                                syn::Type::Path(type_path.clone()),
+                                                false,
+                                                None,
+                                                true,
+                                            ),
+                                        }
+                                    }
+                                    _ => {
+                                        return Err(syn::Error::new(
+                                            type_path.span().clone(),
+                                            "Unexpected Item found inside `Result` (expected Type)",
+                                        ))
+                                    }
+                                }
+                            }
+                            _ => return Err(syn::Error::new(
+                                type_path.span().clone(),
+                                "Unexpected Item found inside `Result` (expected Angle Brackets)",
+                            )),
+                        }
+                    }
                     "Option" => {
                         // Option<VariadicArray<T>>
                         match &last_segment.arguments {
@@ -166,11 +230,13 @@ impl UsedType {
                                                         syn::Type::Path(type_path.clone()),
                                                         true,
                                                         Some(inner_ty.clone()),
+                                                        false,
                                                     ),
                                                     _ => (
                                                         syn::Type::Path(type_path.clone()),
                                                         false,
                                                         Some(inner_ty.clone()),
+                                                        false,
                                                     ),
                                                 }
                                             }
@@ -179,6 +245,7 @@ impl UsedType {
                                                 syn::Type::Path(type_path.clone()),
                                                 false,
                                                 Some(inner_ty.clone()),
+                                                false,
                                             ),
                                         }
                                     }
@@ -199,15 +266,15 @@ impl UsedType {
                         }
                     }
                     // VariadicArray<T>
-                    "VariadicArray" => (syn::Type::Path(type_path), true, None),
+                    "VariadicArray" => (syn::Type::Path(type_path), true, None, false),
                     // T
-                    _ => (syn::Type::Path(type_path), false, None),
+                    _ => (syn::Type::Path(type_path), false, None, false),
                 }
             }
-            original => (original, false, None),
+            original => (original, false, None, false),
         };
 
-        Ok(Self { original_ty, resolved_ty, optional, variadic, default, composite_type })
+        Ok(Self { original_ty, resolved_ty, optional, result, variadic, default, composite_type })
     }
 
     pub fn entity_tokens(&self) -> syn::Expr {

pgx-tests/Cargo.toml

@@ -48,6 +48,9 @@ time = "0.3.17"
 eyre = "0.6.8"
 thiserror = "1.0"
 
+[dev-dependencies]
+eyre = "0.6.8"  # testing functions that return `eyre::Result`
+
 [dependencies.pgx]
 path = "../pgx"
 default-features = false

pgx-tests/src/tests/mod.rs

@@ -38,6 +38,7 @@ mod pgbox_tests;
 mod pgx_module_qualification;
 mod postgres_type_tests;
 mod range_tests;
+mod result_tests;
 mod schema_tests;
 mod shmem_tests;
 mod spi_tests;

pgx-tests/src/tests/result_tests.rs

@@ -0,0 +1,82 @@
+#[cfg(any(test, feature = "pg_test"))]
+#[pgx::pg_schema]
+mod tests {
+    #[allow(unused_imports)]
+    use crate as pgx_tests;
+    use pgx::prelude::*;
+    use std::convert::Infallible;
+
+    #[pg_extern]
+    fn return_result() -> Result<i32, Infallible> {
+        Ok(42)
+    }
+
+    #[pg_extern]
+    fn return_some_optional_result() -> Result<Option<i32>, Infallible> {
+        Ok(Some(42))
+    }
+
+    #[pg_extern]
+    fn return_none_optional_result() -> Result<Option<i32>, Infallible> {
+        Ok(None)
+    }
+
+    #[pg_extern]
+    fn return_some_error() -> Result<i32, Box<dyn std::error::Error>> {
+        Err("error!".into())
+    }
+
+    #[pg_extern]
+    fn return_eyre_result() -> eyre::Result<i32> {
+        Ok(42)
+    }
+
+    #[pg_extern]
+    fn return_eyre_result_error() -> eyre::Result<i32> {
+        Err(eyre::eyre!("error!"))
+    }
+
+    #[pg_test(error = "No such file or directory (os error 2)")]
+    fn test_return_io_error() -> Result<(), std::io::Error> {
+        std::fs::read("/tmp/i-sure-hope-this-doest-exist.pgx-tests::test_result_result").map(|_| ())
+    }
+
+    #[pg_test]
+    fn test_return_result() {
+        Spi::get_one::<i32>("SELECT tests.return_result()").expect("SPI returned NULL");
+    }
+
+    #[pg_test]
+    fn test_return_some_optional_result() {
+        assert_eq!(Some(42), Spi::get_one::<i32>("SELECT tests.return_some_optional_result()"));
+    }
+
+    #[pg_test]
+    fn test_return_none_optional_result() {
+        assert_eq!(None, Spi::get_one::<i32>("SELECT tests.return_none_optional_result()"));
+    }
+
+    #[pg_test(error = "error!")]
+    fn test_return_some_error() {
+        Spi::get_one::<i32>("SELECT tests.return_some_error()").expect("SPI returned NULL");
+    }
+
+    #[pg_test]
+    fn test_return_eyre_result() {
+        Spi::get_one::<i32>("SELECT tests.return_eyre_result()").expect("SPI returned NULL");
+    }
+
+    #[pg_test(error = "error!")]
+    fn test_return_eyre_result_error() {
+        Spi::get_one::<i32>("SELECT tests.return_eyre_result_error()").expect("SPI returned NULL");
+    }
+
+    #[pg_test(error = "got proper sql errorcode")]
+    fn test_proper_sql_errcode() -> Option<i32> {
+        PgTryBuilder::new(|| Spi::get_one::<i32>("SELECT tests.return_eyre_result_error()"))
+            .catch_when(PgSqlErrorCode::ERRCODE_DATA_EXCEPTION, |_| {
+                panic!("got proper sql errorcode")
+            })
+            .execute()
+    }
+}

pgx/src/datum/into.rs

@@ -13,7 +13,9 @@ Use of this source code is governed by the MIT license that can be found in the
 //! cast of the primitive type to pg_sys::Datum
 
 use crate::{pg_sys, rust_regtypein, PgBox, PgOid, WhoAllocated};
-use pgx_pg_sys::{Datum, Oid};
+use core::fmt::Display;
+use pgx_pg_sys::errcodes::PgSqlErrorCode;
+use pgx_pg_sys::{ereport, Datum, Oid};
 
 /// Convert a Rust type into a `pg_sys::Datum`.
 ///
@@ -120,6 +122,33 @@ where
     }
 }
 
+impl<T, E> IntoDatum for Result<T, E>
+where
+    T: IntoDatum,
+    E: Display,
+{
+    /// Returns The `Option<pg_sys::Datum>` representation of this Result's `Ok` variant.
+    ///
+    /// ## Panics
+    ///
+    /// If this Result represents an error, then that error is raised as a Postgres ERROR, using
+    /// the [`PgSqlErrorCode::ERRCODE_DATA_EXCEPTION`] error code
+    #[inline]
+    fn into_datum(self) -> Option<Datum> {
+        match self {
+            Ok(v) => v.into_datum(),
+            Err(e) => {
+                ereport!(ERROR, PgSqlErrorCode::ERRCODE_DATA_EXCEPTION, &format!("{}", e));
+            }
+        }
+    }
+
+    #[inline]
+    fn type_oid() -> pg_sys::Oid {
+        T::type_oid()
+    }
+}
+
 /// for bool
 impl IntoDatum for bool {
     #[inline]
@@ -380,15 +409,16 @@ impl IntoDatum for Vec<u8> {
     }
 }
 
-/// for NULL -- always converts to `None`
+/// for VOID
 impl IntoDatum for () {
     #[inline]
     fn into_datum(self) -> Option<pg_sys::Datum> {
-        None
+        // VOID isn't very useful, but Postgres represents it as a non-null Datum with a zero value
+        Some(Datum::from(0))
     }
 
     fn type_oid() -> u32 {
-        pg_sys::BOOLOID
+        pg_sys::VOIDOID
     }
 }