sqlx icon indicating copy to clipboard operation
sqlx copied to clipboard

Implementation of `sqlx::Decode` is not general enough

Open ajosecueto opened this issue 4 years ago • 26 comments

The error is generated when i'm using a struct with sqlx::Type and some field is an Option. i tried to decode manually my struct but the error is still there.

Sample sql, structs and query

`sql create table demo_seller ( id int not null constraint demo_seller_pk primary key username varchar(30) not null, first_name varchar(30) not null, last_name varchar(30) not null, photo varchar(150) );

create table demo_shop ( shop_id int not null constraint demo_shop_pk primary key, created_at timestamptz default now() not null, price numeric(15,3) not null, seller_id int not null ); `

`rust #[derive(Serialize, Deserialize, FromRow, sqlx::Type)] pub struct Seller { id: i32, username: String, first_name: String, last_name: String, photo: Option< String >, }

#[derive(Serialize, Deserialize, FromRow)] pub struct Shop{ shop_id: i32, price: Decimal, seller: Seller }

let shops: Vec<Shop> = sqlx::query_as!(Shop, r#" SELECT s.shop_id, s.price, (sl.id, sl.username, sl.first_name, sl.last_name, sl.photo) as "seller!:Seller" FROM demo_shop s inner join demo_seller sl on s.seller_id = sl.id "#).fetch_all(&*pool).await?; `

////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// Implementation of sqlx::Decode is not general enough

0 | / pub trait Decode<'r, DB: Database>: Sized { 61 | | /// Decode a new value of this type using a raw value from the database. 62 | | fn decode(value: <DB as HasValueRef<'r>>::ValueRef) -> Result<Self, BoxDynError>; 63 | | } | |_- trait sqlx::Decode defined here | = note: std::option::Option<std::string::String> must implement sqlx::Decode<'0, Postgres>, for any lifetime '0... = note: ...but std::option::Option<std::string::String> actually implements sqlx::Decode<'1, Postgres>, for some specific lifetime '1

ajosecueto avatar Feb 04 '21 08:02 ajosecueto

struct with options fields cant use #[derive(sqlx::Type)]

Implementation of sqlx::Decode is not general enough

ajosecueto avatar Feb 04 '21 08:02 ajosecueto

This might be fixed on master and/or a newer version of Rust (which one are you using?) as I can't reproduce it, though there's another problem relating to the sqlx::Type on master, I'll create a PR for that shortly..

jplatte avatar Feb 04 '21 12:02 jplatte

This might be fixed on master and/or a newer version of Rust (which one are you using?) as I can't reproduce it, though there's another problem relating to the sqlx::Type on master, I'll create a PR for that shortly..

rustc 1.49.0 (e1884a8e3 2020-12-29)

ajosecueto avatar Feb 04 '21 16:02 ajosecueto

Can you try whether this still happens with 0.5.1?

jplatte avatar Feb 04 '21 16:02 jplatte

I have the same issue with Option on 0.5.1 (on rustc 1.51.0-nightly (d1aed50ab 2021-01-26))

Kruptein avatar Feb 05 '21 09:02 Kruptein

For some really weird reason, this happens with a String field and an Option<String> field, but not with an i32 field and an Option<String> field. Minimal repro:

#[derive(sqlx::Type)]
pub struct Seller {
    username: String,
    photo: Option<String>,
}

jplatte avatar Feb 05 '21 12:02 jplatte

Cleaned up the macro output to reduce this to

use std::error::Error;

use sqlx::{decode::Decode, postgres::PgValueRef, types::Type, Postgres};

pub struct Seller {
    //username: String,
    photo: Option<String>,
}

