adwhit/diesel-derive-enum

QueryId error?

Closed this issue ยท 9 comments

New to diesel, so perhaps this is an error by me, and the examples could be clarified to safeguard from me's.

schema.rs: (via diesel print-schema + diesel-derive modifications)

#[derive(PgEnum, Debug, PartialEq)]
#[DieselType = "Foo_status"]
#[warn(non_camel_case_types)]
pub enum FooStatus {
    Active,
    Inactive,
}

table! {
    use diesel::sql_types::{Int4};
    use super::Foo_status;
    foo (foo_id) {
        foo_id -> Int4,
        status -> Foo_status,
    }
}

Sample app:

#[derive(Queryable)]
pub struct Foo {
    pub foo_id: i32,
    pub status: FooStatus,
}
    let results = foo
        .filter(status.eq(FooStatus::Active))
        .limit(5)
        .load::<Foo>(&connection)
        .expect("Error loading posts");

Error:

24 |         .load::<Foo>(&connection)
   |          ^^^^ the trait `diesel::query_builder::QueryId` is not implemented for `diesel_demo::schema::Foo`

And if I add QueryId to FooStatus I get:

1 | #[derive(PgEnum, Debug, PartialEq, QueryId)]
  |                                    ^^^^^^^
  |
  = help: message: #[derive(QueryId)] cannot be used with enums

Yep, that's a problem with the crate, not your code. The fix is quite simple, I will get it merged after the sqlite stuff.

@adwhit Weirdly I'm getting this error again on 2.0.0 (and wasn't getting it on 2.0.0-rc0). Could it be a regression?

@adwhit Weirdly I'm getting this error again on 2.0.0 (and wasn't getting it on 2.0.0-rc0). Could it be a regression?

I have the same issue atm, and it seems to be that diesel-derive-enum no longer impls QueryId due to the official diesel CLI print-schema command doing it in the next release. However, even though diesel-rs/diesel#3308 is merged, it is not released yet in any release, so atm noone is deriving it.
I guess this should be fixed once diesel_cli 2.0.2 is released. In the meantime you can impl/derive it yourself on the generated types.

@dkales Makes absolute sense, thanks for the explanation! @weiznich Is there a tracking issue for the 2.0.2 release of CLI we could link here or something to help you with?

To be clear: The linked PR implements a new feature. New features are never released as patch release, but only as a minor release. It will be included in the next minor diesel release, but that's not ready yet due to other features being not finished yet. You can help finishing this release faster by contributing to diesel for example through code review or by picking one of the help wanted issues (to be clear not all of that is planed for the next feature release).

Generally speaking: As diesel is a free time project for me I do not give any ETA's for releases, other than: As soon as someone asks the release moves back another month. There is also no tracking issue for releases because of that reason.

I'm a bit confused by this regression since I added some tests specifically to make sure this works. Will look into it.

Seems that the QueryId trait is only required by certain query operations (e.g. filter) that was not covered by the tests ๐Ÿ˜”

Probably the best thing to do is re-institute the QueryId derive, then remove it again when diesel-cli 2.1.0 is released.

@adwhit Thanks, this conclusion makes sense to me, although I'd be willing to wait for diesel-cli release as well. Either way, thanks a lot!

@weiznich Got it, thanks a lot for the clear response. I understand your position on this, sorry for bothering.