Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to create SRF function with SETOF #200

Closed
rustprooflabs opened this issue Feb 11, 2023 · 8 comments
Closed

Unable to create SRF function with SETOF #200

rustprooflabs opened this issue Feb 11, 2023 · 8 comments

Comments

@rustprooflabs
Copy link
Contributor

I have not been able to create a set returning function using SETOF. I get the same basic error using both a custom type for the return as well as the generic record type.

Use custom type

CREATE TYPE srf_row AS (id BIGINT);
CREATE FUNCTION spi_srf()
    RETURNS SETOF srf_row
    LANGUAGE plrust
AS
$$
    let query = "SELECT id::BIGINT FROM generate_series(1, 3) id;";

    Spi::connect(|client| {
        let mut results = Vec::new();
        let mut tup_table = client.select(query, None, None)?;

        while let Some(row) = tup_table.next() {
            let id = row["id"].value::<i64>();
            results.push((id,));
        }
        Ok(TableIterator::new(results.into_iter()))
    })

$$
;

This fails with the \errverbose output:

ERROR:  XX000: 
   0: Oid `oid=#83781` was not mappable to a Rust type

Location:
   plrust/src/user_crate/crate_variant.rs:70

Backtrace omitted. Run with RUST_BACKTRACE=1 environment variable to display it.
Run with RUST_BACKTRACE=full to include source snippets.
LOCATION:  lib.rs:191

Confirmed the custom type is the oid mentioned.

select oid, typname from pg_type WHERE oid = 83781;
┌───────┬─────────┐
│  oid  │ typname │
╞═══════╪═════════╡
│ 83781 │ srf_row │
└───────┴─────────┘

Record type

Attempting to return SETOF record also fails with the same basic error.

CREATE FUNCTION spi_srf()
    RETURNS SETOF record
    LANGUAGE plrust
AS
$$
    let query = "SELECT id::BIGINT FROM generate_series(1, 3) id;";

    Spi::connect(|client| {
        let mut results = Vec::new();
        let mut tup_table = client.select(query, None, None)?;

        while let Some(row) = tup_table.next() {
            let id = row["id"].value::<i64>();
            results.push((id,));
        }
        Ok(TableIterator::new(results.into_iter()))
    })

$$
;

Same error location on the RECORDOID.

ERROR:  XX000: 
   0: Oid `oid={#2249, builtin: RECORDOID}` was not mappable to a Rust type

Location:
   plrust/src/user_crate/crate_variant.rs:70

Backtrace omitted. Run with RUST_BACKTRACE=1 environment variable to display it.
Run with RUST_BACKTRACE=full to include source snippets.
LOCATION:  lib.rs:191

@eeeebbbbrrrr
Copy link
Collaborator

plrust doesn’t yet support custom types like this. It only supports the built-in Postgres types

@rustprooflabs
Copy link
Contributor Author