impl<'r> Decode<'r, Postgres> for Seller
where
    String: Decode<'r, Postgres>,
    String: Type<Postgres>,
    Option<String>: Decode<'r, Postgres>,
    Option<String>: Type<Postgres>,
{
    fn decode(value: PgValueRef<'r>) -> Result<Self, Box<dyn Error + 'static + Send + Sync>> {
        let mut decoder = sqlx::postgres::types::PgRecordDecoder::new(value)?;
        //let username = decoder.try_decode::<String>()?;
        let photo = decoder.try_decode::<Option<String>>()?;
        Ok(Seller {
            // username
            photo,
        })
    }
}

Removing the String: Decode<'r, Postgres> bound fixed the issue.

I'm guessing this is probably a bug in rustc.

jplatte avatar Feb 05 '21 12:02 jplatte

It also doesn't make sense to me that SQLx would emit these bounds though. Generally, these should be emitted for generic parameters, not for field types. That theoretically limits use cases but works around this issue, is what other derives do (to avoid making internal field types part of possibly public API) and should not be an issue in practice.

EDIT: Actually it does make sense for Decode impls that are generic over the database. That's not the case here, but often it is.

jplatte avatar Feb 05 '21 12:02 jplatte

should I as a workaround write my own Decode impl based on what you have here above (removing the String: Decode stuff) or is there some easier workaround ?

Kruptein avatar Feb 05 '21 12:02 Kruptein

@Kruptein You would have to write your own Decode, Encode and Type implementations. You can use cargo-expand to obtain the output of the macro invocation on your type and customize that. For now yes, that's the only workaround. I'll probably also create a PR working around this in the coming days (and also create an issue on rustc).

jplatte avatar Feb 05 '21 12:02 jplatte

Ok cool, I'll see if I can figure it out and otherwise I'll have to be patient :)

Kruptein avatar Feb 05 '21 13:02 Kruptein

cargo-expand is really a great help indeed, the first two structs I implemented manually indeed work after removing the String: Decode<'r, Postgres> line. I do also have a struct with an Option that has a similar issue, where removing the Decode for the i32 fixes it.

Kruptein avatar Feb 05 '21 18:02 Kruptein

In general you shouldn't need any of these bounds on concrete types. That's only going to be necessary for generic parameters (when deriving / implementing Decode for a generic type).

jplatte avatar Feb 05 '21 18:02 jplatte

