SqlxCrud::by_id() fails in an asyncronous context.

[TTWNO]

Hi there,

First off, amazing crate! Thank you for all your hard work.

I don't have my example handy on me (I'll upload ir this evening), but the future send from by_id is not send.

This causws problems in Axum when using your crare to retireve db results inside Axum handler functions. You may simply be able to implement Unpin for the future/Box/Pin, but I'm not entirely sure. It's also hard to test since most of your examples are in asynchronous runtime. I can make a PR that runs some tests within a tokio_text::block_on(async { ... })

Or is this simply something you don't support?

[treydempsey]

Thanks for the kind words. I've been dragging my feet on finishing out the feature set, perhaps this will motivate me to finish it.

I'd be happy to support that sort of use case, especially if it means that Axum will work. When I wrote this it was with actix-web in mind and in that case it worked fine.

If you can send some test cases that exhibit the behavior please do. That might make it clearer what the issue is, although I think I understand.

Trey Dempsey 817-521-8146 @.***

On Sat, Mar 25, 2023 at 4:02 PM Tait Hoyem @.***> wrote:

Hi there,

First off, amazing crate! Thank you for all your hard work.

I don't have my example handy on me (I'll upload ir this evening), but the future send from by_id is not send.

This causws problems in Axum when using your crare to retireve db results inside Axum handler functions. You may simply be able to implement Unpin for the future/Box/Pin, but I'm not entirely sure. It's also hard to test since most of your examples are in asynchronous runtime. I can make a PR that runs some tests within a tokio_text::block_on(async { ... })

Or is this simply something you don't support?

— Reply to this email directly, view it on GitHub https://github.com/treydempsey/sqlx-crud/issues/9, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAX3FHSE7LZNLS3CXKLT2LW55MNXANCNFSM6AAAAAAWHXXWQQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[treydempsey]

Okay I reproduced the issue, Futures returned by Axum Handlers must implement Send.

The futures, CrudFut, returned by sqlx-crud for most methods don't implement Send. Adding a bound on Send

type CrudFut<'e, T> = Pin<Box<dyn Future<Output = Result<T, sqlx::Error>> + Send + 'e>>;

causes a lifetime issue where the returned future outlives the borrow on self (T). This is because create the query by binding the values in self for queries like create or update.

So how can we fix this? Should I make create and update move self instead of borrow it?

Trey

On Sun, Apr 2, 2023 at 11:31 PM Trey Dempsey @.***> wrote:

Thanks for the kind words. I've been dragging my feet on finishing out the feature set, perhaps this will motivate me to finish it.

I'd be happy to support that sort of use case, especially if it means that Axum will work. When I wrote this it was with actix-web in mind and in that case it worked fine.

If you can send some test cases that exhibit the behavior please do. That might make it clearer what the issue is, although I think I understand.

Trey Dempsey 817-521-8146 @.***

On Sat, Mar 25, 2023 at 4:02 PM Tait Hoyem @.***> wrote:

Hi there,

First off, amazing crate! Thank you for all your hard work.

I don't have my example handy on me (I'll upload ir this evening), but the future send from by_id is not send.

This causws problems in Axum when using your crare to retireve db results inside Axum handler functions. You may simply be able to implement Unpin for the future/Box/Pin, but I'm not entirely sure. It's also hard to test since most of your examples are in asynchronous runtime. I can make a PR that runs some tests within a tokio_text::block_on(async { ... })

Or is this simply something you don't support?

— Reply to this email directly, view it on GitHub https://github.com/treydempsey/sqlx-crud/issues/9, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAX3FHSE7LZNLS3CXKLT2LW55MNXANCNFSM6AAAAAAWHXXWQQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[treydempsey]

I figured out a solution. I used cargo expand to view the code generated when running query_as!(). It creates a separate owned query object that is borrowed by the fetch future which is tied to its lifetime. I might be stuck with copying arguments in to the query object so it owns them instead of self, but i think i'm okay with that. After all if you were to use query_as!() instead it would do the same thing. Either way now that fetch doesn't borrow self, the lifetime issue is solved. I was also able to add the bound of Send to the future returned by it so it works with Axum.

I just implemented by_id so far and it's a bit rough but it works.

See the axum-support branch for code.

Trey

On Mon, Apr 3, 2023 at 5:20 AM Trey Dempsey @.***> wrote:

Okay I reproduced the issue, Futures returned by Axum Handlers must implement Send.

The futures, CrudFut, returned by sqlx-crud for most methods don't implement Send. Adding a bound on Send

