Refactoring Django ORM

#2
by welltraum - opened

😃: Refactory this Django ORM query:

        if is_archived is not None:
            if is_archived:
                qs = qs.select_related('company').filter(
                    Q(archived_at__isnull=(not is_archived)) |
                    (Q(company__isnull=(not is_archived)) & Q(company__archived_at__isnull=(not is_archived))),
                )
            else:
                qs = qs.select_related('company').filter(
                    Q(archived_at__isnull=(not is_archived)) &
                    (Q(company__isnull=(not is_archived)) | Q(company__archived_at__isnull=(not is_archived))),
                )

🤖: To refactor the given Django ORM query, you can simplify the conditions by removing redundant checks and using more concise expressions. Here's a refactored version of the code:

if is_archived is not None:
    archived_condition = Q(archived_at__isnull=not is_archived)
    company_condition = Q(company__isnull=not is_archived) & Q(company__archived_at__isnull=not is_archived)

    if is_archived:
        # If is_archived is True, we want records where either the main object or its related company is archived.
        qs = qs.select_related('company').filter(archived_condition | company_condition)
    else:
        # If is_archived is False, we want records where neither the main object nor its related company is archived.
        qs = qs.select_related('company').filter(archived_condition & ~company_condition)

However, the ~company_condition in the else block still includes an unnecessary check for company__isnull, which doesn't need to be inverted since it's already part of the negation due to ~. A further simplified version would be:

if is_archived is not None:
    archived_condition = Q(archived_at__isnull=not is_archived)
    company_archived_condition = Q(company__archived_at__isnull=not is_archived)

    if is_archived:
        # If is_archived is True, we want records where either the main object or its related company is archived.
        qs = qs.select_related('company').filter(archived_condition | company_archived_condition)
    else:
        # If is_archived is False, we want records where neither the main object nor its related company is archived.
        qs = qs.select_related('company').filter(archived_condition & ~company_archived_condition)

But the last version still has a logical inconsistency because when is_archived is False, we are filtering out records that have a company with an archived_at field that is not null. The correct implementation should filter for records where both the main object and the related company are not archived, so the final version should look like this:

if is_archived is not None:
    condition = Q(archived_at__isnull=not is_archived)
    
    if is_archived:
        # If is_archived is True, include objects that are archived or have an archived company.
        qs = qs.select_related('company').filter(condition | Q(company__archived_at__isnull=False))
    else:
        # If is_archived is False, include objects that are not archived and have no archived company.
        qs = qs.select_related('company').filter(condition & Q(company__archived_at__isnull=True))

This version correctly handles the filtering based on the value of is_archived.

welltraum changed discussion title from Оптимизация запроса Django ORM to Refactoring Django ORM

Sign up or log in to comment