NyxCode / ormx

bringing orm-like features to sqlx
MIT License
287 stars 32 forks source link

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

Closed Punie closed 5 days ago

Punie commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 5 days ago

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

NyxCode commented 5 days ago

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.