Yeah I just wanted to let you know because you couldn't reproduce it with i32's (https://github.com/launchbadge/sqlx/issues/1031#issuecomment-773997308)

Kruptein avatar Feb 05 '21 19:02 Kruptein

Okay, actually I don't think I'm gonna write the PR for this. My suggested fix is making these two loops: [encode, decode] work over the type's generic type parameter rather than the field types. That's at least theoretically a breaking change, but only for generic structs that derive sqlx::Type, of which there are probably very few, if any, out there. A similar change should probably be done for the other cases where split_for_impl is used, simply because currently the derives leak the types of potentially private fields into the public API (this is why derive(Clone) for a generic type bounds on Clone for all generic type params which results in overly strict bounds sometimes, e.g. when a T is only used as Arc<T>).

EDIT: I'll open a PR for #[automatically_derived] though, as I've already done that.

jplatte avatar Feb 08 '21 14:02 jplatte

Finally was able to reduce this further and posted a rust issue: https://github.com/rust-lang/rust/issues/82219

jplatte avatar Feb 17 '21 14:02 jplatte

Hi, With decimal feature included, I get this

the trait `sqlx::Decode<'_, sqlx::Any>` is not implemented for `rust_decimal::Decimal`

victorteokw avatar Oct 14 '22 07:10 victorteokw

Any news on this matter? Should I use SeaORM instead of manually trying to decode the value into a struct?

acomanescu avatar Oct 21 '23 13:10 acomanescu

In my case I need a generic trait to use with sea-query, and I get this error:

error[E0277]: the trait bound `u64: sqlx::Decode<'_, sqlx::Any>` is not satisfied
  --> src\models\oauth2.rs:9:16
   |
9  | impl Model for OAuth2Client {
   |                ^^^^^^^^^^^^ the trait `sqlx::Decode<'_, sqlx::Any>` is not implemented for `u64`
   |
   = help: the trait `sqlx::Decode<'_, sqlx::MySql>` is implemented for `u64`
note: required for `OAuth2Client` to implement `for<'r> FromRow<'r, AnyRow>`
  --> src\models\oauth2.rs:13:48
   |
13 | #[derive(Clone, Debug, Serialize, Deserialize, FromRow)]
   |                                                ^^^^^^^ unsatisfied trait bound introduced in this `derive` macro
14 | pub struct OAuth2Client {
   |            ^^^^^^^^^^^^
note: required by a bound in `Model`
  --> C:\Projects\Services\src\core\model.rs:11:5
   |
10 | pub trait Model:
   |           ----- required by a bound in this trait
11 |     for<'r> FromRow<'r, AnyRow> + Send + Sync + Unpin + Serialize + DeserializeOwned
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `Model`
   = note: this error originates in the derive macro `FromRow` (in Nightly builds, run with -Z macro-backtrace for more info)
pub trait Model:
    for<'r> FromRow<'r, AnyRow> + Send + Sync + Unpin + Serialize + DeserializeOwned
{
    const TABLE: &'static str;

    fn table_ref() -> TableRef {
        TableRef::Table(StaticIden(Self::TABLE).into_iden())
    }
}

impl Model for OAuth2Client {
    const TABLE: &'static str = "oauth_client";
}

#[derive(Clone, Debug, Serialize, Deserialize, FromRow)]
pub struct OAuth2Client {
    pub id: u64,
    pub name: String,
    #[serde(rename = "type")]
    #[sqlx(rename = "type")]
    pub kind: OAuth2ClientType,
    pub organization_id: u32,
    pub trusted: bool,
    #[serde(skip_serializing_if = "Option::is_none")]
    pub package_id: Option<u32>,
    pub website_url: String,
    pub photo_url: String,
    pub background_url: Option<String>,
    #[serde(skip)]
    pub secret: String,
    pub custom_schema: Option<String>,
    pub redirects: Option<Value>,
    pub scopes: Option<String>,
    pub created: NaiveDateTime,
    pub updated: Option<NaiveDateTime>,
}

#[repr(u8)]
#[derive(Debug, Copy, Clone, PartialEq, Eq, Serialize, Deserialize, Type)]
#[serde(rename_all = "snake_case")]
pub enum OAuth2ClientType {
    Web = 0,
    Native = 1,
}

guilhermewerner avatar Nov 24 '23 00:11 guilhermewerner

Please, what is the conclusion about the "Implementation of sqlx::Decode is not general enough" problem?

Is the conclusion: "one cannot derive sqlx::Type over a struct with an Option field"?

frantisek-heca avatar Mar 19 '24 18:03 frantisek-heca

Hi @frantisek-heca, finally we don't use sqlx anyway. Have a look at Teo. This web framework has builtin ORM support. Write CRUD and aggregates are very fast with it. The framework handles the encoding and decoding for you.

victorteokw avatar Mar 19 '24 18:03 victorteokw

Hi @frantisek-heca, finally we don't use sqlx anyway. Have a look at Teo. This web framework has builtin ORM support. Write CRUD and aggregates are very fast with it. The framework handles the encoding and decoding for you.

How is this relevant?

acomanescu avatar Mar 19 '24 19:03 acomanescu

When I used the stable version, I ran into this problem with the Option type, but it worked fine once I workaround it.

However when testing with the nightly version of CI, I found this error on all #[derive(sqlx::Type)].

uonr avatar Apr 28 '24 17:04 uonr

When I used the stable version, I ran into this problem with the Option type, but it worked fine once I workaround it.

However when testing with the nightly version of CI, I found this error on all #[derive(sqlx::Type)].

What was your workaround for it?

henningcullin avatar Apr 28 '24 20:04 henningcullin

When I used the stable version, I ran into this problem with the Option type, but it worked fine once I workaround it. However when testing with the nightly version of CI, I found this error on all #[derive(sqlx::Type)].

What was your workaround for it?

@henningcullin

Just macro expansion and delete the where clause.

uonr avatar Apr 28 '24 22:04 uonr

@uonr I got my query_as! implementation working with nested structs that use the derived Type macro. It said the error was on an Option enum which was the same error as the op had. But when I used the query! macro to view the resulting record it became clear that issue was not the Type macro and the Option enum. The issue was that I didn't use Option on everything that could be nullable in the query.

henningcullin avatar Apr 30 '24 14:04 henningcullin

Closed by #2940

abonander avatar May 31 '24 20:05 abonander