[Django] #37201: Prefetch with to_attr silently ignored if matching property exists

[Django]

#37201: Prefetch with to_attr silently ignored if matching property exists
-------------------------------------+-------------------------------------
Reporter: dc-strahlkraft | Type: Bug
Status: new | Component: Database
| layer (models, ORM)
Version: 6.0 | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Minimal reproduction https://gitlab.com/Dan_Strahlkraft/django-prefetch

Given these models:

{{{
class Product(models.Model):
@property
def enabled_images(self):
return self.images.filter(enabled=True)


class ProductImage(models.Model):
product = models.ForeignKey(Product, models.PROTECT,
related_name="images")
enabled = models.BooleanField()
}}}

I want to eagerly load the `enabled_images` property so that it doesn't
hit the database on access. So I write:
`Product.objects.prefetch_related(Prefetch("images",
ProductImage.objects.filter(enabled=True), to_attr="enabled_images"))`.
But that doesn't prefetch anything. The Prefetch is silently ignored.

The simplest workaround I can think of is:

{{{
class Product(models.Model):
@property
def enabled_images(self):
try:
return self.prefetched_enabled_images
except AttributeError:
return self.images.filter(enabled=True)
}}}

Then I can write `Product.objects.prefetch_related(Prefetch("images",
ProductImage.objects.filter(enabled=True),
to_attr="prefetched_enabled_images"))` and now accessing
`product.enabled_images` won't hit the database, but I think it's uglier.
--
Ticket URL: <https://code.djangoproject.com/ticket/37201>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#37201: Prefetch with to_attr silently ignored if matching property exists
-------------------------------------+-------------------------------------

Reporter: dc-strahlkraft | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: 6.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* stage: Unreviewed => Accepted

Comment:

It's likely something we should document explicitly but the mere presence
of an attribute conflicting with a specified `to_attr` is assumed to
denote that the relationship is already fetched and this it is ignore by
design.

I guess we could adapt the logic added in #26916 for `cached_property` to
allow any `property` with a setter (not only `cached_property`) and fail
loudly if pointed at one that doesn't have one.

It the mean time your best bet is likely to use `cached_property` instead
of `property`.
--
Ticket URL: <https://code.djangoproject.com/ticket/37201#comment:1>

[Django]

#37201: Prefetch with to_attr silently ignored if matching property exists
-------------------------------------+-------------------------------------

Reporter: dc-strahlkraft | Owner: zky
Type: Bug | Status: assigned

Component: Database layer | Version: 6.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by zky):

* owner: (none) => zky
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/37201#comment:2>

[Django]

#37201: Prefetch with to_attr silently ignored if matching property exists
-------------------------------------+-------------------------------------
Reporter: dc-strahlkraft | Owner: zky
Type: Bug | Status: assigned
Component: Database layer | Version: 6.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Simon Charette):

Re-hashing this issue I wonder if it will even be possible to implement
support for `property` when `callable(prop.fset)` as the prefetching
algorithm requires a form of bookeeping for previously fetched attributes.

In the case where `to_attr` is not used the already-prefetched sentinel is
the presence of the relationship name in `_prefetched_objects_cache` and
if `to_attr` is used to target a `cached_property` it's the presence of
the `to_attr` in `instance.__dict__`. The latter works because
`cached_property` is known to use `__dict__` to store a value assigned to
it but `property` implementations are opaque to Django so it can't assume
the same.

The various approaches I can think of are

1. Adjust the prefetching logic to stop doing already-prefetched
bookkeeping using attribute presence as alluded to in
ticket:26916#comment:1
2. Special case `property` by setting an extra attribute (e.g.
`_prefetched_to_attr_{name} = True`) to denote already-fetched
3. Fail loudly when detecting a `property` (and potentially any descriptor
not recognized by Django)
--
Ticket URL: <https://code.djangoproject.com/ticket/37201#comment:3>