The record type is built-in and still fails: oid={#2249, builtin: RECORDOID} was not mappable to a Rust type

I guess I don't see any way to create an SRF then. Am I missing something? The only other way to define a return type for SETOF that I know of is to reference a table. This fails with the same error.

CREATE TABLE foo (id BIGINT);
CREATE FUNCTION spi_srf()
    RETURNS SETOF foo
    LANGUAGE plrust
AS
$$
...
$$

@eeeebbbbrrrr
Copy link
Collaborator

You can return setof text or bigint for example. Not any kind of UDT.

@rustprooflabs
Copy link
Contributor Author

Okay. Changing to BIGINT to make it a single column SRF results in new errors: missing lifetime specifier and mismatched types. I don't see anything obviously wrong with the function's code.

CREATE FUNCTION spi_srf()
    RETURNS SETOF BIGINT
    LANGUAGE plrust
AS
$$
    let query = "SELECT id::BIGINT FROM generate_series(1, 3) id;";

    Spi::connect(|client| {
        let mut results = Vec::new();
        let mut tup_table = client.select(query, None, None)?;

        while let Some(row) = tup_table.next() {
            let id = row["id"].value::<i64>();
            results.push((id,));
        }
        Ok(TableIterator::new(results.into_iter()))
    })

$$
;

Full error

ERROR:  XX000: 
   0: `cargo build` failed

Location:
   /rustc/897e37553bba8b42751c67658967889d11ecd120/library/core/src/convert/mod.rs:552

`cargo build` stderr:
       Updating git repository `https://github.com/tcdi/plrust`
       Updating crates.io index
      Compiling plrust_fn_oid_16392_83827 v0.0.0 (/tmp/plrust_fn_oid_16392_83827)
   error[E0106]: missing lifetime specifier
    --> src/lib.rs:7:42
     |
   7 |         Option<::pgx::iter::SetOfIterator<Option<i64>>>,
     |                                          ^ expected named lifetime parameter
     |
     = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from
   help: consider using the `'static` lifetime
     |
   7 |         Option<::pgx::iter::SetOfIterator<'static, Option<i64>>>,
     |                                           ++++++++

   error[E0106]: missing lifetime specifier
     --> src/lib.rs:27:42
      |
   27 |         Option<::pgx::iter::SetOfIterator<Option<i64>>>,
      |                                          ^ expected named lifetime parameter
      |
      = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from
   help: consider using the `'static` lifetime
      |
   27 |         Option<::pgx::iter::SetOfIterator<'static, Option<i64>>>,
      |                                           ++++++++

   error[E0308]: mismatched types
     --> src/lib.rs:11:9
      |
   6  |       fn plrust_fn_oid_16392_83827() -> ::std::result::Result<
      |  _______________________________________-
   7  | |         Option<::pgx::iter::SetOfIterator<Option<i64>>>,
   8  | |         Box<dyn ::std::error::Error>,
   9  | |     > {
      | |_____- expected `Result<Option<SetOfIterator<'static, Option<i64>>>, Box<(dyn std::error::Error + 'static)>>` because of return type
   10 |           let query = "SELECT id::BIGINT FROM generate_series(1, 3) id;";
   11 | /         Spi::connect(|client| {
   12 | |             let mut results = Vec::new();
   13 | |             let mut tup_table = client.select(query, None, None)?;
   14 | |             while let Some(row) = tup_table.next() {
   ...  |
   18 | |             Ok(TableIterator::new(results.into_iter()))
   19 | |         })
      | |__________^ expected enum `Option`, found struct `trusted_pgx::TableIterator`
      |
      = note: expected enum `Result<Option<SetOfIterator<'static, Option<i64>>>, Box<(dyn std::error::Error + 'static)>>`
                 found enum `Result<trusted_pgx::TableIterator<'_, (Result<Option<i64>, trusted_pgx::spi::Error>,)>, _>`

   error[E0308]: mismatched types
     --> src/lib.rs:31:9
      |
   26 |       fn plrust_fn_oid_16392_83827() -> ::std::result::Result<
      |  _______________________________________-
   27 | |         Option<::pgx::iter::SetOfIterator<Option<i64>>>,
   28 | |         Box<dyn ::std::error::Error>,
   29 | |     > {
      | |_____- expected `Result<Option<SetOfIterator<'static, Option<i64>>>, Box<(dyn std::error::Error + 'static)>>` because of return type
   30 |           let query = "SELECT id::BIGINT FROM generate_series(1, 3) id;";
   31 | /         Spi::connect(|client| {
   32 | |             let mut results = Vec::new();
   33 | |             let mut tup_table = client.select(query, None, None)?;
   34 | |             while let Some(row) = tup_table.next() {
   ...  |
   38 | |             Ok(TableIterator::new(results.into_iter()))
   39 | |         })
      | |__________^ expected enum `Option`, found struct `trusted_pgx::TableIterator`
      |
      = note: expected enum `Result<Option<SetOfIterator<'static, Option<i64>>>, Box<(dyn std::error::Error + 'static)>>`
                 found enum `Result<trusted_pgx::TableIterator<'_, (Result<Option<i64>, trusted_pgx::spi::Error>,)>, _>`

   Some errors have detailed explanations: E0106, E0308.
   For more information about an error, try `rustc --explain E0106`.
   error: could not compile `plrust_fn_oid_16392_83827` due to 4 previous errors


Source Code:
   #![deny(unsafe_op_in_unsafe_fn)]
   pub mod opened {
       #[allow(unused_imports)]
       use pgx::prelude::*;
       #[pg_extern]
       fn plrust_fn_oid_16392_83827() -> ::std::result::Result<
           Option<::pgx::iter::SetOfIterator<Option<i64>>>,
           Box<dyn ::std::error::Error>,
       > {
           let query = "SELECT id::BIGINT FROM generate_series(1, 3) id;";
           Spi::connect(|client| {
               let mut results = Vec::new();
               let mut tup_table = client.select(query, None, None)?;
               while let Some(row) = tup_table.next() {
                   let id = row["id"].value::<i64>();
                   results.push((id,));
               }
               Ok(TableIterator::new(results.into_iter()))
           })
       }
   }
   mod forbidden {
       #![forbid(unsafe_code)]
       #[allow(unused_imports)]
       use pgx::prelude::*;
       fn plrust_fn_oid_16392_83827() -> ::std::result::Result<
           Option<::pgx::iter::SetOfIterator<Option<i64>>>,
           Box<dyn ::std::error::Error>,
       > {
           let query = "SELECT id::BIGINT FROM generate_series(1, 3) id;";
           Spi::connect(|client| {
               let mut results = Vec::new();
               let mut tup_table = client.select(query, None, None)?;
               while let Some(row) = tup_table.next() {
                   let id = row["id"].value::<i64>();
                   results.push((id,));
               }
               Ok(TableIterator::new(results.into_iter()))
           })
       }
   }


Backtrace omitted. Run with RUST_BACKTRACE=1 environment variable to display it.
Run with RUST_BACKTRACE=full to include source snippets.
LOCATION:  lib.rs:191

@eeeebbbbrrrr
Copy link
Collaborator

eeeebbbbrrrr commented Feb 13, 2023

All plrust functions return Result<Option<T>> so that they can decide to return a non-error value (Ok(Some(...))), a non-error NULL (Ok(None)), or an error (Err(...)).

Your function here, at the end of the Spi::connect(), returns Result<TableIterator<...>> instead of Result<Option<TableIterator<...>>>... so, it should instead read Ok(Some(TableIterator::new(results.into_iter()))).

However, and this is fun, plrust doesn't support TableIterator yet, only SetOfIterator, so in fact, your function should be rewritten as:

CREATE FUNCTION spi_srf()
    RETURNS SETOF BIGINT
    LANGUAGE plrust
AS
$$
    let query = "SELECT id::BIGINT FROM generate_series(1, 3) id;";

    Spi::connect(|client| {
        let mut results = Vec::new();
        let mut tup_table = client.select(query, None, None)?;

        while let Some(row) = tup_table.next() {
            let id = row["id"].value::<i64>()?;
            results.push(id);
        }
        Ok(Some(SetOfIterator::new(results)))
    })

$$;

Also note the ? operator is used at the end of row["id"].value::<i64>(). .value() returns a Result<Option<i64>> (in this case), and we want to push, for convenience, any error getting the value as an i64 back up as an error.

@eeeebbbbrrrr
Copy link
Collaborator

however, it does look like there's a bug in plrust preventing this from working. I'll fix...

eeeebbbbrrrr added a commit that referenced this issue Feb 13, 2023
This adds a `<'a>` lifetime to all user functions so that the few cases where arguments are borrowed (`&'a str`) and return types might borrow (`SetOfIterator<'a, ...>)` can do so.

Also blindly adds the `#[allow(unused_lifetime)]` annotation to all instances of user functions so that compilation warnings won't happen in the cases where all args/return don't borrow.
@eeeebbbbrrrr
Copy link
Collaborator

I've put up a PR to fix the plrust-side of the problem. #202

We'll get this merged ASAP.

workingjubilee pushed a commit that referenced this issue Feb 13, 2023
This adds a `<'a>` lifetime to all user functions so that the few cases where arguments are borrowed (`&'a str`) and return types might borrow (`SetOfIterator<'a, ...>)` can do so.

Also blindly adds the `#[allow(unused_lifetime)]` annotation to all instances of user functions so that compilation warnings won't happen in the cases where all args/return don't borrow.
@rustprooflabs
Copy link
Contributor Author

Success, thank you! Closing.

select spi_srf();
┌─────────┐
│ spi_srf │
╞═════════╡
│       1 │
│       2 │
│       3 │
└─────────┘

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants