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

Re-add support for bulk_create #924

Closed
dehnert opened this issue Oct 19, 2022 · 3 comments · Fixed by #972 · May be fixed by stianjensen/django-reversion#1
Closed

Re-add support for bulk_create #924

dehnert opened this issue Oct 19, 2022 · 3 comments · Fixed by #972 · May be fixed by stianjensen/django-reversion#1

Comments

@dehnert
Copy link

dehnert commented Oct 19, 2022

Between 1.8.0 and 1.8.6 (#380), reversion used bulk_create when creating versions, which presumably dramatically improved performance when creating a revision with many changed model instances[1]. It sounds like the motivation for removing this was inability to get back object primary keys from bulk_create, but that has since been fixed on some popular databases -- Django 1.10 (released about a year and a half after #380) added some support (only PostgreSQL at the time). SQLite was added in Django 4.0, and MariaDB was added sometime in the middle. It looks like there's a convenient feature test for this (connection.features.can_return_rows_from_bulk_insert), so it would be nice if django-reversion could use bulk_create when feasible.

(I found #380 while trying to figure out why tons of queries were being used despite the changelog claiming reversion uses bulk_create, so if you don't re-add bulk_create support it might be nice to note the change somewhere?)

[1] I haven't benchmarked this specifically, but a script that creates several thousand objects went from 30 minute runtime to ~3 minutes by doing far more query batching, so I suspect this is significant.

@spapas
Copy link

spapas commented Oct 21, 2022

Hello friends, this would be really useful to me also. I've got some objects that I create through bulk_insert or update with bulk_update and I am forced to save them one-by-one to have proper revisions.

@etianen
Copy link
Owner

etianen commented Oct 29, 2022

I'm a bit short on time at the moment, but I'd be happy to take a PR that implemented this.

stianjensen added a commit to stianjensen/django-reversion that referenced this issue Jul 18, 2024
stianjensen added a commit to stianjensen/django-reversion that referenced this issue Jul 18, 2024
stianjensen added a commit to stianjensen/django-reversion that referenced this issue Jul 18, 2024
stianjensen added a commit to stianjensen/django-reversion that referenced this issue Jul 19, 2024
@stianjensen
Copy link
Contributor

Hello friends, this would be really useful to me also. I've got some objects that I create through bulk_insert or update with bulk_update and I am forced to save them one-by-one to have proper revisions.

So this, specifically, is another problem than what's described in this issue! A workaround for your problem is explained here:
#925

Essentially, if you use bulk_create or bulk_update to update a lot of models in one query, also iterate over the objects and call add_to_revision on them (that does not by itself cause one immediate db query per model). Then, until this issue gets fixed, that will cause n queries upon save to create the n versions, but hopefully in the future, with a fix for this issue, that saving of the n versions will also itself be performed via only one bulk query (plus one extra query to save the Revision model instance).

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