Opened 2 months ago

Last modified 7 days 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, Abhay Singh Jadaun 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 (13)

comment:1 by Annabelle Wiegart, 2 months ago

Description: modified (diff)

comment:2 by Annabelle Wiegart, 2 months ago

Description: modified (diff)

comment:3 by Zubair Hassan, 2 months ago

​DryORM Fiddle
it is workig with value lookup.

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

Owner: set to Yassin Bahri
Status: new → assigned

comment:6 by Yassin Bahri, 2 months ago

Has patch: set

comment:7 by Annabelle Wiegart, 2 months 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 2 months ago by Annabelle Wiegart (previous) (diff)

in reply to:  7 comment:8 by Yassin Bahri, 2 months 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, 5 weeks 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, 5 weeks 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, 3 weeks ago

Triage Stage: Unreviewed → Accepted

That sounds reasonable to me

comment:12 by Abhay Singh Jadaun, 7 days ago

Hi Yassin Bahri,
I would like to work on this ticket, if you are currently not working to write the patch, can I assign it to myself ?
Actually I am intrested in ORM related tickets.

comment:13 by Abhay Singh Jadaun, 7 days ago

Cc: Abhay Singh Jadaun added
Note: See TracTickets for help on using tickets.
Back to Top