Opened 3 hours ago

Last modified 12 minutes ago

#37218 new Cleanup/optimization

Document that lazy relation access is synchronous only

Reported by: Jacob Walls Owned by: Jacob Walls
Component: Documentation Version: 6.1
Severity: Normal Keywords: fetch modes, async
Cc: Adam Johnson, Simon Charette, Carlton Gibson Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Jacob Walls)

See fiddle:

from asgiref.sync import async_to_sync
from django.contrib.auth.models import User
from django.db import models


class Person(models.Model):
    name = models.CharField(max_length=100)
    user = models.ForeignKey(User, models.CASCADE)
    

@async_to_sync
async def async_view():
    qs = Person.objects.fetch_mode(models.FETCH_PEERS)
    async for el in qs:
        print(el.user)


def run():
    admin = User.objects.create(username="admin")
    instance = Person.objects.create(name='John Doe', user=admin)
    instance2 = Person.objects.create(name='Jane Doe', user=admin)
    async_view()

  File "/app/app/models.py", line 15, in async_view
    print(el.user)
          ^^^^^^^
  File "/django-pr/django/db/models/fields/related_descriptors.py", line 266, in __get__
    instance._state.fetch_mode.fetch(self, instance)
  File "/django-pr/django/db/models/fetch_modes.py", line 38, in fetch
    fetcher.fetch_many(instances)
  File "/django-pr/django/db/models/fields/related_descriptors.py", line 291, in fetch_many
    prefetch_related_objects(missing_instances, self.field.name)
  File "/django-pr/django/db/models/query.py", line 2701, in prefetch_related_objects
    obj_list, additional_lookups = prefetch_one_level(
                                   ^^^^^^^^^^^^^^^^^^^
  File "/django-pr/django/db/models/query.py", line 2876, in prefetch_one_level
    all_related_objects = list(rel_qs)
                          ^^^^^^^^^^^^
  File "/django-pr/django/db/models/query.py", line 434, in __iter__
    self._fetch_all()
  File "/django-pr/django/db/models/query.py", line 2239, in _fetch_all
    self._result_cache = list(self._iterable_class(self))
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/django-pr/django/db/models/query.py", line 98, in __iter__
    results = compiler.execute_sql(
              ^^^^^^^^^^^^^^^^^^^^^
  File "/django-pr/django/db/models/sql/compiler.py", line 1622, in execute_sql
    cursor = self.connection.cursor()
             ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/django-pr/django/utils/asyncio.py", line 24, in inner
    raise SynchronousOnlyOperation(message)
django.core.exceptions.SynchronousOnlyOperation: You cannot call this from an async context - use a thread or sync_to_async.

We probably need to add AFETCH_PEERS and AFETCH_ONE fetch modes, and duplicate the fetch_{one,many}() methods into afetch_{one,many}() on the descriptors.

I can hustle this in before the release candidate if it seems sane.

Change History (7)

comment:1 by Jacob Walls, 3 hours ago

Description: modified (diff)
Severity: NormalRelease blocker

comment:2 by Simon Charette, 2 hours ago

I'm not sure I understand the need to introduce a fetch mode for this case?

Isn't the problem already present without fetch mode involved

@async_to_sync
async def async_view():
    qs = Person.objects.all()
    async for el in qs:
        print(el.user)
  File "/app/app/models.py", line 15, in async_view
    print(el.user)
          ^^^^^^^
  File "/django-pr/django/db/models/fields/related_descriptors.py", line 266, in __get__
    instance._state.fetch_mode.fetch(self, instance)
  File "/django-pr/django/db/models/fetch_modes.py", line 17, in fetch
    fetcher.fetch_one(instance)
  File "/django-pr/django/db/models/fields/related_descriptors.py", line 279, in fetch_one
    rel_obj = self.get_object(instance)
              ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/django-pr/django/db/models/fields/related_descriptors.py", line 231, in get_object
    return qs.get(self.field.get_reverse_related_filter(instance))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/django-pr/django/db/models/query.py", line 679, in get
    num = len(clone)
          ^^^^^^^^^^
  File "/django-pr/django/db/models/query.py", line 416, in __len__
    self._fetch_all()
  File "/django-pr/django/db/models/query.py", line 2239, in _fetch_all
    self._result_cache = list(self._iterable_class(self))
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/django-pr/django/db/models/query.py", line 98, in __iter__
    results = compiler.execute_sql(
              ^^^^^^^^^^^^^^^^^^^^^
  File "/django-pr/django/db/models/sql/compiler.py", line 1622, in execute_sql
    cursor = self.connection.cursor()
             ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/django-pr/django/utils/asyncio.py", line 24, in inner
    raise SynchronousOnlyOperation(message)
django.core.exceptions.SynchronousOnlyOperation: You cannot call this from an async context - use a thread or sync_to_async.

Since the is no way to yield the co-routine to the event loop through the descriptor access (there is no such thing as __aget__) I fear writing an async fetching mode will face the same challenges as trying to get normal descriptor access to asynchronously perform a database query prior to the introduction of fetch mode. This problem has been know since the early days of making the ORM async.

In other words, the only safe mode in an async context is RAISE since there is no way to print(await el.user)

Last edited 2 hours ago by Simon Charette (previous) (diff)

comment:3 by Carlton Gibson, 2 hours ago

Cc: Carlton Gibson added

Yes... this is *exactly* the No Deferred Queries constraint. Totally expected behaviour IMO.

comment:4 by Jacob Walls, 2 hours ago

Resolution: invalid
Status: assignedclosed

I was aware of that limitation, and the prescription to use select_related() to avoid deferred queries, but as the spiritual successor to select_related() when laziness is wanted, I guess I assumed an analogous afetch_peers mode would have a way to swap in aprefetch_related_objects in place of prefetch_related_objects, but looking at that stacktrace, we have that pesky __get__() right there at the top, so, ack--of course not! Thanks for confirming.

I'll have a look around to see if there's a sentence or two we should add about this.

Last edited 114 minutes ago by Jacob Walls (previous) (diff)

comment:5 by Jacob Walls, 43 minutes ago

Component: Database layer (models, ORM)Documentation
Severity: Release blockerNormal
Type: BugCleanup/optimization

I was surprised to see we only have information about "beware defer() and only()" rather than also "beware lazy relation access" in the async queries section.

I did a few tweaks here, let me know if you think any of it is useful?
PR

comment:6 by Jacob Walls, 41 minutes ago

Has patch: set
Resolution: invalid
Status: closednew
Summary: Fetch modes aren't async-compatibleDocument that lazy relation access is synchronous only

comment:7 by Carlton Gibson, 12 minutes ago

Triage Stage: UnreviewedAccepted

Seems reasonable. (As per my comment on the PR, I think the new No deferred queries addition to the Asynchronous queries section is probably sufficient.)

Note: See TracTickets for help on using tickets.
Back to Top