Skip to content

feat: rewrote the method for getting m2m objects#2205

Open
VladislavYar wants to merge 4 commits into
tortoise:developfrom
VladislavYar:expanding-prefetch-capabilities
Open

feat: rewrote the method for getting m2m objects#2205
VladislavYar wants to merge 4 commits into
tortoise:developfrom
VladislavYar:expanding-prefetch-capabilities

Conversation

@VladislavYar
Copy link
Copy Markdown

@VladislavYar VladislavYar commented May 27, 2026

Description

The _prefetch_m2m_relation method of the BaseExecutor class has been rewritten. Now objects are received directly through the specified queryset, which allows you to correctly use select_related, annotate, etc.

Motivation and Context

prefetch_related (m2m) functionality becomes like in Django.

How Has This Been Tested?

Added tests to tests/test_prefetching.py with verification of receiving data from annotate and select_related.

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added the changelog accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 27, 2026

Merging this PR will not alter performance

✅ 24 untouched benchmarks


Comparing VladislavYar:expanding-prefetch-capabilities (bbafb5b) with develop (179ea6f)

Open in CodSpeed

@waketzheng
Copy link
Copy Markdown
Contributor

Thanks for the patch. The change looks useful for enabling select_related and annotate inside M2M prefetches, but a few behaviors seem worth confirming before merging:

  • tortoise/backends/base/executor.py:639-645 rebuilds each parent's relation list from the through-table rows. That means the final order is driven by the through-table query, not by the order returned from the custom queryset. Could you confirm whether Prefetch("participants", Team.all().order_by("name")) is expected to preserve queryset ordering?

  • tortoise/backends/base/executor.py:624-627 already executes the custom queryset, and the result is then passed through _execute_prefetch_queries() again. Could you confirm whether this is intended, or whether nested prefetches on the custom queryset may be run twice?

  • tortoise/backends/base/executor.py:611-619 assumes each related object has a usable pk. If the queryset uses .only() and omits the primary key, the mapping step may not be able to match through-table rows back to related objects. Could you confirm whether .only() should be supported here, and if so whether the PK needs to be forced into the select list?

Overall, the direction looks promising, but these cases seem worth verifying with focused regression tests.

@VladislavYar
Copy link
Copy Markdown
Author

VladislavYar commented May 28, 2026

@waketzheng
You're right on all counts.

  1. Supported order_by. Added a test;
  2. Deleted the repeated prefetch;
  3. When using only, I will force the primary key to be added. Added a test.

@waketzheng
Copy link
Copy Markdown
Contributor

Thanks for the update. Consider this:

The previous concerns around order_by, nested prefetch execution, and .only() requiring the related PK look addressed in the latest version.

One remaining edge case may be worth confirming: in the new through-table lookup, the parent PKs are converted with model_pk.to_db_value(...), but the related PKs are passed to isin() as Python-level obj.pk values:

.where(forward_field.isin(tuple(related_objects_by_pks)))

For UUIDField, obj.pk is a uuid.UUID, while UUIDField.to_db_value() converts it to str(value). This appears to make M2M prefetch_related() fail for UUID primary keys on SQLite with:

sqlite3.ProgrammingError: Error binding parameter 2: type 'UUID' is not supported

A regression test in tests/fields/test_m2m_uuid.py should expose this, especially because the existing m2m_uuid_models fixture already covers both normal UUID PKs and source-field UUID PKs:

@pytest.mark.asyncio
async def test_prefetch_related(db, m2m_uuid_models):
    UUIDPkModel, UUIDM2MRelatedModel = m2m_uuid_models
    one = await UUIDPkModel.create()
    two = await UUIDM2MRelatedModel.create()
    await one.peers.add(two)

    fetched = await UUIDPkModel.get(pk=one.pk).prefetch_related("peers")

    assert list(fetched.peers) == [two]

It may be safer to convert the related PKs before the through-table query as well, analogous to instance_pks, for example by using related_model_pk.to_db_value(obj.pk, obj) for the objects in related_objects_by_pks.values().

@VladislavYar
Copy link
Copy Markdown
Author

@waketzheng Thanks! I corrected it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants