Opened 11 years ago

Closed 2 years ago

#21204 closed Bug (fixed)

Query.defer() failure - deferred columns calculated per table instead per alias

Reported by: Anssi Kääriäinen Owned by: Simon Charette
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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

Change History (6)

comment:1 by Simon Charette, 10 years ago

Has patch: set
Owner: changed from nobody to Simon Charette
Patch needs improvement: set
Status: newassigned

I started working on this here.

comment:2 by Tim Graham, 9 years ago

While the patch has gone quite stale, the test there still fails as of c6c00fbfbb659de4beaad3c612c271ac74f892a7.

comment:3 by Mariusz Felisiak, 3 years ago

Owner: Simon Charette removed
Status: assignednew

comment:4 by Simon Charette, 2 years ago

Owner: set to Simon Charette
Patch needs improvement: unset
Status: newassigned

comment:5 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

In 5d12650e:

Refs #21204 -- Added more QuerySet.defer()/only() tests for invalid fields.

comment:6 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

Resolution: fixed
Status: assignedclosed

In b3db6c8d:

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.

Note: See TracTickets for help on using tickets.
Back to Top