4 ms·

This is very useful! I'm gonna start integrating it with my projects. However having a way to allow/not allow n+1 queries (like a context manager) would be much

This is very useful! I'm gonna start integrating it with my projects. However having a way to allow/not allow n+1 queries (like a context manager) would be much better. The thing is that there are times where n+1 isn't a big problem and fixing it would be a form of premature optimisation. I'd prefer to be in control and decide if I care about the n+1 query situation or not for some specific view.

Suor 4y ago
Should be easy enough to implement. You only need a context manager that adds 1 to some threadlocal flag on enter and subrracts on exit then check this flag in the monkey patch. Not sure how costly that will be though.
- Suor 4y ago
  Here is an example of such thing https://github.com/Suor/django-cacheops/blob/8b3a79de29b2545179dcb591bf5d136c07ead4a2/cacheops/invalidation.py#L84-L101 https://github.com/Suor/django-cacheops/blob/8b3a79de29b2545...
selcuka 4y ago
It's not ideal, but you can simply log `DeferredAttribute.__get__` calls instead of raising an exception.
tantalor 4y ago
Just keep a list of exemptions in a file
squeaky-clean 4y ago
You could probably implement in in a context manager, have the `__enter__()` method execute the `_DA_get_original, DeferredAttribute.__get__ = DeferredAttribute.__get__, _DeferredAttribute_get` code and have the `__exit__` method undo that re-assignment. (Or maybe the reverse is better. Ban N+1 by default and the context manager `__enter__` puts back the original assignment, `__exit__` brings back the banned version).
brody_hamer 4y ago
I agree that in most cases this will amount to premature optimisation. Most django sites will forever remain small, and there's a lot of benefit to a simple, intuitive (if inefficient) ORM. Namely; that's it's easy for a jr developer to get in, and get results. (That's why we're coding in python, yes?) Naturally yes, there's a certain scale where N+1 starts to really matter. And in those situations it might be helpful to be able to enable some feature to warn (or error) on N+1. But optimizing your code to avoid run in fewer queries is often (in cases where prefetch_related doesn't cut it) non-trivial. And presumably when you get to this scale, you're also code-reviewing the jr developers.. so I'm not sure how urgent it is that we all "Ban N+1 in Django" as a rule.

Suor 4y ago

Should be easy enough to implement. You only need a context manager that adds 1 to some threadlocal flag on enter and subrracts on exit then check this flag in the monkey patch. Not sure how costly that will be though.

Suor 4y ago
Here is an example of such thing https://github.com/Suor/django-cacheops/blob/8b3a79de29b2545179dcb591bf5d136c07ead4a2/cacheops/invalidation.py#L84-L101 https://github.com/Suor/django-cacheops/blob/8b3a79de29b2545...

selcuka 4y ago

It's not ideal, but you can simply log `DeferredAttribute.__get__` calls instead of raising an exception.

tantalor 4y ago

Just keep a list of exemptions in a file

squeaky-clean 4y ago

You could probably implement in in a context manager, have the `__enter__()` method execute the `_DA_get_original, DeferredAttribute.__get__ = DeferredAttribute.__get__, _DeferredAttribute_get` code and have the `__exit__` method undo that re-assignment. (Or maybe the reverse is better. Ban N+1 by default and the context manager `__enter__` puts back the original assignment, `__exit__` brings back the banned version).

brody_hamer 4y ago

I agree that in most cases this will amount to premature optimisation. Most django sites will forever remain small, and there's a lot of benefit to a simple, intuitive (if inefficient) ORM. Namely; that's it's easy for a jr developer to get in, and get results. (That's why we're coding in python, yes?) Naturally yes, there's a certain scale where N+1 starts to really matter. And in those situations it might be helpful to be able to enable some feature to warn (or error) on N+1. But optimizing your code to avoid run in fewer queries is often (in cases where prefetch_related doesn't cut it) non-trivial. And presumably when you get to this scale, you're also code-reviewing the jr developers.. so I'm not sure how urgent it is that we all "Ban N+1 in Django" as a rule.