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

Derive Insertable for struct / Arc / Rc rather then for plain reference #1501

Closed
alleycat-at-git opened this Issue Jan 20, 2018 · 2 comments

Comments

Projects
None yet
2 participants
@alleycat-at-git

alleycat-at-git commented Jan 20, 2018

Currently there's only implementation for &'a struct when Insertable is auto-derived https://github.com/diesel-rs/diesel/blob/master/diesel/src/macros/insertable.rs#L103

This is kind of painful when working with multithreading, because multithreading doesn't understand lifetimes. E.g. I have this method:

    pub fn create(&self, payload: NewUser) -> Box<Future<Item=User, Error=Error> {
        let conn = self.get_connection();

        Box::new(
            self.cpu_pool.spawn_fn(move || {
                let query = diesel::insert_into(users).values(&payload);
                query.get_result(&*conn).map_err(|e| Error::from(e))
            })
        )
    }

I want to generalize all the async stuff into one method to reduce boilerplate. This function works fine for 'static queries:

    fn execute_query<T: Send + 'static, U: LoadQuery<PgConnection, T> + Send + 'static>(&self, query: U) -> RepoFuture<T> {
        let conn = match self.r2d2_pool.get() {
            Ok(connection) => connection,
            Err(_) => return Box::new(future::err(Error::Connection("Cannot connect to users db".to_string())))
        };

        Box::new(
            self.cpu_pool.spawn_fn(move || {
                query.get_result::<T>(&*conn).map_err(|e| Error::from(e))
            })
        )
    }

But this thing let query = diesel::insert_into(users).values(&payload); isn't static, but has 'a lifetime that is equal to payload. To avoid this, I tried to implement smth like this:

    fn execute_query_fn<U, Q, F, P>(&self, payload: P, query_fn: F) -> RepoFuture<U>
        where
            U: Send + 'static,
            Q: LoadQuery<PgConnection, U> ,
            F: for<'a> Fn(&'a P) -> Q<'a> + Send + 'static,
            P: Send + 'static
    {
        let conn = match self.r2d2_pool.get() {
            Ok(connection) => connection,
            Err(_) => return Box::new(future::err(Error::Connection("Cannot connect to users db".to_string())))
        };

        Box::new(
            self.cpu_pool.spawn_fn(move || {
                let pl = payload;
                let query = query_fn(&pl);
                query.get_result::<U>(&*conn).map_err(|e| Error::from(e))
            })
        )
    }

But this fails due to rust compile limitations F: for<'a> Fn(&'a P) -> Q<'a> + Send + 'static, i.e. it doesn't allow to specify lifetime for generic parameter. The corresponding Rust RFC is in progress https://github.com/rust-lang/rfcs/blob/master/text/1598-generic_associated_types.md.

All this pain comes from the fact that I have to use reference to payload here let query = diesel::insert_into(users).values(&payload);. Because only reference auto-derived for Insertable. If it was smth like struct or Arc everything would work ok.

Does it make sense to implement these?

@sgrif

This comment has been minimized.

Member

sgrif commented Jan 23, 2018

I've been meaning to generate an impl on the struct as well as a reference to the struct. Would that sufficiently handle your use case?

@alleycat-at-git

This comment has been minimized.

alleycat-at-git commented Feb 12, 2018

@sgrif Yeah, that would solve the issue

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