From ecadf675690c2cc9375e9af70ebd1528841eae76 Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Tue, 3 Feb 2009 11:07:21 +0000 Subject: [PATCH] Fixed #10127 -- Corrected (no, really, this time!) the way the select_related() cache is populated when annotations are also contained in the query. Thanks to Sylvain Pasche for the report and test case. git-svn-id: http://code.djangoproject.com/svn/django/trunk@9808 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/db/models/query.py | 6 ++++-- .../aggregation/fixtures/initial_data.json | 8 ++++++- tests/modeltests/aggregation/models.py | 15 ++++++------- .../fixtures/initial_data.json | 8 ++++++- .../aggregation_regress/models.py | 21 ++++++++++--------- 5 files changed, 37 insertions(+), 21 deletions(-) diff --git a/django/db/models/query.py b/django/db/models/query.py index aa2034919c701..e33a858a814a9 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -281,7 +281,8 @@ def iterator(self): for row in self.query.results_iter(): if fill_cache: obj, _ = get_cached_row(self.model, row, - index_start, max_depth, requested=requested) + index_start, max_depth, + requested=requested, offset=len(aggregate_select)) else: # omit aggregates in object creation obj = self.model(*row[index_start:aggregate_start]) @@ -898,7 +899,7 @@ def iterator(self): def get_cached_row(klass, row, index_start, max_depth=0, cur_depth=0, - requested=None): + requested=None, offset=0): """ Helper function that recursively returns an object with the specified related attributes already populated. @@ -915,6 +916,7 @@ def get_cached_row(klass, row, index_start, max_depth=0, cur_depth=0, obj = None else: obj = klass(*fields) + index_end += offset for f in klass._meta.fields: if not select_related_descend(f, restricted, requested): continue diff --git a/tests/modeltests/aggregation/fixtures/initial_data.json b/tests/modeltests/aggregation/fixtures/initial_data.json index 0ff26d9ed3a38..a0021001e7802 100644 --- a/tests/modeltests/aggregation/fixtures/initial_data.json +++ b/tests/modeltests/aggregation/fixtures/initial_data.json @@ -49,6 +49,7 @@ "price": "30.00", "rating": 4.5, "authors": [1, 2], + "contact": 1, "pages": 447, "pubdate": "2007-12-6" } @@ -63,6 +64,7 @@ "price": "23.09", "rating": 3.0, "authors": [3], + "contact": 3, "pages": 528, "pubdate": "2008-3-3" } @@ -77,6 +79,7 @@ "price": "29.69", "rating": 4.0, "authors": [4], + "contact": 4, "pages": 300, "pubdate": "2008-6-23" } @@ -91,6 +94,7 @@ "price": "29.69", "rating": 4.0, "authors": [5, 6, 7], + "contact": 5, "pages": 350, "pubdate": "2008-11-3" } @@ -105,6 +109,7 @@ "price": "82.80", "rating": 4.0, "authors": [8, 9], + "contact": 8, "pages": 1132, "pubdate": "1995-1-15" } @@ -119,6 +124,7 @@ "price": "75.00", "rating": 5.0, "authors": [8], + "contact": 8, "pages": 946, "pubdate": "1991-10-15" } @@ -195,7 +201,7 @@ "fields": { "age": 37, "friends": [6, 7], - "name": "Jeffrey Forcier " + "name": "Jeffrey Forcier" } }, { diff --git a/tests/modeltests/aggregation/models.py b/tests/modeltests/aggregation/models.py index a94c475543ef2..8537d0bd66a47 100644 --- a/tests/modeltests/aggregation/models.py +++ b/tests/modeltests/aggregation/models.py @@ -28,6 +28,7 @@ class Book(models.Model): rating = models.FloatField() price = models.DecimalField(decimal_places=2, max_digits=6) authors = models.ManyToManyField(Author) + contact = models.ForeignKey(Author, related_name='book_contact_set') publisher = models.ForeignKey(Publisher) pubdate = models.DateField() @@ -180,7 +181,7 @@ class Clues(models.Model): # Count the number of books written by each author >>> authors = Author.objects.annotate(num_books=Count('book')) >>> sorted([(a.name, a.num_books) for a in authors]) -[(u'Adrian Holovaty', 1), (u'Brad Dayley', 1), (u'Jacob Kaplan-Moss', 1), (u'James Bennett', 1), (u'Jeffrey Forcier ', 1), (u'Paul Bissex', 1), (u'Peter Norvig', 2), (u'Stuart Russell', 1), (u'Wesley J. Chun', 1)] +[(u'Adrian Holovaty', 1), (u'Brad Dayley', 1), (u'Jacob Kaplan-Moss', 1), (u'James Bennett', 1), (u'Jeffrey Forcier', 1), (u'Paul Bissex', 1), (u'Peter Norvig', 2), (u'Stuart Russell', 1), (u'Wesley J. Chun', 1)] # On OneToMany Relationships @@ -201,7 +202,7 @@ class Clues(models.Model): # Calling values on a queryset that has annotations returns the output # as a dictionary >>> Book.objects.filter(pk=1).annotate(mean_age=Avg('authors__age')).values() -[{'rating': 4.5, 'isbn': u'159059725', 'name': u'The Definitive Guide to Django: Web Development Done Right', 'pubdate': datetime.date(2007, 12, 6), 'price': Decimal("30..."), 'id': 1, 'publisher_id': 1, 'pages': 447, 'mean_age': 34.5}] +[{'rating': 4.5, 'isbn': u'159059725', 'name': u'The Definitive Guide to Django: Web Development Done Right', 'pubdate': datetime.date(2007, 12, 6), 'price': Decimal("30..."), 'contact_id': 1, 'id': 1, 'publisher_id': 1, 'pages': 447, 'mean_age': 34.5}] >>> Book.objects.filter(pk=1).annotate(mean_age=Avg('authors__age')).values('pk', 'isbn', 'mean_age') [{'pk': 1, 'isbn': u'159059725', 'mean_age': 34.5}] @@ -214,7 +215,7 @@ class Clues(models.Model): # An empty values() call before annotating has the same effect as an # empty values() call after annotating >>> Book.objects.filter(pk=1).values().annotate(mean_age=Avg('authors__age')) -[{'rating': 4.5, 'isbn': u'159059725', 'name': u'The Definitive Guide to Django: Web Development Done Right', 'pubdate': datetime.date(2007, 12, 6), 'price': Decimal("30..."), 'id': 1, 'publisher_id': 1, 'pages': 447, 'mean_age': 34.5}] +[{'rating': 4.5, 'isbn': u'159059725', 'name': u'The Definitive Guide to Django: Web Development Done Right', 'pubdate': datetime.date(2007, 12, 6), 'price': Decimal("30..."), 'contact_id': 1, 'id': 1, 'publisher_id': 1, 'pages': 447, 'mean_age': 34.5}] # Calling annotate() on a ValuesQuerySet annotates over the groups of # fields to be selected by the ValuesQuerySet. @@ -231,7 +232,7 @@ class Clues(models.Model): >>> len(authors) 9 >>> sorted([(a.name, a.friends__age__avg) for a in authors]) -[(u'Adrian Holovaty', 32.0), (u'Brad Dayley', None), (u'Jacob Kaplan-Moss', 29.5), (u'James Bennett', 34.0), (u'Jeffrey Forcier ', 27.0), (u'Paul Bissex', 31.0), (u'Peter Norvig', 46.0), (u'Stuart Russell', 57.0), (u'Wesley J. Chun', 33.6...)] +[(u'Adrian Holovaty', 32.0), (u'Brad Dayley', None), (u'Jacob Kaplan-Moss', 29.5), (u'James Bennett', 34.0), (u'Jeffrey Forcier', 27.0), (u'Paul Bissex', 31.0), (u'Peter Norvig', 46.0), (u'Stuart Russell', 57.0), (u'Wesley J. Chun', 33.6...)] # The Count aggregation function allows an extra parameter: distinct. @@ -268,9 +269,9 @@ class Clues(models.Model): # Lets add a publisher to test the different possibilities for filtering >>> p = Publisher(name='Expensive Publisher', num_awards=0) >>> p.save() ->>> Book(name='ExpensiveBook1', pages=1, isbn='111', rating=3.5, price=Decimal("1000"), publisher=p, pubdate=date(2008,12,1)).save() ->>> Book(name='ExpensiveBook2', pages=1, isbn='222', rating=4.0, price=Decimal("1000"), publisher=p, pubdate=date(2008,12,2)).save() ->>> Book(name='ExpensiveBook3', pages=1, isbn='333', rating=4.5, price=Decimal("35"), publisher=p, pubdate=date(2008,12,3)).save() +>>> Book(name='ExpensiveBook1', pages=1, isbn='111', rating=3.5, price=Decimal("1000"), publisher=p, contact_id=1, pubdate=date(2008,12,1)).save() +>>> Book(name='ExpensiveBook2', pages=1, isbn='222', rating=4.0, price=Decimal("1000"), publisher=p, contact_id=1, pubdate=date(2008,12,2)).save() +>>> Book(name='ExpensiveBook3', pages=1, isbn='333', rating=4.5, price=Decimal("35"), publisher=p, contact_id=1, pubdate=date(2008,12,3)).save() # Publishers that have: diff --git a/tests/regressiontests/aggregation_regress/fixtures/initial_data.json b/tests/regressiontests/aggregation_regress/fixtures/initial_data.json index 7c5d62c2c630d..9fb88572b09b4 100644 --- a/tests/regressiontests/aggregation_regress/fixtures/initial_data.json +++ b/tests/regressiontests/aggregation_regress/fixtures/initial_data.json @@ -49,6 +49,7 @@ "price": "30.00", "rating": 4.5, "authors": [1, 2], + "contact": 1, "pages": 447, "pubdate": "2007-12-6" } @@ -63,6 +64,7 @@ "price": "23.09", "rating": 3.0, "authors": [3], + "contact": 3, "pages": 528, "pubdate": "2008-3-3" } @@ -77,6 +79,7 @@ "price": "29.69", "rating": 4.0, "authors": [4], + "contact": 4, "pages": 300, "pubdate": "2008-6-23" } @@ -91,6 +94,7 @@ "price": "29.69", "rating": 4.0, "authors": [5, 6, 7], + "contact": 5, "pages": 350, "pubdate": "2008-11-3" } @@ -105,6 +109,7 @@ "price": "82.80", "rating": 4.0, "authors": [8, 9], + "contact": 8, "pages": 1132, "pubdate": "1995-1-15" } @@ -119,6 +124,7 @@ "price": "75.00", "rating": 5.0, "authors": [8], + "contact": 8, "pages": 946, "pubdate": "1991-10-15" } @@ -195,7 +201,7 @@ "fields": { "age": 37, "friends": [6, 7], - "name": "Jeffrey Forcier " + "name": "Jeffrey Forcier" } }, { diff --git a/tests/regressiontests/aggregation_regress/models.py b/tests/regressiontests/aggregation_regress/models.py index 3e5f873472567..24de920290db7 100644 --- a/tests/regressiontests/aggregation_regress/models.py +++ b/tests/regressiontests/aggregation_regress/models.py @@ -29,6 +29,7 @@ class Book(models.Model): rating = models.FloatField() price = models.DecimalField(decimal_places=2, max_digits=6) authors = models.ManyToManyField(Author) + contact = models.ForeignKey(Author, related_name='book_contact_set') publisher = models.ForeignKey(Publisher) pubdate = models.DateField() @@ -80,19 +81,19 @@ def __unicode__(self): # Annotations get combined with extra select clauses >>> sorted(Book.objects.all().annotate(mean_auth_age=Avg('authors__age')).extra(select={'manufacture_cost' : 'price * .5'}).get(pk=2).__dict__.items()) -[('id', 2), ('isbn', u'067232959'), ('manufacture_cost', ...11.545...), ('mean_auth_age', 45.0), ('name', u'Sams Teach Yourself Django in 24 Hours'), ('pages', 528), ('price', Decimal("23.09")), ('pubdate', datetime.date(2008, 3, 3)), ('publisher_id', 2), ('rating', 3.0)] +[('contact_id', 3), ('id', 2), ('isbn', u'067232959'), ('manufacture_cost', ...11.545...), ('mean_auth_age', 45.0), ('name', u'Sams Teach Yourself Django in 24 Hours'), ('pages', 528), ('price', Decimal("23.09")), ('pubdate', datetime.date(2008, 3, 3)), ('publisher_id', 2), ('rating', 3.0)] # Order of the annotate/extra in the query doesn't matter >>> sorted(Book.objects.all().extra(select={'manufacture_cost' : 'price * .5'}).annotate(mean_auth_age=Avg('authors__age')).get(pk=2).__dict__.items()) -[('id', 2), ('isbn', u'067232959'), ('manufacture_cost', ...11.545...), ('mean_auth_age', 45.0), ('name', u'Sams Teach Yourself Django in 24 Hours'), ('pages', 528), ('price', Decimal("23.09")), ('pubdate', datetime.date(2008, 3, 3)), ('publisher_id', 2), ('rating', 3.0)] +[('contact_id', 3), ('id', 2), ('isbn', u'067232959'), ('manufacture_cost', ...11.545...), ('mean_auth_age', 45.0), ('name', u'Sams Teach Yourself Django in 24 Hours'), ('pages', 528), ('price', Decimal("23.09")), ('pubdate', datetime.date(2008, 3, 3)), ('publisher_id', 2), ('rating', 3.0)] # Values queries can be combined with annotate and extra >>> sorted(Book.objects.all().annotate(mean_auth_age=Avg('authors__age')).extra(select={'manufacture_cost' : 'price * .5'}).values().get(pk=2).items()) -[('id', 2), ('isbn', u'067232959'), ('manufacture_cost', ...11.545...), ('mean_auth_age', 45.0), ('name', u'Sams Teach Yourself Django in 24 Hours'), ('pages', 528), ('price', Decimal("23.09")), ('pubdate', datetime.date(2008, 3, 3)), ('publisher_id', 2), ('rating', 3.0)] +[('contact_id', 3), ('id', 2), ('isbn', u'067232959'), ('manufacture_cost', ...11.545...), ('mean_auth_age', 45.0), ('name', u'Sams Teach Yourself Django in 24 Hours'), ('pages', 528), ('price', Decimal("23.09")), ('pubdate', datetime.date(2008, 3, 3)), ('publisher_id', 2), ('rating', 3.0)] # The order of the values, annotate and extra clauses doesn't matter >>> sorted(Book.objects.all().values().annotate(mean_auth_age=Avg('authors__age')).extra(select={'manufacture_cost' : 'price * .5'}).get(pk=2).items()) -[('id', 2), ('isbn', u'067232959'), ('manufacture_cost', ...11.545...), ('mean_auth_age', 45.0), ('name', u'Sams Teach Yourself Django in 24 Hours'), ('pages', 528), ('price', Decimal("23.09")), ('pubdate', datetime.date(2008, 3, 3)), ('publisher_id', 2), ('rating', 3.0)] +[('contact_id', 3), ('id', 2), ('isbn', u'067232959'), ('manufacture_cost', ...11.545...), ('mean_auth_age', 45.0), ('name', u'Sams Teach Yourself Django in 24 Hours'), ('pages', 528), ('price', Decimal("23.09")), ('pubdate', datetime.date(2008, 3, 3)), ('publisher_id', 2), ('rating', 3.0)] # A values query that selects specific columns reduces the output >>> sorted(Book.objects.all().annotate(mean_auth_age=Avg('authors__age')).extra(select={'price_per_page' : 'price / pages'}).values('name').get(pk=1).items()) @@ -119,17 +120,17 @@ def __unicode__(self): >>> Book.objects.all().aggregate(num_authors=Count('foo')) Traceback (most recent call last): ... -FieldError: Cannot resolve keyword 'foo' into field. Choices are: authors, id, isbn, name, pages, price, pubdate, publisher, rating, store +FieldError: Cannot resolve keyword 'foo' into field. Choices are: authors, contact, id, isbn, name, pages, price, pubdate, publisher, rating, store >>> Book.objects.all().annotate(num_authors=Count('foo')) Traceback (most recent call last): ... -FieldError: Cannot resolve keyword 'foo' into field. Choices are: authors, id, isbn, name, pages, price, pubdate, publisher, rating, store +FieldError: Cannot resolve keyword 'foo' into field. Choices are: authors, contact, id, isbn, name, pages, price, pubdate, publisher, rating, store >>> Book.objects.all().annotate(num_authors=Count('authors__id')).aggregate(Max('foo')) Traceback (most recent call last): ... -FieldError: Cannot resolve keyword 'foo' into field. Choices are: authors, id, isbn, name, pages, price, pubdate, publisher, rating, store, num_authors +FieldError: Cannot resolve keyword 'foo' into field. Choices are: authors, contact, id, isbn, name, pages, price, pubdate, publisher, rating, store, num_authors # Old-style count aggregations can be mixed with new-style >>> Book.objects.annotate(num_authors=Count('authors')).count() @@ -149,7 +150,7 @@ def __unicode__(self): # Regression for #10064: select_related() plays nice with aggregates >>> Book.objects.select_related('publisher').annotate(num_authors=Count('authors')).values()[0] -{'rating': 4.0, 'isbn': u'013790395', 'name': u'Artificial Intelligence: A Modern Approach', 'pubdate': datetime.date(1995, 1, 15), 'price': Decimal("82.8..."), 'id': 5, 'num_authors': 2, 'publisher_id': 3, 'pages': 1132} +{'rating': 4.0, 'isbn': u'013790395', 'name': u'Artificial Intelligence: A Modern Approach', 'pubdate': datetime.date(1995, 1, 15), 'price': Decimal("82.8..."), 'contact_id': 8, 'id': 5, 'num_authors': 2, 'publisher_id': 3, 'pages': 1132} # Regression for #10010: exclude on an aggregate field is correctly negated >>> len(Book.objects.annotate(num_authors=Count('authors'))) @@ -196,8 +197,8 @@ def __unicode__(self): # Regression for #10127 - Empty select_related() works with annotate >>> books = Book.objects.all().filter(rating__lt=4.5).select_related().annotate(Avg('authors__age')) ->>> sorted([(b.name, b.authors__age__avg) for b in books]) -[(u'Artificial Intelligence: A Modern Approach', 51.5), (u'Practical Django Projects', 29.0), (u'Python Web Development with Django', 30.3...), (u'Sams Teach Yourself Django in 24 Hours', 45.0)] +>>> sorted([(b.name, b.authors__age__avg, b.publisher.name, b.contact.name) for b in books]) +[(u'Artificial Intelligence: A Modern Approach', 51.5, u'Prentice Hall', u'Peter Norvig'), (u'Practical Django Projects', 29.0, u'Apress', u'James Bennett'), (u'Python Web Development with Django', 30.3..., u'Prentice Hall', u'Jeffrey Forcier'), (u'Sams Teach Yourself Django in 24 Hours', 45.0, u'Sams', u'Brad Dayley')] """ }