pgcentralfoundation / pgrx

Build Postgres Extensions with Rust!
Other
3.55k stars 237 forks source link

Usability: bad error message for unsigned integer arguments #1536

Open nyurik opened 7 months ago

nyurik commented 7 months ago

This example produces an un-reasonably complex error, and only during cargo pgrx test phase. I also tried it with Option<u32>

#[pg_extern]
fn hello_postile(value: u32) -> String {
    format!("hello {value}")
}

#[cfg(any(test, feature = "pg_test"))]
#[pg_schema]
mod tests {
    use pgrx::prelude::*;

    #[pg_test]
    fn test_hello_postile() {
        assert_eq!("Hello 10", crate::hello_postile(10));
    }
}

Expected

Actual results

<compilation succeeded>
    Finished dev [unoptimized + debuginfo] target(s) in 7.49s

 Discovering SQL entities
  Discovered 4 SQL entities: 1 schemas (1 unique), 3 functions, 0 types, 0 enums, 0 sqls, 0 ords, 0 hashes, 0 aggregates, 0 triggers
     Writing SQL entities to /home/nyurik/.pgrx/13.13/pgrx-install/share/postgresql/extension/postile--0.0.0.sql
Error: 
   0: Could not write SQL to /home/nyurik/.pgrx/13.13/pgrx-install/share/postgresql/extension/postile--0.0.0.sql
   1: Macro expansion time suggested a source only mapping in return

Location:
   /home/nyurik/.cargo/registry/src/index.crates.io-6f17d22bba15001f/pgrx-sql-entity-graph-0.11.3/src/pg_extern/entity/mod.rs:200

  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ SPANTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

   0: cargo_pgrx::command::schema::generate_schema with pg_version=13.13 profile=Dev test=true path=/home/nyurik/.pgrx/13.13/pgrx-install/share/postgresql/extension/postile--0.0.0.sql features=["pg_test", "pg13"]
      at /home/nyurik/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cargo-pgrx-0.11.3/src/command/schema.rs:173
   1: cargo_pgrx::command::install::install_extension with pg_version=13.13 profile=Dev test=true features=["pg_test", "pg13"]
      at /home/nyurik/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cargo-pgrx-0.11.3/src/command/install.rs:114
   2: cargo_pgrx::command::install::execute
      at /home/nyurik/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cargo-pgrx-0.11.3/src/command/install.rs:63

Backtrace omitted. Run with RUST_BACKTRACE=1 environment variable to display it.
Run with RUST_BACKTRACE=full to include source snippets.

Location:
    /home/nyurik/.cargo/registry/src/index.crates.io-6f17d22bba15001f/pgrx-tests-0.11.3/src/framework.rs:372:20
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

failures:
    tests::pg_test_hello_postile

The last line is the closest indication to the cause, and pretty bad at that.

workingjubilee commented 7 months ago

This errors more eagerly on 0.12.0-alpha.0, as even if you run cargo check you will get something like:

   Compiling test_extension v0.0.0 (/home/jubilee/tcdi/test_extension)
error[E0277]: the trait bound `fn(u32) -> String: FunctionMetadata<_>` is not satisfied
 --> src/lib.rs:6:1
  |
6 | fn hello_postile(value: u32) -> String {
  | ^^ the trait `FunctionMetadata<_>` is not implemented for `fn(u32) -> String`
  |
  = help: the following other types implement trait `FunctionMetadata<A>`:
            <unsafe fn(T0) -> R as FunctionMetadata<(T0,)>>
            <unsafe fn(T0, T1) -> R as FunctionMetadata<(T0, T1)>>
            <unsafe fn(T0, T1, T2) -> R as FunctionMetadata<(T0, T1, T2)>>
            <unsafe fn(T0, T1, T2, T3) -> R as FunctionMetadata<(T0, T1, T2, T3)>>
            <unsafe fn(T0, T1, T2, T3, T4) -> R as FunctionMetadata<(T0, T1, T2, T3, T4)>>
            <unsafe fn(T0, T1, T2, T3, T4, T5) -> R as FunctionMetadata<(T0, T1, T2, T3, T4, T5)>>
            <unsafe fn(T0, T1, T2, T3, T4, T5, T6) -> R as FunctionMetadata<(T0, T1, T2, T3, T4, T5, T6)>>
            <unsafe fn(T0, T1, T2, T3, T4, T5, T6, T7) -> R as FunctionMetadata<(T0, T1, T2, T3, T4, T5, T6, T7)>>
          and 25 others

error[E0599]: the method `entity` exists for struct `PhantomData<u32>`, but its trait bounds were not satisfied
   --> src/lib.rs:5:1
    |
5   | #[pg_extern]
    | ^^^^^^^^^^^^ method cannot be called on `PhantomData<u32>` due to unsatisfied trait bounds
    |
   ::: /home/jubilee/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/marker.rs:809:1
    |
809 | pub struct PhantomData<T: ?Sized>;
    | --------------------------------- doesn't satisfy `PhantomData<u32>: PhantomDataExt`
    |
    = note: the following trait bounds were not satisfied:
            `u32: SqlTranslatable`
            which is required by `PhantomData<u32>: PhantomDataExt`

And while "PhantomDataExt???" is probably confounding, PhantomDataExt will likely not exist soon, but the same bounds will have to be satisfied, which will leave the obvious unsatisfied trait bounds as just fn(u32) -> String: FunctionMetadata and u32: SqlTranslatable.

And there is, indeed, no uint4 type as far as I'm aware.

workingjubilee commented 7 months ago

It is not terribly obvious how to generate significantly better messages than that, or how to make u32s "just work", considering the constraints, although there are ways, but they get increasingly hacky.

workingjubilee commented 7 months ago

Oh, the one free improvement is being more diligent about passing through spans so that the error message points directly to the u32 arg.

workingjubilee commented 7 months ago

It is not terribly obvious how to generate significantly better messages than that, considering the constraints, although there are ways, but they get increasingly hacky.

Some options for making u32 work:

None of these really "spark joy".