type CrudFut<'e, T> = Pin<Box<dyn Future<Output = Result<T, sqlx::Error>> + Send + 'e>>;

causes a lifetime issue where the returned future outlives the borrow on self (T). This is because create the query by binding the values in self for queries like create or update.

So how can we fix this? Should I make create and update move self instead of borrow it?

Trey

On Sun, Apr 2, 2023 at 11:31 PM Trey Dempsey @.***> wrote:

Thanks for the kind words. I've been dragging my feet on finishing out the feature set, perhaps this will motivate me to finish it.

I'd be happy to support that sort of use case, especially if it means that Axum will work. When I wrote this it was with actix-web in mind and in that case it worked fine.

If you can send some test cases that exhibit the behavior please do. That might make it clearer what the issue is, although I think I understand.

Trey Dempsey 817-521-8146 @.***

On Sat, Mar 25, 2023 at 4:02 PM Tait Hoyem @.***> wrote:

Hi there,

First off, amazing crate! Thank you for all your hard work.

I don't have my example handy on me (I'll upload ir this evening), but the future send from by_id is not send.

This causws problems in Axum when using your crare to retireve db results inside Axum handler functions. You may simply be able to implement Unpin for the future/Box/Pin, but I'm not entirely sure. It's also hard to test since most of your examples are in asynchronous runtime. I can make a PR that runs some tests within a tokio_text::block_on(async { ... })

Or is this simply something you don't support?

— Reply to this email directly, view it on GitHub https://github.com/treydempsey/sqlx-crud/issues/9, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAX3FHSE7LZNLS3CXKLT2LW55MNXANCNFSM6AAAAAAWHXXWQQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[TTWNO]

Oh I just realixed I never responded to your other comments.

Great that you solved the issue! Would uou like me to finish it for the other functions or would you rather do it yourself?

[treydempsey]

I believe it's done for all of the functions. Check out the axum-support branch and tell me what you think.

[treydempsey]

I'm also working up some examples, using my existing actix-web project and the axum project i wrote for testing this work as a starting point.

[crat0z]

Hello! I agree this is an awesome crate. I ran into a similar issue, but in my case I'm writing a Discord bot with serenity. The axum-support branch fixed by_id, however update and delete don't work. Here is a minimal main.rs:

use serenity::async_trait;
use serenity::prelude::*;
use sqlx::{FromRow, Pool, Sqlite, SqlitePool};
use sqlx_crud::SqlxCrud;

struct Handler {
    db: Pool<Sqlite>,
}

#[derive(FromRow, SqlxCrud)]
struct User {
    id: i64,
    name: String,
}

#[async_trait]
impl EventHandler for Handler {
    async fn message(&self, _ctx: Context, _msg: serenity::model::channel::Message) {
        use sqlx_crud::Crud;

        // works
        let user = User {
            id: 1,
            name: "test".to_owned(),
        }
        .create(&self.db)
        .await
        .unwrap();

        // works
        let user = User::by_id(&self.db, 1).await.unwrap().unwrap();

        // doesn't work
        // let user = user.update(&self.db).await.unwrap();

        // doesn't work
        // let user = user.delete(&self.db).await.unwrap();
    }
}

#[tokio::main]
async fn main() {
    let db = SqlitePool::connect("sqlite:sqlite.db").await.unwrap();

    let _ = Handler { db };
}

And associated Cargo.toml dependencies:

tokio = { version = "1", features = ["macros"]}
serenity = "0.11"
sqlx = { version = "*", features = ["runtime-tokio-rustls", "sqlite"] }
sqlx-crud = { version = "*", features = ["runtime-tokio-rustls"], git = "https://github.com/treydempsey/sqlx-crud", branch = "axum-support" }

Thanks for the help!

[treydempsey]

Thanks for the feedback and testing. Im not sure why update or delete aren't working. Its probably an ownership issue where the sqlx "args" is borrowing self. I thought i fixed that by having it consume self. Ill take a look and see what I can do. Additional feedback or testing is appreciated.

On Sat, Apr 15, 2023, 5:16 AM crat0z @.***> wrote:

Hello! I agree this is an awesome crate. I ran into a similar issue, but in my case I'm writing a Discord bot with serenity. The axum-support branch fixed by_id, however update and delete don't work. Here is a minimal main.rs:

use serenity::async_trait;use serenity::prelude::*;use sqlx::{FromRow, Pool, Sqlite, SqlitePool};use sqlx_crud::SqlxCrud; struct Handler { db: Pool,}

[derive(FromRow, SqlxCrud)]struct User {

id: i64,
name: String,}

