fix(optimizer): Fix nested pagination optimization for m2m relations

[bellini666]

When annotating something from the relation into the prefetch queryset for a m2m relation, Django will mistakenly not reuse the existing join and end up resulting in the generation of spurious results.

There's an ongoing fix for this i this ticket: https://code.djangoproject.com/ticket/35677

This is monkey patching older versions of Django which doesn't contain the fix, and most likely won't (Django usually only backports security issues), to fix the issue.

Thanks @SupImDos for providing an MRE in the form of a test for this!

Fix #650

Summary by Sourcery

Fix nested pagination optimization for many-to-many relations in Django 5.1 or older by monkey-patching the _filter_prefetch_queryset and add_q methods to reuse existing aliases and joins.

Bug Fixes:

• Fixed an issue where nested pagination with many-to-many relationships generated spurious results in older Django versions.

Enhancements:

• Applied a monkey patch to improve pagination performance for nested many-to-many relations in Django 5.1 or older.

Tests:

• Added a test case to verify the fix for nested pagination with many-to-many relationships.

[sourcery-ai]

Reviewer's Guide by Sourcery

This pull request fixes a bug in nested pagination optimization for many-to-many (m2m) relations in Django versions prior to 5.2. The bug occurs when annotating related objects in a prefetch queryset, causing Django to generate incorrect results. The fix involves monkey-patching older Django versions to address the issue.

Sequence diagram for patched nested pagination optimization flow

sequenceDiagram
    participant Q as QuerySet
    participant FP as _filter_prefetch_queryset
    participant AQ as Query.add_q

    Q->>FP: Filter prefetch queryset
    Note over FP: Create predicate with field_name__in
    Note over FP: Apply window functions if sliced
    FP->>AQ: add_q(predicate, reuse_all_aliases=True)
    Note over AQ: Use all existing aliases
    Note over AQ: Add clause to WHERE conditions
    AQ-->>FP: Return modified query
    FP-->>Q: Return filtered queryset

Loading

Class diagram showing patched Django query components

classDiagram
    class Query {
        +add_q(q_object, reuse_all_aliases)
        -_add_q(q_object, can_reuse)
        +where
        +alias_map
    }

    class _filter_prefetch_queryset {
        +queryset
        +field_name
        +instances
    }

    class OptimizerExtension {
        +enable_nested_relations_prefetch: bool
        +prefetch_custom_queryset: bool
        +on_execute()
    }

    OptimizerExtension ..> Query: uses
    Query ..> _filter_prefetch_queryset: uses

    note for OptimizerExtension "Applies pagination fix when nested relations prefetch is enabled"
    note for Query "Patched to support alias reuse"
    note for _filter_prefetch_queryset "Patched to use reuse_all_aliases"

Loading

File-Level Changes

Change	Details	Files
Monkey-patched Django's _filter_prefetch_queryset and Query.add_q methods to reuse aliases correctly.	Added a new apply_pagination_fix function to apply the monkey-patch. Copied the original _filter_prefetch_queryset and Query.add_q methods from Django 4.2, 5.0, and 5.1. Modified the _filter_prefetch_queryset method to reuse aliases when adding predicates to the queryset. Modified the Query.add_q method to reuse all aliases when available. Called the apply_pagination_fix function in the DjangoOptimizerExtension class if enable_nested_relations_prefetch is True.	strawberry_django/utils/patches.py strawberry_django/optimizer.py
Added a test case for nested pagination with m2m relations.	Created a new test function test_nested_pagination_m2m to verify the fix. Added two tags and three issues with m2m relationships. Queried the tags with their related issues. Asserted the expected number of database queries and the correctness of the results.	tests/test_pagination.py
Added Meta classes with ordering to the Tag and Issue models.	Added a Meta class to the Tag model with ordering = (\"id\",). Added a Meta class to the Issue model with ordering = (\"id\",).	tests/projects/models.py
Added a variable to store existing aliases in the queryset.	Added existing_aliases = set(queryset.query.alias_map) before annotating the queryset.	strawberry_django/pagination.py

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#650	Fix nested pagination through m2m fields when optimizer is enabled to prevent duplicate results	✅

Possibly linked issues

• Optimized nested pagination is broken for m2m fields #650: The PR fixes the issue by correcting how Django handles m2m relations in prefetch queries, resolving the incorrect pagination.

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time. You can also use
  this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey @bellini666 - I've reviewed your changes and they look great!

Here's what I looked at during the review

• 🟢 General issues: all looks good
• 🟢 Security: all looks good
• 🟡 Testing: 1 issue found
• 🟢 Complexity: all looks good
• 🟢 Documentation: all looks good

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 75.60976% with 10 lines in your changes missing coverage. Please review.

Project coverage is 89.00%. Comparing base (48b54fd) to head (bbfcbf6).

Files with missing lines	Patch %	Lines
strawberry_django/utils/patches.py	73.68%	10 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #681      +/-   ##
==========================================
- Coverage   89.17%   89.00%   -0.18%     
==========================================
  Files          41       42       +1     
  Lines        3778     3819      +41     
==========================================
+ Hits         3369     3399      +30     
- Misses        409      420      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[patrick91]

nice!