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 Annabelle Wiegart)

.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}]>

The bug was discussed in ​PR21803 for #36945.

Change History (11)

comment:1 by Annabelle Wiegart, 6 weeks ago

Description: modified (diff)

comment:2 by Annabelle Wiegart, 6 weeks ago

Description: modified (diff)

comment:3 by Zubair Hassan, 6 weeks ago

​DryORM Fiddle
it is workig with value lookup.

comment:4 by Yassin Bahri, 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 Yassin Bahri, 6 weeks ago

Owner: set to Yassin Bahri
Status: new → assigned

comment:6 by Yassin Bahri, 6 weeks ago

Has patch: set

comment:7 by Annabelle Wiegart, 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.

Last edited 6 weeks ago by Annabelle Wiegart (previous) (diff)

in reply to:  7 comment:8 by Yassin Bahri, 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 Jacob Walls, 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 Jacob Walls, 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?

comment:11 by Sarah Boyce, 40 hours ago

Triage Stage: Unreviewed → Accepted

That sounds reasonable to me

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