[async_trait]impl EventHandler for Handler {

async fn message(&self, _ctx: Context, _msg: serenity::model::channel::Message) {
    use sqlx_crud::Crud;

    // works
    let user = User {
        id: 1,
        name: "test".to_owned(),
    }
    .create(&self.db)
    .await
    .unwrap();

    // works
    let user = User::by_id(&self.db, 1).await.unwrap().unwrap();

    // doesn't work
    // let user = user.update(&self.db).await.unwrap();

    // doesn't work
    // let user = user.delete(&self.db).await.unwrap();
}}

[tokio::main]async fn main() {

let db = SqlitePool::connect("sqlite:sqlite.db").await.unwrap();

let _ = Handler { db };}

And associated Cargo.toml dependencies:

tokio = { version = "1", features = ["macros"]}serenity = "0.11"sqlx = { version = "", features = ["runtime-tokio-rustls", "sqlite"] }sqlx-crud = { version = "", features = ["runtime-tokio-rustls"], git = "https://github.com/treydempsey/sqlx-crud", branch = "axum-support" }

Thanks for the help!

— Reply to this email directly, view it on GitHub https://github.com/treydempsey/sqlx-crud/issues/9#issuecomment-1509718232, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAX3FDGVDP2WY3FXJ4PZKLXBJYO5ANCNFSM6AAAAAAWHXXWQQ . You are receiving this because you were assigned.Message ID: @.***>

[treydempsey]

Okay I believe I see the issue. I didnt change the CrudFut type to include Send and update is still using the type alias. I expanded the type for by_id and added Send there. I just need to roll Send into CrudFut. And i need to double check the proc macro generates an owned sqlx args instance. Im away from a computer now but ill take a look this evening (US).

Cheers

On Sat, Apr 15, 2023, 10:51 AM Trey Dempsey @.***> wrote:

Thanks for the feedback and testing. Im not sure why update or delete aren't working. Its probably an ownership issue where the sqlx "args" is borrowing self. I thought i fixed that by having it consume self. Ill take a look and see what I can do. Additional feedback or testing is appreciated.

On Sat, Apr 15, 2023, 5:16 AM crat0z @.***> wrote:

Hello! I agree this is an awesome crate. I ran into a similar issue, but in my case I'm writing a Discord bot with serenity. The axum-support branch fixed by_id, however update and delete don't work. Here is a minimal main.rs:

use serenity::async_trait;use serenity::prelude::*;use sqlx::{FromRow, Pool, Sqlite, SqlitePool};use sqlx_crud::SqlxCrud; struct Handler { db: Pool,}

[derive(FromRow, SqlxCrud)]struct User {

id: i64,
name: String,}

[async_trait]impl EventHandler for Handler {

async fn message(&self, _ctx: Context, _msg: serenity::model::channel::Message) {
    use sqlx_crud::Crud;

    // works
    let user = User {
        id: 1,
        name: "test".to_owned(),
    }
    .create(&self.db)
    .await
    .unwrap();

    // works
    let user = User::by_id(&self.db, 1).await.unwrap().unwrap();

    // doesn't work
    // let user = user.update(&self.db).await.unwrap();

    // doesn't work
    // let user = user.delete(&self.db).await.unwrap();
}}

[tokio::main]async fn main() {

let db = SqlitePool::connect("sqlite:sqlite.db").await.unwrap();

let _ = Handler { db };}

And associated Cargo.toml dependencies:

tokio = { version = "1", features = ["macros"]}serenity = "0.11"sqlx = { version = "", features = ["runtime-tokio-rustls", "sqlite"] }sqlx-crud = { version = "", features = ["runtime-tokio-rustls"], git = "https://github.com/treydempsey/sqlx-crud", branch = "axum-support" }

Thanks for the help!

— Reply to this email directly, view it on GitHub https://github.com/treydempsey/sqlx-crud/issues/9#issuecomment-1509718232, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAX3FDGVDP2WY3FXJ4PZKLXBJYO5ANCNFSM6AAAAAAWHXXWQQ . You are receiving this because you were assigned.Message ID: @.***>

[crat0z]

Yep, delete just needed Send. update currently takes &'e self, however changing it to self gives an error about update_binds I think, and 'e must outlive 'static blah blah. I tried fiddling with the code, but wasn't able to make much progress besides that.

[treydempsey]

Okay I pushed a branch that fixes update and delete. I also included test code for axum under examples/axum. If this looks good I'll bump the version and merge.

[crat0z]

Works wonderfully for me. Thanks a lot!