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

Fix #298 -- create m2m when using _bulk_create=True #354

Merged
merged 6 commits into from Oct 24, 2022

Conversation

amureki
Copy link
Collaborator

@amureki amureki commented Oct 15, 2022

Describe the change
Extending logic of _save_related_objs that was added in #206.
This is optimized because of bulk_create use on through model, but it is still quite heavy on queries.

I had to switch to Ubuntu 22 in CI to upgrade default SQLite version which supports returning pks in bulk create operations.

PR Checklist

@amureki amureki self-assigned this Oct 15, 2022
@amureki amureki force-pushed the issues/298/bulk-create-for-m2m branch 2 times, most recently from 540ad16 to e0b7632 Compare October 17, 2022 11:26
@amureki amureki force-pushed the issues/298/bulk-create-for-m2m branch from e0b7632 to 095ace0 Compare October 17, 2022 11:26
@amureki amureki requested a review from a team October 17, 2022 11:32
@amureki
Copy link
Collaborator Author

amureki commented Oct 17, 2022

@model-bakers/core would love to get your opinion here. I solved the original issue, but it adds extra queries to resolve M2M relations (which I expected). However, due to QuerySet.bulk_create not returning pks in old SQLite versions (and Django < 4.0), there is an extra step that adds even more queries (however, I limited it to not affect Django 4.0+ users).

The other option would be to explicitly say that we are not supporting M2M properly in QuerySet.bulk_create, which is also not desirable...

model_bakery/baker.py Outdated Show resolved Hide resolved
Co-authored-by: Tim Klein <timjklein36@users.noreply.github.com>
@amureki amureki merged commit ef3cc50 into main Oct 24, 2022
@amureki amureki deleted the issues/298/bulk-create-for-m2m branch October 24, 2022 06:07
amureki added a commit that referenced this pull request Oct 24, 2022
- Fixed a bug with `seq` being passed a tz-aware start value [PR #353](#353)
- Create m2m when using `_bulk_create=True` [PR #354](#354)
- [dev] Use official postgis docker image in CI [PR #355](#355)
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

Successfully merging this pull request may close these issues.

None yet

2 participants