Техпрогон
ВойтиНачать бесплатно
Код-ревью Middle Django ORM и N+1 спрашивают часто обновлено

Код-ревью: отчёт по клиентам на Django ORM

Вопрос

Функция строит отчёт по активным клиентам. Найдите проблемы производительности и корректности.

from django.db.models import Sum
from .models import Customer, Order


def customer_report():
    rows = []
    customers = Customer.objects.all()
    for c in customers:
        if not c.is_active:
            continue
        orders = Order.objects.filter(customer=c)
        if len(orders) == 0:
            continue
        total = 0
        for o in orders:
            total += o.amount
        rows.append({
            'name': c.name,
            'manager': c.manager.name,
            'orders': orders.count(),
            'total': total,
        })
    return rows

Коротко

Весь отчёт укладывается в один запрос: Одна связь «ко многим» в аннотациях безопасна; если добавить вторую (например, оплаты), суммы начнут задваиваться, и понадобятся подзапросы. Если manager может быть пустым, исходный код упал бы на .name, это тоже стоит учесть.

Что должен найти ревьюер

  1. Производительностьстрока 9
    Фильтрация по is_active в Python: из базы грузятся все клиенты; нужно filter(is_active=True)
  2. Производительностьстрока 11
    Запрос заказов на каждого клиента (N+1); посчитать всё одним запросом через annotate
  3. Производительностьстрока 12
    len(orders) загружает все заказы ради проверки; достаточно exists() или фильтра по аннотации
  4. Производительностьстрока 16
    Сумма считается в Python перебором; нужно Sum("orders__amount") в базе
  5. Производительностьстрока 19
    Обращение к c.manager делает ещё по запросу на клиента; нужен select_related("manager")
  6. Производительностьстрока 20
    count() по уже загруженному QuerySet делает лишний запрос; число есть в аннотации

Разбор

Весь отчёт укладывается в один запрос:

from django.db.models import Count, Sum

def customer_report():
    qs = (
        Customer.objects.filter(is_active=True)
        .select_related('manager')
        .annotate(orders_count=Count('orders'), total=Sum('orders__amount'))
        .filter(orders_count__gt=0)
    )
    return [
        {'name': c.name, 'manager': c.manager.name,
         'orders': c.orders_count, 'total': c.total}
        for c in qs
    ]

Одна связь «ко многим» в аннотациях безопасна; если добавить вторую (например, оплаты), суммы начнут задваиваться, и понадобятся подзапросы. Если manager может быть пустым, исходный код упал бы на .name, это тоже стоит учесть.

Ответьте сами, а не прочитайте

В тренажёре вопрос попадёт в повторения и вернётся, когда начнёт забываться. Плюс билеты с зачётом: 20 вопросов, не больше 2 ошибок.

Потренироваться бесплатно

Предыдущий вопрос

Задача Celery и незакоммиченная транзакция

Следующий вопрос

Защита от двойного списания

Урок по теме

Django ORM: ленивые QuerySet, N+1, select_related и prefetch_related

Ещё по теме «Django ORM и N+1»

Все вопросы модуля