n+1 queries for ListModelMixin in rest_framework.mixins

[pradeep-mokara]

class ListModelMixin:
    """
    List a queryset.
    """
    def list(self, request, *args, **kwargs):
        queryset = self.filter_queryset(self.get_queryset())

        page = self.paginate_queryset(queryset)
        if page is not None:
            serializer = self.get_serializer(page, many=True)
            return self.get_paginated_response(serializer.data)

        serializer = self.get_serializer(queryset, many=True)
        return Response(serializer.data)

This seems to iterating through a queryset which is causing n+1 queries

class ListRelatedModelMixin(mixins.ListModelMixin):
    """
    ListMixin to fetch related fields
    """
    def list(self, request, *args, **kwargs):
        related_field_types = ['ForeignKey', 'OneToOneField']
        queryset = self.filter_queryset(self.get_queryset())
        related_fields = [field.name for field in queryset.model._meta.fields if field.get_internal_type() in related_field_types]
        queryset = queryset.select_related(*related_fields)

        page = self.paginate_queryset(queryset)
        if page:
            serializer = self.get_serializer(page, many=True)
            return self.get_paginated_response(serializer.data)

        serializer = self.get_serializer(queryset, many=True)
        return Response(serializer.data)

This removed the n+1 problem in my case, but it feels like it is still incomplete

I would love to know if my approach to solving this problem is correct or the changes are needed elsewhere (glaring at get_paginated_response())

Thanks in advance, Cheers!

[browniebroke]

You can add the appropriate select_related() and prefetch_related() by overriding the get_queryset() method.

Which fields to add will depend a lot on your model and serializers... DRF can't do much automatically here.

[pradeep-mokara]

Hi @browniebroke,
Thank you for your response,

Will there be any downsides for using prefetch_related() and select_related() for related fields on ListModelMixin ?
It seems it will always iterate through each related object to serialize it which results in extra db call.

Side Note: Your profile looks awesome.

[browniebroke]

As I said, it depends. As an example, let's take the models from the Django tutorial:

# models.py
import datetime
from django.db import models
from django.utils import timezone


class Question(models.Model):
    question_text = models.CharField(max_length=200)
    pub_date = models.DateTimeField("date published")

    def __str__(self):
        return self.question_text

    def was_published_recently(self):
        now = timezone.now()
        return now - datetime.timedelta(days=1) <= self.pub_date <= now


class Choice(models.Model):
    question = models.ForeignKey(
        Question, 
        on_delete=models.CASCADE,
        related_name="choices",
    )
    choice_text = models.CharField(max_length=200)
    votes = models.IntegerField(default=0)

    def __str__(self):
        return self.choice_text

You may want to serialize the fields from the Question model only, in which case you don't need any select/pretech related:

# serializers.py
from rest_framework import serializers


class QuestionSerializer(serializers.ModelSerializer):
    class Meta:
        model = Question
        fields = (
            "id",
            "question_text",
            "pub_date",
        )



# views.py
from rest_framework.mixins import ListModelMixin
from rest_framework.viewsets import GenericViewSet


class QuestionViewSet(ListModelMixin, GenericViewSet):
    queryset = Question.objects.all()
    serializer_class = QuestionSerializer

This will work fine an not cause any N+1 queries. At this point, it would be a waste to prefecth related the choices, because your serializer is not using them. You'd over-fetch data, causing extra work in your DB, in the Django ORM, and in the DRF serializer.

However, if you need each question's choices, your serializers may look like this:

# serializers.py
from rest_framework import serializers


class ChoiceSerializer(serializers.ModelSerializer):
    class Meta:
        model = Choice
        fields = (
            "id",
            "choice_text",
            "votes",
        )


class QuestionSerializer(serializers.ModelSerializer):
    choices = ChoiceSerializer(many=True)
    
    class Meta:
        model = Question
        fields = (
            "id",
            "question_text",
            "pub_date",
            "choices",
        )

If you don't change the view, DRF and Django will try to fetch the list of choices only when serializing each each question, causing a SQL query for each question, as opposed to a single SQL queries to fetch choices for all questions at once. In this case, it's worth adding prefetch related to your view, because your serializer need it.

You can do it like this:

# views.py
from rest_framework.mixins import ListModelMixin
from rest_framework.viewsets import GenericViewSet


class QuestionViewSet(ListModelMixin, GenericViewSet):
    queryset = Question.objects.all()
    serializer_class = QuestionSerializer

    def get_queryset(self):
        return super().get_queryset().prefetch_related("choices")  # new prefetch_related

To avoid any unexpected regressions in your application, as fields are added and removed, I usually have a test for the list view that creates a few instances and count the number of DB queries using assertNumQueries (or, if you use pytest-django, assert_num_queries fixture). If you want to dive more into the topic of testing, I recommend this post: https://testdriven.io/blog/django-performance-testing/

[pradeep-mokara]

Thank you so much @browniebroke