Opened 6 weeks ago
Last modified 40 hours ago
#37305 assigned Cleanup/optimization
Document that annotation aliases can shadow lookups and transforms
| Reported by: | Annabelle Wiegart | Owned by: | Yassin Bahri |
|---|---|---|---|
| Component: | Documentation | Version: | 6.1 |
| Severity: | Normal | Keywords: | annotate, alias |
| Cc: | Annabelle Wiegart | Triage Stage: | Accepted |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description (last modified by )
.annotate() raises a ValueError when an annotation alias conflicts with a field on the annotated model. However, this is not the case for legitimate lookups and transforms, as demonstrated in this DryORM fiddle.
Here is the reproducer:
from django.db import models from django.db.models.expressions import Value from django.contrib.auth.models import User class Person(models.Model): name = models.CharField(max_length=100) creator = models.ForeignKey(User, models.CASCADE, null=True) creation_date = models.DateField(auto_now=True) def run(): admin = User.objects.create(username="admin") Person.objects.create(creator=admin, name="Violet") qs1 = Person.objects.all() qs2 = Person.objects.annotate(creator__username=Value(1)) qs3 = Person.objects.annotate(creation_date__year=Value(1)) # raises ValueError # qs4 = Person.objects.annotate(creator=Value(1)) # qs5 = Person.objects.annotate(creation_date=Value(1)) # lookup without shadowing print(qs1.values("creator__username")) # lookup with shadowing print(qs2.values("creator__username")) # transform without shadowing print(qs1.values("creation_date__year")) # transform with shadowing print(qs3.values("creation_date__year"))
Output:
<QuerySet [{'creator__username': 'admin'}]> <QuerySet [{'creator__username': 1}]> <QuerySet [{'creation_date__year': 2026}]> <QuerySet [{'creation_date__year': 1}]>
Change History (11)
comment:1 by , 6 weeks ago
| Description: | modified (diff) |
|---|
comment:2 by , 6 weeks ago
| Description: | modified (diff) |
|---|
comment:3 by , 6 weeks ago
comment:4 by , 6 weeks ago
| Triage Stage: | Unreviewed → Accepted |
|---|
I reproduced this on current main.
The issue affects both relationship lookups and transforms when an annotation alias uses the same LOOKUP_SEP path.
Example regression tests using the existing annotations test app:
def test_annotation_alias_shadows_lookup_in_values(self):
qs = Book.objects.annotate(publisher__name=Value("shadowed")).values(
"publisher__name"
)
self.assertIn(
{"publisher__name": self.p1.name},
qs,
)
def test_annotation_alias_shadows_transform_in_values(self):
qs = Book.objects.annotate(pubdate__year=Value(1)).values("pubdate__year")
self.assertIn(
{"pubdate__year": self.b1.pubdate.year},
qs,
)
Both tests currently fail.
For the relationship lookup case, Django returns the annotation value instead of resolving the real join path:
AssertionError: {'publisher__name': 'Apress'} not found in <QuerySet [
{'publisher__name': 'shadowed'},
...
]>
For the transform case, Django returns the annotation value instead of resolving the date transform:
AssertionError: {'pubdate__year': 2007} not found in <QuerySet [
{'pubdate__year': 1},
...
]>
I also checked that filtering can still intentionally use an annotation alias containing LOOKUP_SEP:
Book.objects.annotate(publisher__name=Value("shadowed")).filter(
publisher__name="shadowed"
)
So I think the issue is real, but the fix should probably be careful: either reject annotation aliases that shadow valid lookup/transform paths, or otherwise make values() / related name-resolution paths avoid this surprising shadowing. I would avoid a broad ban on all aliases containing __ unless that is the preferred direction, since existing code may rely on such aliases.
comment:5 by , 6 weeks ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
comment:6 by , 6 weeks ago
| Has patch: | set |
|---|
follow-up: 8 comment:7 by , 6 weeks ago
I would also avoid a broad ban on aliases containing __, especially since default aliases may contain them. This has also been discussed in the context of #36945: https://github.com/django/django/pull/21803#discussion_r3823261022.
comment:8 by , 6 weeks ago
Replying to Annabelle Wiegart:
I would also avoid a broad ban on aliases containing
__, especially since default aliases may contain them. This has also been discussed in the context of #36945: https://github.com/django/django/pull/21803#discussion_r3823261022.
Agreed. The patch doesn’t reject aliases merely because they contain __. It checks whether the alias resolves to an actual lookup, transform, or related field path on the model. Non-resolving aliases remain allowed, including Django-generated default aggregate aliases such as authors__count, which is covered by a regression test.
comment:9 by , 13 days ago
I noticed this proposal is in tension with #37305, which asks for the restriction on shadowing to be lifted entirely. We should work out which of these tickets to work on and close the other one.
comment:10 by , 12 days ago
| Component: | Database layer (models, ORM) → Documentation |
|---|---|
| Has patch: | unset |
| Summary: | Annotation aliases can shadow lookups and transforms → Document that annotation aliases can shadow lookups and transforms |
| Triage Stage: | Accepted → Unreviewed |
| Type: | Bug → Cleanup/optimization |
I just noticed that shadowing lookups & transforms via an annotation is documented, perhaps as a workaround for not having a transform registered.
Given that, I'm unsure we should change any behavior here. We would be imposing a breaking change to support a (relatively rare) user-controlled alias use case. Instead, we could just document this as a caveat under a new section about user-controlled aliases. ("If you expose the ability for users to control annotation aliases, be aware that they can shadow lookups and transforms, and be careful not to reference any such lookups or transforms ...")
How does that sound?
DryORM Fiddle
it is workig with
valuelookup.