[Django] #23001: Annotation breaks with deferred fields and select_related
Django
----------------------------------------------+----------------------
Reporter: smeatonj | Owner: smeatonj
Type: Uncategorized | Status: new
Component: Database layer (models, ORM) | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+----------------------
I've discovered a bug that I think was introduced by:
https://github.com/django/django/commit/0b6f05ede648ce62a5c91c7c38a0a362711f0656
The problem manifests as annotations being assigned an incorrect value
when there are deferred fields and select_related is in use.
Passing Test:
{{{
def test_annotate_defer(self):
qs = Book.objects.annotate(
page_sum=Sum("pages")).defer('name').filter(pk=1)
rows = [
(1, "159059725", 447, "The Definitive Guide to Django: Web
Development Done Right")
]
self.assertQuerysetEqual(
qs.order_by('pk'), rows,
lambda r: (r.id, r.isbn, r.page_sum, r.name)
)
}}}
Failing Test:
{{{
def test_annotate_defer_select_related(self):
qs = Book.objects.select_related('contact').annotate(
page_sum=Sum("pages")).defer('name').filter(pk=1)
rows = [
(1, "159059725", 447, "Adrian Holovaty",
"The Definitive Guide to Django: Web Development Done Right")
]
self.assertQuerysetEqual(
qs.order_by('pk'), rows,
lambda r: (r.id, r.isbn, r.page_sum, r.contact.name, r.name)
)
}}}
The problem is in the iterator method of django.db.models.query, which
results in the aggregate_start variable being off by len(deferred):
{{{
if load_fields and not fill_cache:
# Some fields have been deferred, so we have to initialize
# via keyword arguments.
skip = set()
init_list = []
for field in fields:
if field.name not in load_fields:
skip.add(field.attname)
else:
init_list.append(field.attname)
model_cls = deferred_class_factory(self.model, skip)
else:
model_cls = self.model
init_list = [f.attname for f in fields]
}}}
fill_cache is set to true when select_related is in use, which skips to
the else block. The else block doesn't take deferred fields into account
like the if block does. I'll attempt to patch this by including similar
logic in the else block (skipping deferred fields), but I'm not yet sure
what else that might affect.
--
Ticket URL: <https://code.djangoproject.com/ticket/23001>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: smeatonj | Owner: smeatonj
Type: Uncategorized | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by smeatonj):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
Both tests pass on the 1.7 branch.
--
Ticket URL: <https://code.djangoproject.com/ticket/23001#comment:1>
Django
Reporter: smeatonj | Owner: smeatonj
Type: Uncategorized | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by smeatonj):
* has_patch: 0 => 1
Comment:
https://github.com/django/django/pull/2903
--
Ticket URL: <https://code.djangoproject.com/ticket/23001#comment:2>
Django
Reporter: smeatonj | Owner: smeatonj
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) | Resolution:
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* type: Uncategorized => Bug
* stage: Unreviewed => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/23001#comment:3>
Django
Reporter: smeatonj | Owner: jarshwah
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by jarshwah):
* owner: smeatonj => jarshwah
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/23001#comment:4>
Django
Reporter: smeatonj | Owner: jarshwah
Component: Database layer | Version: master
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Anssi Kääriäinen <akaariai@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"5b0375ec3e7473fcc29c21003fef80c0d0be183f"]:
{{{
#!CommitTicketReference repository=""
revision="5b0375ec3e7473fcc29c21003fef80c0d0be183f"
Fixed #23001 -- Fixed mixing defer and annotations
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23001#comment:5>