Opened 2 hours ago
Last modified 6 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: | Unreviewed |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description (last modified by )
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 (6)
comment:1 by , 2 hours ago
| Description: | modified (diff) |
|---|---|
| Severity: | Normal → Release blocker |
comment:3 by , 95 minutes ago
| Cc: | added |
|---|
Yes... this is *exactly* the No Deferred Queries constraint. Totally expected behaviour IMO.
comment:4 by , 79 minutes ago
| Resolution: | → invalid |
|---|---|
| Status: | assigned → closed |
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.
comment:5 by , 8 minutes ago
| Component: | Database layer (models, ORM) → Documentation |
|---|---|
| Severity: | Release blocker → Normal |
| Type: | Bug → Cleanup/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 , 6 minutes ago
| Has patch: | set |
|---|---|
| Resolution: | invalid |
| Status: | closed → new |
| Summary: | Fetch modes aren't async-compatible → Document that lazy relation access is synchronous only |
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
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.The only safe mode in an async context is
RAISE.