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

insert taking a &mut Connection instead of an impl Executor #22

Closed
Punie opened this issue Jan 10, 2022 · 6 comments
Closed

insert taking a &mut Connection instead of an impl Executor #22

Punie opened this issue Jan 10, 2022 · 6 comments

Comments

@Punie
Copy link
Contributor

Punie commented Jan 10, 2022

Hello!

I started experimenting with this crate and so far, I love it!

There is one thing that I'm curious about though:
All methods on Table take a db: impl Executor<...> argument except for insert that takes a db: &mut <Db as Database>::Connection.
Is there any reason for that? I find it quite annoying to be able to simply pass my &PgPool anywhere except for insert where I need to &mut pool.acquire().await?.

@NyxCode
Copy link
Owner

NyxCode commented Jan 11, 2022

Hey! Lovely to hear that it's usefull to you!
insert currently requires a re-usable connection on MySQL. This is because MySQL doesn't support INSERT .. RETURNING ... Instead, we insert and then manually select the last inserted row on MySQL.
An impl Executor<..>, on the other hand, can only be used once.

@NyxCode
Copy link
Owner

NyxCode commented Jan 11, 2022

I guess we could have different Table::insert for different backends, though i'm not sure that would be a great idea. Will need to think about this a bit.

@Punie
Copy link
Contributor Author

Punie commented Jan 11, 2022

Oh that is interesting and it does make sense. I suppose it's only a small annoyance I can easily live with for now.
I might take some time in the near future to explore the codebase and see if I can come up with something myself in a PR 😄

Thank you for that swift answer 🙏

@Punie
Copy link
Contributor Author

Punie commented Jan 11, 2022

Wouldn't it be possible to run the MySQL version in a single query as a transaction though?

START TRANSACTION;

INSERT INTO {} ({)) VALUES ({});
SELECT LAST_INSERT_ID() AS id;

COMMIT;

@NyxCode
Copy link
Owner

NyxCode commented Sep 8, 2024

Fixed in #40. Now the MariaDB backend also uses a form of INSERT .. RETURNING ...

@NyxCode NyxCode closed this as completed Sep 8, 2024
@NyxCode
Copy link
Owner

NyxCode commented Sep 8, 2024

sqlx 0.7 sadly changed the API of Executor, so the MySQL backend still needs a &mut MySqlConnection. I did add explicit support for MariaDB though, which is fine with just a impl Executor.

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

2 participants