sqlx icon indicating copy to clipboard operation
sqlx copied to clipboard

Can't decode PostgreSQL arrays of custom types

Open Thomasdezeeuw opened this issue 2 years ago • 9 comments

I'm trying to decode an array of custom (PosgreSQL) types inside a record, using PgRecordDecoder. It hits the panic found here: https://github.com/launchbadge/sqlx/blob/2182925e92bc58ec4579c5b1e63377dcf5c6ae37/sqlx-core/src/postgres/type_info.rs#L866

It has an oid which matches the array version of my custom enum (e.g. _mytype for the code below).

I think the following should reproduce it.

CREATE TYPE MyType AS ENUM ('val1', 'val2');

SELECT 'abc', ARRAY['val1', 'val1']::MyType[];
struct S {
    text: String,
    array: Vec<E>,
}

#[derive(Copy, Clone, Debug, Eq, PartialEq, sqlx::Type)]
#[sqlx(rename_all = "snake_case", type_name = "mytype")]
enum E {
    Val1,
    Val2,
}

impl sqlx::postgres::PgHasArrayType for ValueBinding {
    fn array_type_info() -> sqlx::postgres::PgTypeInfo {
        // Array type name is the name of the element type prefixed with `_`.
        sqlx::postgres::PgTypeInfo::with_name("_mytype")
    }
}


impl S {
    fn try_decode<'r>(decoder: &mut PgRecordDecoder<'r>) -> Result<S, Box<dyn std::error::Error + Send + Sync + 'static>> {
        let text = decoder.try_decode::<String>()?;
        let array = decoder.try_decode::<Vec<ValueBinding>>()?;
        Ok(S { text, array })
    }
}

Thomasdezeeuw avatar Feb 03 '22 15:02 Thomasdezeeuw

Any update on this?

Thomasdezeeuw avatar Feb 10 '22 15:02 Thomasdezeeuw

Anyone who could point into the correct direction on how to fix this? We really need this feature.

Thomasdezeeuw avatar Feb 16 '22 14:02 Thomasdezeeuw

Try adding type_name = "MyType" to the #[sqlx] attribute on enum E

abonander avatar Feb 16 '22 20:02 abonander

Thank you for helping @abonander. I added the attribute, but it didn't help unfortunately. I looked up the oid and it's _mytype, so I implemented sqlx::postgres::PgHasArrayType for the Rust enum E, but that didn't work. I also tried a custom type instead of the Vec<E>, like https://github.com/launchbadge/sqlx/blob/32f1273565e1cd676e0514655fe8aec67b59b634/tests/postgres/postgres.rs#L1423-L1425, but no luck either.

Thomasdezeeuw avatar Feb 17 '22 09:02 Thomasdezeeuw

@abonander I guess I have the same issue. I described it on the forum: https://users.rust-lang.org/t/how-should-one-use-sqlx-with-newtype-pattern/73172. Is it a bug or a feature?

teenjuna avatar Mar 19 '22 11:03 teenjuna

Hi, I submitted support for arrays of custom types a few months ago, this issue is definitely a bug. Sorry. Arrays of custom types should be supported.

I've hit this bug in my own code last week and started working on a patch. I believe that the best solution to fix it is to refactor how SQLx handles types. Even if refactoring type resolution does not land immediately, I think it should help locating the source of the bug.

I will do my best to provide a fix in a timely manner.

demurgos avatar Apr 13 '22 16:04 demurgos

A quick update on this bug. Over the last weeks, I was working on refactoring Postgres types. The linked PR is pretty exploratory and will need a lot of work before it can be in a state where it could get merged, however it was pretty helpful: it turned this runtime panic into a compilation error.

The essence of the issue is that custom types must be resolved from the DB, and then decoders must get access to their metadata. However, decoders can't pass arbitrary state and context gets lost. When context gets lost, the decoders fallback to using PgTypeInfo::try_from_oid which only supports a handful of builtin types. This happens here in record.rs. (this also occurs in tuples, and a couple other decoders in text mode).

Using try_from_oid is always a bug inside a decoder: on custom types, this returns a type references (PgType::{DeclareByOid, DeclareByName}) and breaks the invariant that decoders only use resolved types (thus the panic).

A quick solution would be to maintain context in a thread local or other similar ambient store, similarly to how async runtime or logger sinks are passed down implicitly. I prefer serde's style of using explicit deserializers, but this a larger change.

I'll continue work on my experimental branch to fix the issue, but full support for the Postgres type system (including custom types) will need deeper changes to SQLx. This means that the fix may take a long time before it's merged. In the meantime, I can at least replace panics with regular errors.

demurgos avatar May 04 '22 15:05 demurgos

I have a good news: I have a branch where I am able to decode anonymous records and arrays of custom types.

The bad news is that it requires quite a few changes. I'll write a message going more into details but here are the main points:

  • Anonymous records are hard to handle because they act as barriers to type resolution: you don't know the types they contain until you start decoding them.
  • Values need access to the cache of fetched types while we decode them, so we can resolve values in anonymous records synchronously.
  • There needs to be a way to declare that we want to prefetch some types, so they are always present before we access any anonymous record.
  • Splitting the type representation between unresolved and resolved types is really helpful, but it brings some changes to the API

My plan is to fix Clippy issues / make sure CI passes, and then discuss how to implement the various changes required to support this feature.

demurgos avatar May 16 '22 21:05 demurgos

@demurgos is this fixed with #1483?

djc avatar Jun 09 '22 13:06 djc

@demurgos

Anonymous records are hard to handle because they act as barriers to type resolution: you don't know the types they contain until you start decoding them.

I could really use this. I don't care that the failure will occur at decode time, having some way have mapping anonymous records and arrays of anon records is better than no way.

My first attempt at this was similar to the following:

#[derive(Clone, Debug)]
pub struct Data {
    pub sub_data: Vec<SubData>,
    pub some_bool: bool,
}

// This just has scalar fields so derive `FromRow` works
// but this type _does not_ exist in the database, it's just for 
// data returned by a query I'm running.
#[derive(Clone, Debug, sqlx::Decode, sqlx::FromRow)]
pub struct SubData { ...stuff... }

impl<'r> sqlx::FromRow<'r, PgRow> for Data {
    fn from_row(row: &'r PgRow) -> Result<Self, sqlx::Error> {
        // The column with array of records in my returned data is named `sub_data`.
        let sub_data: Vec<SubData> = row
            .try_get::<Vec<PgRow>, _>("sub_data")?
            .into_iter()
            .map(|sub_data_tuple| SubData::from_row(&sub_data_tuple))
            .collect();
        Ok(Data {
            sub_data,
            some_bool: row.try_get("some_bool")?,
        })
    }
}

but I end up with

the trait bound `for<'a> PgRow: sqlx::Decode<'a, Postgres>` is not satisfied

It seems like if at least PgRow implemented sqlx::Decode then we'd have some flexible, albeit not-pretty, way around this. Is that change easy to make?

rex-remind101 avatar Dec 20 '22 01:12 rex-remind101

@rex-remind101 did you find a way?

frederikhors avatar Jan 11 '23 09:01 frederikhors

Looks like for arrays to work one needs a newtype and implement Type and Decode for it as seen in https://github.com/launchbadge/sqlx/pull/1483/files#diff-227edfb063a81fad99246f78753bb75d1d2262b95efc8c2115fe30081fd97c3bR1154-R1170

qrilka avatar Jan 04 '24 14:01 qrilka