Implementing diesel::Expression, etc., for custom types outside diesel?

[emk]

As a followup to #561, I'd like to try defining a custom ToSql and FromSql implementation for a type in a crate besides diesel. I'm going to try this with https://github.com/faradayio/bigml-rs, which a client for the commercial BigML machine learning service.

This crate contains a type Id, which represents a string of the form source/1234 or execution/5678. Because these strings refer to different types of resources, the Id type takes parameters: Id<Source> or Id<Execution>. So far, no problems.

#[derive(Clone, PartialEq, Eq, PartialOrd, Ord)]
pub struct Id<R: Resource> {
    /// The ID of the resource.
    id: String,
    /// A special 0-byte field which exists just to mention the type `R`
    /// inside the struct, and thus avoid compiler errors about unused type
    /// parameters.
    _phantom: PhantomData<R>,
}

I can declare ToSql and FromSql instances as follows:

    impl<R: Resource, DB: Backend<RawValue=[u8]>> FromSql<types::Text, DB> for Id<R> {
        fn from_sql(bytes: Option<&[u8]>) -> Result<Self, Box<Error+Send+Sync>> {
            (FromSql::<types::Text, DB>::from_sql(bytes) as Result<String, _>)
                .and_then(|s| {
                    Id::from_str(&s).map_err(|e| format!("{}", e).into())
                })
        }
    }

    impl<R: Resource, DB: Backend> ToSql<types::Text, DB> for Id<R>
        where for<'a> &'a str: ToSql<types::Text, DB>
    {
        fn to_sql<W: Write>(&self, out: &mut W)
                            -> Result<IsNull, Box<Error+Send+Sync>> {
            self.as_str().to_sql(out)
        }
    }

But if I try to use these impls, I run into problems. This code:

    table! {
        example_structs {
            id -> Integer,
            source_id -> Text,
        }
    }

    // This should compile without errors.
    #[derive(Queryable, Insertable)]
    #[table_name="example_structs"]
    struct ExampleStruct {
        id: i32,
        source_id: Id<Source>,
    }

...gives me the errors:

error[E0277]: the trait bound `resource::id::Id<resource::source::Source>: diesel::Expression` is not satisfied
   --> src/resource/id.rs:170:5
    |
170 |     #[derive(Queryable, Insertable)]
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expression_impls! for `resource::id::Id<resource::source::Source>`
    |
    = note: required because of the requirements on the impl of `diesel::Expression` for `&'insert resource::id::Id<resource::source::Source>`
    = note: required because of the requirements on the impl of `diesel::expression::AsExpression<diesel::types::Text>` for `&'insert resource::id::Id<resource::source::Source>`
    = note: this error originates in a macro outside of the current crate
error[E0277]: the trait bound `DB: diesel::backend::SupportsDefaultKeyword` is not satisfied
   --> src/resource/id.rs:170:5
    |
170 |     #[derive(Queryable, Insertable)]
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `diesel::backend::SupportsDefaultKeyword` is not implemented for `DB`
    |
    = help: consider adding a `where DB: diesel::backend::SupportsDefaultKeyword` bound
    = note: required because of the requirements on the impl of `diesel::persistable::InsertValues<DB>` for `(diesel::persistable::ColumnInsertValue<resource::id::diesel::example_structs::columns::id, diesel::expression::bound::Bound<diesel::types::Integer, &i32>>, diesel::persistable::ColumnInsertValue<resource::id::diesel::example_structs::columns::source_id, &resource::id::Id<resource::source::Source>>)`
    = note: required because of the requirements on the impl of `diesel::Insertable<resource::id::diesel::example_structs::table, DB>` for `&'insert resource::id::diesel::ExampleStruct`
    = note: this error originates in a macro outside of the current crate

error[E0277]: the trait bound `resource::id::Id<resource::source::Source>: diesel::query_builder::QueryFragment<DB>` is not satisfied
   --> src/resource/id.rs:170:5
    |
170 |     #[derive(Queryable, Insertable)]
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `diesel::query_builder::QueryFragment<DB>` is not implemented for `resource::id::Id<resource::source::Source>`
    |
    = note: required because of the requirements on the impl of `diesel::query_builder::QueryFragment<DB>` for `&resource::id::Id<resource::source::Source>`
    = note: required because of the requirements on the impl of `diesel::persistable::InsertValues<DB>` for `(diesel::persistable::ColumnInsertValue<resource::id::diesel::example_structs::columns::id, diesel::expression::bound::Bound<diesel::types::Integer, &i32>>, diesel::persistable::ColumnInsertValue<resource::id::diesel::example_structs::columns::source_id, &resource::id::Id<resource::source::Source>>)`
    = note: required because of the requirements on the impl of `diesel::Insertable<resource::id::diesel::example_structs::table, DB>` for `&'insert resource::id::diesel::ExampleStruct`
    = note: this error originates in a macro outside of the current crate

