Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Ban 1+N in Django

suor.github.io

Post reply on HN
Print view View original

41–50 of 153 posts

Re: Ban 1+N in Django

#41

Post by squeaky-clean »

irjustin wrote: post #32

Rails has Bullet[0] to help identify and warn you against N+1 Does Django have anything active? Quick search revealed nplusone[1] but its been dead since 2018. [0] https://github.com/flyerhzm/bullet [1] https://github.com/jmcarp/nplusone

The second, longer snippet from the OP article seems to do basically this. To modify it to be exactly like what bullet describes (warning you when in debug or test mode, instead of raising an exception, and have no effect in prod) you can replace line 31 with the logger.error() from line 33, and delete the else-case (lines 32 & 33)

Re: Ban 1+N in Django

#42

Post by Suor »

spapas82 wrote: post #14

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.

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.

Re: Ban 1+N in Django

#43

Post by Suor »

Suor wrote: post #42
spapas82 wrote: post #14

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.

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.

Here is an example of such thing https://github.com/Suor/django-cacheops/blob/8b3a79de29b2545...

Re: Ban 1+N in Django

#44

Post by Suor »

btown wrote: post #18

My Chaotic Good take on this: one could implement a qs.auto_fetch_deferred() that emits model instances with weak back-references to a WeakSet of all instances emitted, and on a deferred get on ANY instance, it prefetches that attribute onto ALL of the instances... so that it doesn't just complain, but actually fixes your 1+N issue. But here lies absolute madness...

Actually there is such thing already https://pypi.org/project/django-auto-prefetch/ Not a monkey patch though, so will only work on the models you inherit from this.

Re: Ban 1+N in Django

#45

Post by squeaky-clean »

spapas82 wrote: post #14

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.

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).

Re: Ban 1+N in Django

#46

Post by rlawson »

jonatron wrote: post #8

See also django-zen-queries https://github.com/dabapps/django-zen-queries , which can make it impossible for changes to a template to trigger queries.

Came here to post just that. Really like zen queries

Re: Ban 1+N in Django

#47

Post by roflyear »

binarymax wrote: post #16

This is why I always advocated against ORMs. It’s so easy to fall into traps like this without even knowing it, and while you can work around it in some ORMs it is not obvious. Writing SQL is not that hard, and mapping the results to a type isn’t that hard either. So with an ORM you might end up saving several hours of work up front for lots of pain later.

ORMs generally give you a lot of nice things, and you usually (always??) can just write pure SQL and use the models you've defined (and all those nice things).

So, use an ORM, but write SQL if you want? Sounds like a good idea, actually.

Re: Ban 1+N in Django

#48

Post by ralmidani »

mattbillenstein wrote: post #28
nickjj wrote: post #7

There is a case where having N+1 queries are beneficial. In Rails terms, it's when you perform Russian doll caching, but you can do this in any framework. The idea is you can cache a specific X thing which might make a query to an associated Y thing. A textbook N+1 query case (ie. a list of posts (X) that get the author's name (Y)). If you render the view without any cache with 10 things then you'd perform 20 queries…

Now you have like three problems instead of one - N+1 queries in the cold-cache case is slow, cache invalidation when something changes, and much more overall complexity...

Also, off-by-one errors.

Re: Ban 1+N in Django

#49

Post by Suor »

lr4444lr wrote: post #9

I don't disagree with the author in principle, but I find once the data gets big enough where it makes a difference, I've already shifted to using ".values()" to avoid the overhead of model creation, and the KeyErrors that will throw if I leave the query lazy is tantamount to the solution he describes.

I usually use `.values()` and `.values_list()` only when model creation is really slow. Usually many rows fetched.

Re: Ban 1+N in Django

#50

Post by irjustin »

squeaky-clean wrote: post #41
irjustin wrote: post #32

Rails has Bullet[0] to help identify and warn you against N+1 Does Django have anything active? Quick search revealed nplusone[1] but its been dead since 2018. [0] https://github.com/flyerhzm/bullet [1] https://github.com/jmcarp/nplusone

The second, longer snippet from the OP article seems to do basically this. To modify it to be exactly like what bullet describes (warning you when in debug or test mode, instead of raising an exception, and have no effect in prod) you can replace line 31 with the logger.error() from line 33, and delete the else-case (lines 32 & 33)

It's not a risk I'm willing to take nor do I believe other general Django engs should take. It's fine if it's a pet project or if you've got deep knowledge of Django's inner workings. This requires understanding the affects of monkey patching core __get__ on DeferredAttribute.

When reading the code vs core Django, it doesn't faithfully reproduce the normal case missing _check_parent_chain[0]. I'm not sure if that code path is supposed to be left out or of it's simply missing? The documentation of the code snippet doesn't explicitly state either.

Code snippets that affect the project as a whole combined with one layer removed is library code. It needs to come with strong specs. The worst case scenario is you get differing behavior in production and development.

I 100% appreciate the spirit of the post, but Monkey Patching core Django files is not to be taken lightly in production code.

[0] https://github.com/django/django/blob/main/django/db/models/...

Post reply on HN