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

3.15 regression: UpdateModelMixin breaks views using Manager objects as queryset #9319

Closed
peterthomassen opened this issue Mar 21, 2024 · 4 comments · Fixed by #9327
Closed
Labels

Comments

@peterthomassen
Copy link
Contributor

peterthomassen commented Mar 21, 2024

  • Since Re-prefetch related objects after updating #8043, UpdateModelMixin.update() accesses the queryset's _prefetch_related_lookups attribute.
  • Manager objects don't have this attribute.
  • This causes an exception when the queryset is, e.g., a RelatedManager:
     def get_queryset(self):
         return self.user.somerelated_set

Notes:

  • get_queryset() is not expected to return a QuerySet. This is explicitly stated in get_queryset()'s docstring.
  • In fact, that docstring implies that there is no expectation that there is a _prefetch_related_lookups attribute.
  • My particular instance of the problem could be fixed on the application side by adding .all(). However, that would cause early evaluation of the queryset before filtering. That's not an option; SQL queries should run only once all WHERE conditions have been applied.

This is a regression that simply breaks stuff that has been working with the stable API. The change should be reverted.

(This is similar to #9306, but not the same.)

@browniebroke
Copy link
Contributor

I think that explains that error which was reported in a discussion #9304

I was wondering what use case would trigger it, that makes sense now.

@browniebroke
Copy link
Contributor

I think that might be covered in my fix #9314

Do you have a minimal example that I could use in the tests?

@tomchristie
Copy link
Member

@browniebroke Okay thanks, so would a sensible course of action be to start by reverting #8043 for a 3.15.1, so we can later consider re-issuing a non-breaking fix without the additional time-pressure?

@browniebroke
Copy link
Contributor

browniebroke commented Mar 21, 2024

Sounds sensible yes 👍🏻 I don't even understand what #8043 was trying to achieve in the first place...

EDIT seems like this is a good explanation: #8043 (comment)

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

Successfully merging a pull request may close this issue.

3 participants