New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allowing a type that's Queryable and Insertable to autoincrement its ID #1440

Closed
brandur opened this Issue Dec 29, 2017 · 5 comments

Comments

Projects
None yet
4 participants
@brandur
Contributor

brandur commented Dec 29, 2017

Setup

Versions

  • Rust: 1.22.1
  • Diesel: 1.0.0-rc1
  • Database: Postgres
  • Operating System Mac OS

Problem Description

I'm running into a problem whereby I'm not sure how to make a type both Queryable and Insertable while still having it autoincrement its primary key on inserts.

I have a Postgres table with a BIGSERIAL type like so:

CREATE TABLE episodes (
    id BIGSERIAL PRIMARY KEY,
    description TEXT,
...

In my generated schema, this translates to something like this as you'd expect:

table! {
    episodes (id) {
        id -> Int8,
        description -> Nullable<Text>,
...

And finally, I have a model in my code that I've marked as Insertable and Queryable:

#[derive(Insertable, Queryable)]
#[table_name = "episodes"]
pub struct Episode {
    pub id:           i64,
    pub description:  Option<String>,
...

I'm trying to figure out how I'd use this setup to insert a new record while delegating ID generation to the database. The SQL for this type of operation looks roughly like this:

INSERT INTO episodes (description, ...) 
    VALUES ('foo', ...);

Or like this:

INSERT INTO episodes (id, description, ...) 
    VALUES (DEFAULT, 'foo', ...);

If my id field is a simple i64, there's no valid value that will make the database autoincrement — 0, -1 etc. are all insertable BIGINTs and are inserted literally. As far as I know, only omitting the field or using the special DEFAULT value will do.

I tried changing my id to an Option<i64>:

pub struct Episode {
    pub id:           Option<i64>, // <-- changed from i64
    pub description:  Option<String>,
...

But then constraints on Queryable rightly kick in and tell me I'm not allowed to do this:

error[E0277]: the trait bound `std::option::Option<i64>: diesel::Queryable<diesel::types::BigInt, diesel::pg::Pg>` is not satisfied
   --> src/graphql.rs:155:14
    |
155 |             .load::<model::Episode>(&*context.get_conn()?)
    |              ^^^^ the trait `diesel::Queryable<diesel::types::BigInt, diesel::pg::Pg>` is not implemented for `std::option::Option<i64>`
    |
    = help: the following implementations were found:
              <std::option::Option<T> as diesel::Queryable<diesel::types::Nullable<ST>, DB>>
    = note: required because of the requirements on the impl of `diesel::Queryable<(diesel::types::BigInt, diesel::types::Nullable<diesel::types::Text>, diesel::types::Nullable<diesel::types::Bool>, diesel::types::Text, diesel::types::Nullable<diesel::types::Text>, diesel::types::Nullable<diesel::types::Text>, diesel::types::Text, diesel::types::BigInt, diesel::types::Timestamptz, diesel::types::Text), diesel::pg::Pg>` for `(std::option::Option<i64>, std::option::Option<std::string::String>, std::option::Option<bool>, std::string::String, std::option::Option<std::string::String>, std::option::Option<std::string::String>, std::string::String, i64, chrono::DateTime<chrono::Utc>, std::string::String)`
    = note: required because of the requirements on the impl of `diesel::Queryable<(diesel::types::BigInt, diesel::types::Nullable<diesel::types::Text>, diesel::types::Nullable<diesel::types::Bool>, diesel::types::Text, diesel::types::Nullable<diesel::types::Text>, diesel::types::Nullable<diesel::types::Text>, diesel::types::Text, diesel::types::BigInt, diesel::types::Timestamptz, diesel::types::Text), diesel::pg::Pg>` for `model::Episode`
    = note: required because of the requirements on the impl of `diesel::query_dsl::LoadQuery<_, model::Episode>` for `diesel::query_builder::SelectStatement<schema::episodes::table, diesel::query_builder::select_clause::DefaultSelectClause, diesel::query_builder::distinct_clause::NoDistinctClause, diesel::query_builder::where_clause::WhereClause<diesel::expression::operators::Eq<schema::episodes::columns::podcast_id, diesel::expression::bound::Bound<diesel::types::BigInt, i64>>>, diesel::query_builder::order_clause::OrderClause<diesel::dsl::Desc<schema::episodes::columns::published_at>>, diesel::query_builder::limit_clause::LimitClause<diesel::expression::bound::Bound<diesel::types::BigInt, i64>>>`

Any advice? Thanks!

(This looks a little like #640, but I don't think it's the same.)

@brandur

This comment has been minimized.

Contributor

brandur commented Dec 29, 2017

Ah, I just found this section on Insertable in a draft guide.

When implementing Insertable, you probably won't be setting the auto-incremented id field of the row. Usually you will also ignore fields such as created_at and updated_at. For this reason, it's not advisable to use Queryable and Insertable on the same struct due to the field number constraints of Queryable. Create another struct that you may use for database insertions that will have all the fields you would like to set. This section will not cover nullable fields (we'll cover that in AsChangeset), so we will assume every field must have data in our example. When making a separate struct for database inserts, Diesel needs to know the corresponding table name, so the struct must also be annotated with the #[table_name="some_table_name"] attribute. If your new struct has different field names, each of them may be annotated with #[column_name(some_column_name)].

I'll do this for now, but this is a little suboptimal given the amount of overlap you're going to have between the insertable and queryable versions for most models — models tend to balloon to lots and lots of fields given enough time, and all of them will need to be written twice (although I suppose the approach has some advantages as well).

@sgrif

This comment has been minimized.

Member

sgrif commented Dec 30, 2017

given the amount of overlap you're going to have between the insertable and queryable versions for most models

This is what I like to call "incidental duplication" -- which is the term I use to refer to things that happen to be structurally similar right now, but change in the future for different reasons. The approach we've taken is very specifically designed to separate things that represent the results of queries from things that represent input from external sources. The desire to keep these separate comes from experience with hundreds of applications. You can see similar approaches in other modern frameworks like Phoenix.

If you really want to share these structs, Queryable composes with other structs that are Queryable. You could have a struct like this:

struct UserWithId {
    id: i32,
    user: User,
}

It sounds like you've found the answers you're looking for, so I'm going to close this issue.

@sgrif sgrif closed this Dec 30, 2017

@anilanar

This comment has been minimized.

anilanar commented Apr 9, 2018

@sgrif What do you mean by:

If you really want to share these structs, Queryable composes with other structs that are Queryable.

When I have the following:

#[derive(Queryable)]
struct UserWithId {
    id: i32,
    user: User,
}

#[derive(Queryable)]
struct User {
    age: i32,
}

users::table.load::<UserWithId>(&conn)?; emits an error:

error[E0277]: the trait bound `(i32, User): diesel::Queryable<(diesel::sql_types::Integer, diesel::sql_types::Integer), _>` is not satisfied
@weiznich

This comment has been minimized.

Contributor

weiznich commented Apr 9, 2018

@anilanar You need to add an explicit select clause to your query to transform the return type into the right one.

users::table.select((users::id, (users::age,))).load::<UserWithId>(&conn);

Should work. A other solution would be to manually implement Queryable

@anilanar

This comment has been minimized.

anilanar commented Apr 9, 2018

It does work, but then it doesn't generate BelongsToDsl implementation for XWithId struct when foreign key field is defined in X struct. You can work around it by defining it in both XWithId and X and then do .select((x::id, x::user_id, (x::user_id, x::others)). Duplication again. Far from ideal 😕.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment