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

Updating M2M relations causes unnecessary DB queries #1317

Closed
alimony opened this issue Mar 14, 2024 · 11 comments · Fixed by #1333
Closed

Updating M2M relations causes unnecessary DB queries #1317

alimony opened this issue Mar 14, 2024 · 11 comments · Fixed by #1333
Assignees
Labels
accepted Issue accepted for completion bug Issues related to confirmed bugs

Comments

@alimony
Copy link

alimony commented Mar 14, 2024

If I have the models e.g. User and Group where Group has an m2m field to User and also has a field like history = HistoricalRecords(m2m_fields=[users]), calling any of the m2m set update methods such as group.users.set(users) or group.users.add(user1, user2) causes duplicate queries.

I have an example of this where I add 25 users to a group, and it generates 25 extra user selects and 25 extra group selects.

Is there any way to work around this? Since I do this right after creating a new Group, I tried bypassing history saving on the Group model as suggested by the documentation, but related sets seem unaffected by this.

@ddabble
Copy link
Member

ddabble commented Mar 15, 2024

I'm not sure I'm able to reproduce your issue 🤔 Calling e.g. add() with multiple arguments should only create one historical record (see e.g. this test case)... Could you provide some (complete) test case code that reproduces your issue? 🙂

@alimony
Copy link
Author

alimony commented Mar 15, 2024

This is not about duplicate records, but duplicate database queries, as identified by e.g. Django Debug Toolbar. It’s a performance issue. Can you also see duplicate queries by inspecting with debug toolbar? If not, I’ll try and make a test case.

@ddabble
Copy link
Member

ddabble commented Mar 15, 2024

Ah, sorry, I misread your description 😅 I'm able to reproduce it, and will open a PR for it :)

@ddabble ddabble self-assigned this Mar 15, 2024
@ddabble ddabble added bug Issues related to confirmed bugs accepted Issue accepted for completion labels Mar 15, 2024
@alimony
Copy link
Author

alimony commented Mar 16, 2024

Thank you, lovely to hear! Let me know if I can help in any way.

@alimony
Copy link
Author

alimony commented Mar 20, 2024

Any update on this?

@ddabble
Copy link
Member

ddabble commented May 3, 2024

I've made a fix for it locally, but considering that @tim-schilling and I are the only currently active maintainers, I want to focus on finishing #1128 first, which seems close to being merged 🙂

@tim-schilling
Copy link
Contributor

@alimony if this is an urgent thing for you, please write the test case and attempt a fix. Anything that gets things started will help tremendously.

@ddabble ddabble changed the title Updating an m2m set on a model with history causes a lot of duplicate queries Updating M2M relations causes unnecessary DB queries May 14, 2024
@alimony
Copy link
Author

alimony commented May 15, 2024

Thanks a lot @ddabble !

@alimony
Copy link
Author

alimony commented May 15, 2024

@ddabble Any idea when we might see this in a release?

@ddabble
Copy link
Member

ddabble commented May 15, 2024

I'm planning on creating a new release after #1344 has been merged 🙂

@ddabble
Copy link
Member

ddabble commented May 26, 2024

@alimony I've created a GitHub release for version 3.6.0, but it's yet to be published to PyPI; see progress in #1345

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Issue accepted for completion bug Issues related to confirmed bugs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants