News for hackers. Stuff that matters.
Re:My £4 a month server can handle 4.2M requests a day by benhoyt on Tuesday September 07, 2021 @07:28PM (#28451096)
Yeah, I see this a lot. I think it's especially easy to introduce this kind of "accidentally quadratic" behaviour using magical ORMs like Django's, where an innocent-looking attribute access like user.groups can trigger a database query ... access user.groups inside a loop and things get bad quickly.

In the case of groups and permissions there's probably only a few of each, so fetching all of them is probably fine. But depending on your data -- say you're fetching comments written by a subset of users, you can tweak the above to use IN filtering, something like this Python-ish code:

  users = select('SELECT id, name FROM users WHERE id IN $1', user_ids)
  comments = select('SELECT user_id, text FROM comments WHERE user_id IN $1', user_ids)
  comments_by_user_id = defaultdict(list)
  for c in comments:
    comments_by_user_id[c.user_id].append(c)
  for u in users:
    u.comments = comments_by_user_id[u.id]
Only two queries, and O(users + comments).

For development, we had a ?queries=1 query parameter you could add to the URL to show the number of SQL queries and their total time at the bottom of the page. Very helpful when trying to optimize this stuff. "Why is this page doing 350 queries totalling 5 seconds? Oops, I must have an N+1 query issue!"

Re:My £4 a month server can handle 4.2M requests a day by simonw on Tuesday September 07, 2021 @07:53PM (#28451266)
Django's prefetch_related mechanism essentially implements the pattern you describe here for you: https://docs.djangoproject.com/en/3.2/ref/models/querysets/#...
Re:My £4 a month server can handle 4.2M requests a day by benhoyt on Tuesday September 07, 2021 @08:19PM (#28451440)
Thanks. Yeah, I think I used that years ago when I first ran into this problem, and it worked well. Whether one uses an ORM or not, one needs to know how to use one's tools. My problem (not just with Django, but with ORMs in general) is how they make bad code look good. Like the following (I don't know Django well anymore, but something like this):

  users = User.objects.all()
  for u in user:
    print(u.name, len(u.comments))
To someone who doesn't know the ORM (or Python) well, u.comments looks cheap ("good"), but it's actually doing a db query under the hood each time around the loop ("bad"). Not to mention it's fetching all the comments when we're only using their count. Whereas if you did that in a more direct-SQL way:

  users = select('SELECT id, name FROM users WHERE id IN $1', user_ids)
  for u in user:
    num_comments = get('SELECT COUNT(*) FROM comments WHERE user_id = $1', u.id)
    print(u.name, num_comments)
This makes the bad pattern look bad. "Oops, I'm doing an SQL query every loop!"

The other thing I don't like about (most) ORMs is they fetch all the columns by default, even if the code only uses one or two of them. I know most ORMs provide a way to explicitly specify the columns you want, but the easy/simple default is to fetch them all.

I get the value ORMs provide: save a lot of boilerplate and give you nice classes with methods for your tables. I wonder if there's a middle ground where you couldn't do obviously bad things with the ORM without explicitly opting into them. Or even just a heuristic mode for development where it yelled loudly if it detected what looked like an N+1 issue or other query inside a loop.

Re:My £4 a month server can handle 4.2M requests a day by simonw on Tuesday September 07, 2021 @08:35PM (#28451553)
I agree that this stuff can definitely be handled better.

https://github.com/django-query-profiler/django-query-profil... has a neat option for detecting likely N+1 queries. I usually use the Django Debug Toolbar for this.

Django's ".only()" method lets you specify just the columns you want to retrieve - with the downside that any additional property access can trigger another SQL query. I thought I'd seen code somewhere that can turn those into errors but I'm failing to dig it up again now.

I've used the assertNumQueries() assertion in tests to guard against future changes that accidentally increase the number of queries being made without me intending that.

The points you raise are valid, but there are various levels of mitigations for them. Always room for improvement though!

Re:My £4 a month server can handle 4.2M requests a day by benhoyt on Tuesday September 07, 2021 @08:41PM (#28451591)
Thanks for the thoughtful responses and the link -- Django Query Profiler looks nice!
Re:My £4 a month server can handle 4.2M requests a day by cuu508 on Tuesday September 07, 2021 @09:49PM (#28452057)

    users = User.objects.all()
    for u in user:
        print(u.name, len(u.comments))
This is fine if you are working with a small data set. It is inefficient, but if it's quick enough, readability trumps efficiency IMHO.

Django ORM has a succinct way of doing the "SELECT COUNT(*)" pattern:

    users = User.objects.all()
    for u in user:
        print(u.name, u.comments.count())
And you can use query annotations to get rid of the N+1 query issue altogether:

    users = User.objects.annotate(n_comments=Count("comments"))
    for u in user:
        print(u.name, u.n_comments)
Re:My £4 a month server can handle 4.2M requests a day by tinus_hn on Wednesday September 08, 2021 @03:42AM (#28454276)
> This is fine if you are working with a small data set. It is inefficient, but if it's quick enough, readability trumps efficiency IMHO.

And this is how you end up with the problems the parent is describing. During testing and when you setup the system you always have a small dataset so it appears to work fine. But when it’s real work the system collapses.

Re:My £4 a month server can handle 4.2M requests a day by danjac on Wednesday September 08, 2021 @01:10AM (#28453283)
A good habit when writing unit tests is to use assertNumQueries especially with views. It's very easy even for an experienced developer to inadvertently add an extra query per row: for example if you have a query using only() to restrict the columns you return, and in a template you refer to another column not covered by only(), Django will do another query to fetch that column value (not 100% if that's still the case, but that was the behavior last time I looked). The developer might be just fixing what they think is a template issue and won't know they've just stepped on a performance landmine.
Re:My £4 a month server can handle 4.2M requests a day by benhoyt on Wednesday September 08, 2021 @01:17AM (#28453321)
Sounds like a good habit, yeah. But the fact that an attribute access in a template (which might be quite far removed from the view code) can kick off a db query is an indication there's something a bit too magical about the design. Often front-end developers without much knowledge of the data model or the SQL schema are writing templates, so this is a bit of a foot-gun.
Re:My £4 a month server can handle 4.2M requests a day by simonw on Wednesday September 08, 2021 @04:22PM (#28462539)
I've wanted a Django mechanism in the past that says "any SQL query triggered from a template is a hard error" - that way you could at least prevent accidental template edits from adding more SQL load.
Re:My £4 a month server can handle 4.2M requests a day by jbboehr on Wednesday September 08, 2021 @02:30AM (#28453769)
My SQL is a bit rusty, but, to reduce the queries even further, I think you can do:

> SELECT user_id, COUNT(*) FROM comments WHERE user_id IN $1 GROUP BY user_id

Re:My £4 a month server can handle 4.2M requests a day by bradstewart on Wednesday September 08, 2021 @09:43AM (#28456837)
> Or even just a heuristic mode for development where it yelled loudly if it detected what looked like an N+1 issue or other query inside a loop.

Fwiw, ActiveRecord with the Bullet gem does exactly that. I'd guess there's an equivalent for Django.

Re:My £4 a month server can handle 4.2M requests a day by petters on Wednesday September 08, 2021 @03:26AM (#28454168)
select_related and prefetch_related are absolutely essential when coding Django.
Re:My £4 a month server can handle 4.2M requests a day by theptip on Wednesday September 08, 2021 @03:37AM (#28454248)
I’m coming around to considering Django-seal to be mandatory for non-trivial applications. It will “seal” a query set so that you can’t make accidental DB lookups after the initial fetch. That way you can be more confident that you are doing the right joins in the initial query, and you are safe from the dreaded O(N) ORM issue.

SqlAlchemy has this as part of the ORM, it should really be part of Django IMO.

Re:My £4 a month server can handle 4.2M requests a day by c17r on Wednesday September 08, 2021 @11:30AM (#28458551)
I've never heard of django-seal before, thanks!
Re:My £4 a month server can handle 4.2M requests a day by kbenson on Tuesday September 07, 2021 @11:23PM (#28452654)
When I was doing a lot of DBIx::Class work, I contemplated writing a plugin to allow me to lock and and unlock automatic relation fetches and die (throw) if one was attempted when locked. I would carefully prefetch my queries (auto-fill relations from a larger joined query) and if something changed or there was an edge case I missed that caused fetching deep in a loop, it might kill performance, but be hard to track down if not given the best directions on what caused it other than "this is slow".

It's one of those things that in the long run would have been much more time effective to write, but the debugging never quite took long enough each time to make me take the time.

Re:My £4 a month server can handle 4.2M requests a day by mschuster91 on Wednesday September 08, 2021 @05:35AM (#28454944)
> For development, we had a ?queries=1 query parameter you could add to the URL to show the number of SQL queries and their total time at the bottom of the page. Very helpful when trying to optimize this stuff. "Why is this page doing 350 queries totalling 5 seconds? Oops, I must have an N+1 query issue!"

I'm so glad Symfony (PHP framework) has a built-in profiler and analysis tooling there...