How to use less database look-ups during filter?

Question

I have a section of code that makes several db calls and I am trying to optimize it. Is there a way to get rid of:

username = users.models.User.objects.filter(groups__name=group)

or..

user_obj = users.models.User.objects.get(username=user)

or combine them so that I am only making one database lookup during this transaction instead of two?

        for group in groups:
            username = users.models.User.objects.filter(groups__name=group)
            for user in username:
                form.instance.unread.add(user)
                user_obj = users.models.User.objects.get(username=user)
                user_obj.send_sms('memo')
                user_obj.send_email('memo')

Is this something I am unnecessarily worrying about?

score 1 · Accepted Answer · edited Oct 22 '19 at 06:26

Combining (from the example code of the question) is not a good idea, its looking up DB even more:

 for group in groups:
        username = users.models.User.objects.filter(groups__name=group)
        for user in username:  # <-- Hitting DB 
            form.instance.unread.add(user)
            user_obj = users.models.User.objects.get(username=user) # <-- DB hit again
            user_obj.send_sms('memo')
            user_obj.send_email('memo')

So, minimum optimization you can do is to evaluate the queryset at first, then iterate through loop:

for group in groups:
    username = list(users.models.User.objects.filter(groups__name=group))
    for user in username:
        form.instance.unread.add(user)
        user.send_sms('memo')
        user.send_email('memo')

More information can be found on documentation regarding how querysets are evaluated.

And IMHO, the most optimum solution is like this(I got rid of the for loop):

for group in groups:
    users = users.models.User.objects.filter(groups__name=group)
    form.instance.unread.add(*users) # unpacking the queryset and passing it through add (m2m) method. Reference: https://stackoverflow.com/a/4959580/2696165
    send_sms('memo', users.values_list('phone_number'))  # you need to convert this method so that it is not attached to user object and can accept a list of phone numbers
    send_email('memo', users.values_list('email')) # you need to convert this method so that it is not attached to user object and can accept a list of emails

What would you recommend doing with the `send_sms` and `send_email` methods? I literally just moved them from this view into the `User` class and was quite happy that I was able to get that code out of my views.py file :| — David Alford, Oct 22 '19 at 05:56
I usually keep them in a separate file, I call it a service. You can checkout my answer regarding this: https://stackoverflow.com/a/57387971/2696165 — ruddra, Oct 22 '19 at 05:59
I actually just realized that these methods were not in my views.py - I had them in a file called helpers.py. The issue I am seeing now is that these methods use 6 attributes from this instance and one of them uses an instance method as well... So I feel that these really belong as methods of the class. I think the `minimum optimization` you offered which contains the `for user in usernames` is actually the way to go here. I have learned quite a lot from you tonight so thank you much! — David Alford, Oct 22 '19 at 06:12

score 1 · Answer 2 · answered Oct 22 '19 at 05:45

All the groups matching the list can be retrieved first in one query using __in lookup.

Then users in the matched group queryset can be queried looking up the resulting group queryset.

group_qs = users.models.Group.objects.filter(name__in=groups)
users_qs = users.models.User.objects.filter(groups__in=group_qs)

for user in users_qs:
    form.instance.unread.add(user.username)
    user.send_sms('memo')
    user.send_email('memo')

You can chain the lookups to be:

users_qs = users.models.User.objects.filter(groups__name__in=groups)

How to use less database look-ups during filter?

2 Answers2