Skip to content
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

Can't use &dyn ToSql with Async/Await syntax with tokio-postgres #445

Closed
kaibyao opened this issue Jun 20, 2019 · 10 comments
Closed

Can't use &dyn ToSql with Async/Await syntax with tokio-postgres #445

kaibyao opened this issue Jun 20, 2019 · 10 comments

Comments

@kaibyao
Copy link
Contributor

kaibyao commented Jun 20, 2019

See https://github.com/kaibyao/tokio-postgres-tosql-multithread-issue for a reference implementation of this problem.

// example endpoint using actix_web 1.0
async fn index() -> Result<HttpResponse, Error> {
    // ...
    let prepared_values: Vec<&dyn ToSql> = vec![&true];
    let rows = client.query(&statement, &prepared_values).collect().compat().await;
    let rows: Vec<i32> = rows.unwrap().into_iter().map(|row| row.get(0)).collect();

    Ok(HttpResponse::build(StatusCode::OK).json(rows))
}

// in another thread
fn returns_future() -> impl Future01<Item = HttpResponse, Error = Error> {
    index().boxed().compat()
    //      ^^^^^^^  `dyn tokio_postgres::types::ToSql` cannot be shared between threads safely
}

What's the best way to pass Vec<&dyn ToSql> (for prepared statements) in multi-threaded environments?

The following don't work:

  • Using &(dyn ToSql + Send) instead of &dyn ToSql
  • Creating a "wrapper" trait ToSqlSend that does have Send and ToSql and changing &dyn ToSql to &dyn ToSqlSend.
@sfackler
Copy link
Owner

Are you sure you need specifically a Vec<&dyn ToSql>? client.query(&statement, &[&true]).

@kaibyao
Copy link
Contributor Author

kaibyao commented Jun 20, 2019

Yes: it's a simplified example above, but my use case is that I will be sending variables of different types in the prepared statement. I imagine this form of usage will be common once async/await hits stable.

Will I have to fork the library and add Send to ToSql? I'm still new to rust/systems programming so I'm not sure what the ramifications for this are.

@sfackler
Copy link
Owner

But are you dynamically building the set of values? If not, you can still avoid creating the Vec entirely: client.query(&statement, &[&foo, &bar, &baz]).

@kaibyao
Copy link
Contributor Author

kaibyao commented Jun 20, 2019

Unfortunately I am. The prepared statement is built based on external input at runtime.

@sfackler
Copy link
Owner

Ah awkward. #265 might be a solution there but the ergonomics in the simple case are a bit worse last time I tried.

@sfackler
Copy link
Owner

This should work as a (gross) workaround in the meantime:

async fn index() -> Result<HttpResponse, Error> {
    // ...
    let future = {
        let prepared_values: Vec<&dyn ToSql> = vec![&true];
        client.query(&statement, &prepared_values)
    };
    let rows = future.collect().compat().await;
    let rows: Vec<i32> = rows.unwrap().into_iter().map(|row| row.get(0)).collect();

    Ok(HttpResponse::build(StatusCode::OK).json(rows))
}

As long as the Vec is never alive across an await point, it shouldn't end up affecting the Send-ness of the generated Future.

@kaibyao
Copy link
Contributor Author

kaibyao commented Jun 23, 2019

That works! Thanks for that solution and explanation. I'm going to close that PR in the meantime, as it really isn't the ideal solution.

@kaibyao kaibyao closed this as completed Jun 23, 2019
@sfackler
Copy link
Owner

#458 should improve things here.

@kaibyao
Copy link
Contributor Author

kaibyao commented Jul 10, 2019 via email

@Venryx
Copy link

Venryx commented Apr 3, 2022

I'm pretty new to Rust, and have spent the last couple hours trying to get the new query_raw function to work for my case.

I've seen the example code in the documentation for query_raw:

async fn async_main(client: &tokio_postgres::Client) -> Result<(), tokio_postgres::Error> {
    use tokio_postgres::types::ToSql;
    use futures::{pin_mut, TryStreamExt};
    
    let params: Vec<String> = vec![
        "first param".into(),
        "second param".into(),
    ];
    let mut it = client.query_raw(
        "SELECT foo FROM bar WHERE biz = $1 AND baz = $2",
        params,
    ).await?;
    
    pin_mut!(it);
    while let Some(row) = it.try_next().await? {
        let foo: i32 = row.get("foo");
        println!("foo: {}", foo);
    }
    Ok(())
}

That example works. However, it seems that it stops working as soon as the params variable is changed to a non-concrete type.

For example, I tried changing it to:

let params: Vec<&(dyn ToSql)> = vec![
    &"first param",
    &555i32,
];

The problem is that then the Future for the call-stack no longer satisfies the + Send constraint, causing it to fail in my app-server codebase (since it uses async_graphql, which requires the + Send constraint)

When I add the + Send constraint to the params vector (I also tried with + Sync added), I get the error:

the size for values of type `dyn ToSql + std::marker::Send` cannot be known at compilation time
the trait `Sized` is not implemented for `dyn ToSql + std::marker::Send`
required because of the requirements on the impl of `ToSql` for `&dyn ToSql + std::marker::Send`
  1. Is there a way to use the new query_raw function without locking the params vector to all be of the same concrete type?
  2. If not, then how should one send in a (dynamically generated) set of parameters that includes a variety of types? (eg. strings, numbers, bools, or even more advanced structs like HashMap<String, Option<String>>, which is apparently supported)

EDIT: I ended up using this, which works fine (despite not looking great): #712 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants