Fail loudly and clearly when an Annotation alias matches a field name.

[donspaulding]

Continued from ​a django-users thread

Given the following two models:

class Author(models.Model):
  last_updated = models.DateField()

class Book(models.Model):
  author = models.ForeignKey(Author, related_name='books')
  pubdate = models.DateField()

The following annotation query will produce unexpected results:

authors = Author.objects.annotate(last_updated=Max('books__pubdate'))

And it may not raise an error until you combine it with other parts of the queryset api, like order_by:

authors = Author.objects.annotate(last_updated=Max('books__pubdate')).order_by('last_updated')

While it's true that these invalid queries are the developer's fault, it's not at all obvious what's gone wrong. Unless there is some valid case for clobbering your own namespace, I suggest that instead of leaving this behavior defined by the particular DB and query, an Exception be raised to let the developer know they've made a mistake before it hits the SQL level.

[Russell Keith-Magee]

Agreed - an attempt to annotate using a name that already exists on the model should raise an exception at time of query construction.

[Carl Meyer]

#14373 reported this same issue, with the additional factor that under certain (not yet entirely clear) circumstances with m2m fields, this can lead to data loss. Bumping to 1.3 milestone.

[Carl Meyer]

updated patch

[Carl Meyer]

New patch updates tests to apply post-doctest-removal, and modifies the duplicate-alias error message to be a bit more informative (tells you what alias is a dupe).

[Russell Keith-Magee]

Looks good to me.

[Carl Meyer]

Doh. Forgot fixes ref in commit message; how embarrassing. Fixed in r14092, backported to 1.2.X in r14093.

[Carl Meyer]

(In [14116]) Refs #11256 -- Extended the annotation field name conflict check to cover m2ms and reverse related descriptors as well. This is needed to actually cover the case raised by #14373.

[Carl Meyer]

(In [14117]) [1.2.X] Refs #11256 -- Extended the annotation field name conflict check to cover m2ms and reverse related descriptors as well. This is needed to actually cover the case raised by #14373. Backport of [14116].

[Jacob]

Milestone 1.3 deleted