upsert multi

[feluelle]

Is your feature request related to a problem? Please describe.

If I currently want to upsert multiple records I would have to call upsert for each model which means a lot of overhead and unnecessary db calls. A Multi-upsert can also be done in just one query.

Describe the solution you'd like

I would like to have an upsert function that allows inserting or updating multiple models. It doesn't have to be in one query necessarily. We can also continue with the current strategy using get and create or update.

Describe alternatives you've considered

The current alternative (postgres specific) I am using is:

from sqlalchemy.dialects import postgresql

stmt = postgresql.insert(...).on_conflict_do_update(...)
await session.execute(statement=stmt, params=...)

[JakNowy]

Postgresql solution looks best, but it makes it fail if other dbms is used.

Honestly I would opt for focusing on postgresql as the most popular dbms and making it as efficient as possible (I don't quite like the get and create approach as it costs additional call). That way we could efficiently cover bulk operations of all crud types. Igor mentioned that we might redesign some parts of the library and relase a breaking 1.x.x version, so that might be a good moment to get that transition.

For now I think we should stick to the get and create/update approach for consistency. If you need that done soon, feel more than welcome to open a PR. Otherwise I might cover that but I'm having a few other issues to be finished first.

@igorbenav your thoughts?

[igorbenav]

I think what we should aim for eventually is a db "engine" approach, falling back to this get and create when engine is not specified, identified or covered, I think that would justify a breaking version. Until we do something like this, I believe creating the fallback would be the way to go

[igorbenav]

Btw there's a lot that still needs to be done before we (as in me and @JakNowy) actually get to creating the db engine approach, so if you feel like you could do it (and need it done), @feluelle, we'd be more than happy.

[feluelle]

Cool, will give it a try.

[slaarti]

Belatedly, one thing I want to note about this, because it bit me as I was trying to get an unrelated PR together, is that there is now an unspoken development dependency on Docker by way of the tests bringing in testcontainers. Tests which use containers to run will fail if you don't have Docker installed, which I don't since I've never had a need for it.

If you're okay with that, then fine, but it should at least be documented. (My preference, however, would be to somehow remove the dependency, or at least make it so it's only needed if you want to specifically run the tests that require it, separate from the main body of tests.)

[feluelle]

I didn't know we document our dev deps anywhere. We can skipIf docker is not installed or running, but for those tests I think it is worth using it. I'll take a look later today.

Imho testcontainers makes it much easier to run different dbs locally instead of having to install system libs on the host.

Btw you can also exclude those tests by adding sth. like -m "not dialect" to your pytest cmd.

[slaarti]

Well, we kind of do, by way of pyproject.toml; it's just that Docker lives outside that ecosystem. (Also, while checking on the contributing guidelines to see what else is/n't documented, I just discovered that the guidelines doc in docs is an unmodified boilerplate and doesn't match the doc in the root of the repo. That's not really relevant here, though; I'll do a PR about that.)

Anyway, I don't necessarily disagree about testcontainers vs. locally installing DB libraries. Having tests that have any reliance on databases other than SQLite is new and unexpected, though, and I super didn't enjoy having my test run suddenly light up red when I didn't change anything that should cause it. (At least I'm not going to try to insist that this should be handled by mocks/ stubs/ whatever? 😁)