It looks like the missing pieces are basically:

        queryable_impls!($Source -> $Target,);
        expression_impls!($Source -> $Target,);

But the macros queryable_impls! and expression_impls! are private, and relatively large and complex. Is there a story for implementing these traits correctly from outside diesel itself?

[sgrif]

Is there a story for implementing these traits correctly from outside diesel itself?

Not at the moment. We should probably make those macros public (with a #[doc(hidden)] or a note that they are likely to have their APIs change pre-1.0, and decide if there's a better API post-1.0. Ideally I'd like to provide more blanket impls in Diesel itself and make those macros obsolete, but that at minimum would require specialization, and likely even that wouldn't be enough without the lattice rule.

Anyway it needs further exploration and discussion. I'm fine with making those macros public for the time being if you'd like to open a PR.

[aperezdc]

I am preparing a small PR to make the macros public, as I would like to use them as well. Would it make sense to also make primitive_impls! public?

Edit: There's also not_none! in the same source file, but it is small and easy to understand, so my guess is that it would be better to keep it private.

[dstu]

Thanks for this effort. It should make it easier to deal with #343.

AIUI, registering a new type T entails entails adding a diesel::types::HasSqlType<T> impl to a database type that is also in diesel. Since both the trait and the implementing type are in diesel, foreign crates can't add an impl. I worked around this in #580 by creating a trait that can be implemented for T directly and a blanket impl of HasSqlType. If it seems warranted, can we coordinate on this change? (I can factor it out into a separate PR if that seems warranted. Or maybe further discussion and design are warranted, since it is arguable that it would be simpler to do away with HasSqlType entirely.)

Edit: Sorry, I should have read the issue more closely. It looks like you are not adding a new database type, but registering a Rust type for use with Diesel. Please disregard the above if it does not actually apply to your situation.

[sgrif]

@DTSu No, HasSqlType<T> is considered local if T is local. https://github.com/diesel-rs/diesel_full_text_search/blob/master/src/lib.rs#L8-L27

[jimmycuadra]

Did the details of how to do this change in diesel 0.10? I'm now getting these errors, using diesel and diesel_codegen 0.10.0 (with the postgres feature):

error[E0277]: the trait bound `ruma_identifiers::UserId: diesel::Expression` is not satisfied
  --> src/models/profile.rs:18:1
   |
18 | #[derive(AsChangeset, Debug, Clone, Identifiable, Insertable, Queryable)]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `diesel::Expression` is not implemented for `ruma_identifiers::UserId`
   |
   = note: required because of the requirements on the impl of `diesel::Expression` for `&'insert ruma_identifiers::UserId`
   = note: required because of the requirements on the impl of `diesel::expression::AsExpression<diesel::types::Nullable<diesel::types::Text>>` for `&'insert ruma_identifiers::UserId`
   = note: this error originates in a macro outside of the current crate

error[E0277]: the trait bound `ruma_identifiers::UserId: diesel::Expression` is not satisfied
  --> src/models/profile.rs:18:1
   |
18 | #[derive(AsChangeset, Debug, Clone, Identifiable, Insertable, Queryable)]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `diesel::Expression` is not implemented for `ruma_identifiers::UserId`
   |
   = note: required because of the requirements on the impl of `diesel::Expression` for `&'insert ruma_identifiers::UserId`
   = note: required because of the requirements on the impl of `diesel::expression::AsExpression<diesel::types::Nullable<diesel::types::Text>>` for `&'insert ruma_identifiers::UserId`
   = note: this error originates in a macro outside of the current crate

error[E0277]: the trait bound `ruma_identifiers::UserId: diesel::Expression` is not satisfied
  --> src/models/profile.rs:18:1
   |
18 | #[derive(AsChangeset, Debug, Clone, Identifiable, Insertable, Queryable)]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `diesel::Expression` is not implemented for `ruma_identifiers::UserId`
   |
   = note: required because of the requirements on the impl of `diesel::Expression` for `&'insert ruma_identifiers::UserId`
   = note: required because of the requirements on the impl of `diesel::expression::AsExpression<diesel::types::Nullable<diesel::types::Text>>` for `&'insert ruma_identifiers::UserId`
   = note: this error originates in a macro outside of the current crate

error[E0277]: the trait bound `&'insert models::profile::Profile: diesel::Insertable<schema::profiles::table, DB>` is not satisfied
  --> src/models/profile.rs:18:1
   |
18 | #[derive(AsChangeset, Debug, Clone, Identifiable, Insertable, Queryable)]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `diesel::Insertable<schema::profiles::table, DB>` is not implemented for `&'insert models::profile::Profile`
   |
   = help: the following implementations were found:
             <&'insert models::profile::Profile as diesel::Insertable<schema::profiles::table, DB>>
   = note: this error originates in a macro outside of the current crate

error[E0277]: the trait bound `ruma_identifiers::UserId: diesel::Expression` is not satisfied
  --> src/models/profile.rs:18:1
   |
18 | #[derive(AsChangeset, Debug, Clone, Identifiable, Insertable, Queryable)]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `diesel::Expression` is not implemented for `ruma_identifiers::UserId`
   |
   = note: required because of the requirements on the impl of `diesel::Expression` for `&'insert ruma_identifiers::UserId`
   = note: required because of the requirements on the impl of `diesel::expression::AsExpression<diesel::types::Nullable<diesel::types::Text>>` for `&'insert ruma_identifiers::UserId`
   = note: this error originates in a macro outside of the current crate

error[E0277]: the trait bound `DB: diesel::backend::SupportsDefaultKeyword` is not satisfied
  --> src/models/profile.rs:18:1
   |
18 | #[derive(AsChangeset, Debug, Clone, Identifiable, Insertable, Queryable)]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `diesel::backend::SupportsDefaultKeyword` is not implemented for `DB`
   |
   = help: consider adding a `where DB: diesel::backend::SupportsDefaultKeyword` bound
   = note: required because of the requirements on the impl of `diesel::insertable::InsertValues<DB>` for `(diesel::insertable::ColumnInsertValue<schema::profiles::columns::id, &'insert ruma_identifiers::UserId>, diesel::insertable::ColumnInsertValue<schema::profiles::columns::avatar_url, diesel::expression::bound::Bound<diesel::types::Nullable<diesel::types::Text>, &std::option::Option<std::string::String>>>, diesel::insertable::ColumnInsertValue<schema::profiles::columns::displayname, diesel::expression::bound::Bound<diesel::types::Nullable<diesel::types::Text>, &std::option::Option<std::string::String>>>)`
   = note: required by `diesel::insertable::InsertValues`
   = note: this error originates in a macro outside of the current crate

error[E0277]: the trait bound `ruma_identifiers::UserId: diesel::query_builder::QueryFragment<DB>` is not satisfied
  --> src/models/profile.rs:18:1
   |
18 | #[derive(AsChangeset, Debug, Clone, Identifiable, Insertable, Queryable)]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `diesel::query_builder::QueryFragment<DB>` is not implemented for `ruma_identifiers::UserId`
   |
   = note: required because of the requirements on the impl of `diesel::query_builder::QueryFragment<DB>` for `&'insert ruma_identifiers::UserId`
   = note: required because of the requirements on the impl of `diesel::insertable::InsertValues<DB>` for `(diesel::insertable::ColumnInsertValue<schema::profiles::columns::id, &'insert ruma_identifiers::UserId>, diesel::insertable::ColumnInsertValue<schema::profiles::columns::avatar_url, diesel::expression::bound::Bound<diesel::types::Nullable<diesel::types::Text>, &std::option::Option<std::string::String>>>, diesel::insertable::ColumnInsertValue<schema::profiles::columns::displayname, diesel::expression::bound::Bound<diesel::types::Nullable<diesel::types::Text>, &std::option::Option<std::string::String>>>)`
   = note: required by `diesel::insertable::InsertValues`
   = note: this error originates in a macro outside of the current crate

The struct in question is:

#[derive(AsChangeset, Debug, Clone, Identifiable, Insertable, Queryable)]
#[table_name = "profiles"]
pub struct Profile {
    pub id: ruma_identifiers::UserId,
    pub avatar_url: Option<String>,
    pub displayname: Option<String>,
}

The previously working implementations for ruma_identifiers::UserId can be found here: https://github.com/ruma/ruma-identifiers/blob/29f76ca95d295bca36ec53cd8da72217c5aa0f75/src/lib.rs#L689

[jimmycuadra]

Moving my question to its own issue: #640

[sgrif]

Closing as a duplicate of #162. See #343 (comment) for some thoughts on this.

[sgrif]

Also I should expand on this a little bit since this issue is slightly different. Specifically for the case of "implementing support for a Rust type that is not crate local, for a SQL type which is crate local" (an example of this is implementing support for HashMap<String, Option<String>> for an Hstore type). Currently the only thing we implement for types in Diesel that you cannot write from outside of Diesel is impl AsExpression<Nullable<Hstore>> for HashMap. Right now the concrete affect of that is that you cannot have an Insertable impl where one of the columns is Nullable<Hstore>. That's going to get fixed (probably in 1.1). The only other place that's visible is that you can't do nullable_column.eq(map), you'll have to do nullable_column.eq(Some(map)) instead. If that last bit is the only visible difference between types in Diesel, and types outside of Diesel, I think that's acceptable.