[Django] #21204: Query.defer() failure - deferred columns calculated per table instead per alias
Django
alias
-------------------------------------+-------------------------------------
Reporter: akaariai | Owner: nobody
Type: Bug | Status: new
Component: Database | Version: master
layer (models, ORM) | Keywords:
Severity: Normal | Has patch: 0
Triage Stage: Accepted | Needs tests: 0
Needs documentation: 0 | Easy pickings: 0
Patch needs improvement: 0 |
UI/UX: 0 |
-------------------------------------+-------------------------------------
In sql.compiler the deferred columns are calculated per table. This
doesn't work correctly as it is possible to have the same table multiple
times in the query. For example (using models from
tests/queries/models.py):
{{{
qs = Tag.objects.select_related(
'parent', 'parent__parent'
).defer(
'parent__name', 'parent__parent__category'
)
}}}
Currently this causes both name and category to be deferred for the base
model, its parent, and parent's parent. The correct interpretation is that
for base model all columns are loaded, for parent category is loaded, name
is deferred and for parent's parent name is loaded and category is
deferred.
The proper solution is to key the only_load dictionary by alias instead of
db_table. This doesn't look to be particularly easy case to solve, as both
only_load's construction and usage are somewhat complex.
The bug has likely existed for as long as defer has existed. I'll mark
this as master as I don't see a reason why this must be backpatched. The
user-visible result of this bug is that more queries than expected are
executed in some rare situations, so this isn't too severe.
Failing test case at:
https://github.com/akaariai/django/compare/only_load_bug
--
Ticket URL: <https://code.djangoproject.com/ticket/21204>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
alias
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 1
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by charettes):
* owner: nobody => charettes
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
* status: new => assigned
Comment:
I started working on this
[https://github.com/charettes/django/compare/ticket-21204-deferr-alias
here].
--
Ticket URL: <https://code.djangoproject.com/ticket/21204#comment:1>
Django
alias
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by timgraham):
While the patch has gone quite stale, the test there still fails as of
c6c00fbfbb659de4beaad3c612c271ac74f892a7.
--
Ticket URL: <https://code.djangoproject.com/ticket/21204#comment:2>
Django
alias
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: Simon Charette => (none)
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/21204#comment:3>
Django
alias
-------------------------------------+-------------------------------------
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: (none) => Simon Charette
* needs_better_patch: 1 => 0
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/21204#comment:4>
Django
alias
-------------------------------------+-------------------------------------
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"5d12650ed9269acb3cba97fd70e8df2e35a55a54" 5d12650e]:
{{{
#!CommitTicketReference repository=""
revision="5d12650ed9269acb3cba97fd70e8df2e35a55a54"
Refs #21204 -- Added more QuerySet.defer()/only() tests for invalid
fields.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21204#comment:5>
Django
alias
-------------------------------------+-------------------------------------
| Charette
Component: Database layer | Version: dev
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"b3db6c8dcb5145f7d45eff517bcd96460475c879" b3db6c8d]:
{{{
#!CommitTicketReference repository=""
revision="b3db6c8dcb5145f7d45eff517bcd96460475c879"
Fixed #21204 -- Tracked field deferrals by field instead of models.
This ensures field deferral works properly when a model is involved
more than once in the same query with a distinct deferral mask.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21204#comment:6>