[Django] #32704: Unexpected behavior of .defer() and .only()
Django
-------------------------------------+-------------------------------------
Reporter: Manuel | Owner: nobody
Baclet |
Type: Bug | Status: new
Component: Database | Version: 3.1
layer (models, ORM) |
Severity: Normal | Keywords: defer only
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Considering a simple `Company` model with four fields: `id`, `name`,
`trade_number` and `country`. If we evaluate a queryset containing a
`.defer()` following a `.only()`, the generated sql query selects
unexpected fields. For example:
{{{#!python
Company.objects.only("name").defer("name")
}}}
loads all the fields with the following query:
{{{#!sql
SELECT "company"."id", "company"."name", "company"."trade_number",
"company"."country" FROM "company"
}}}
and
{{{#!python
Company.objects.only("name").defer("name").defer("country")
}}}
also loads all the fields with the same query:
{{{#!sql
SELECT "company"."id", "company"."name", "company"."trade_number",
"company"."country" FROM "company"
}}}
In those two cases, i would expect the sql query to be:
{{{#!sql
SELECT "company"."id" FROM "company"
}}}
In the following example, we get the expected behavior:
{{{#!python
Company.objects.only("name", "country").defer("name")
}}}
only loads "id" and "country" fields with the following query:
{{{#!sql
SELECT "company"."id", "company"."country" FROM "company"
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32704>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: defer only | Triage Stage:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Manuel Baclet (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/32704#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Manuel Baclet | Owner: nobody
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
Comment:
Replying to [ticket:32704 Manuel Baclet]:
> Considering a simple `Company` model with four fields: `id`, `name`,
`trade_number` and `country`. If we evaluate a queryset containing a
`.defer()` following a `.only()`, the generated sql query selects
unexpected fields. For example:
>
> {{{#!python
> Company.objects.only("name").defer("name")
> }}}
> loads all the fields with the following query:
> {{{#!sql
> SELECT "company"."id", "company"."name", "company"."trade_number",
"company"."country" FROM "company"
> }}}
This is an expected behavior, `defer()` removes fields from the list of
fields specified by the `only()` method (i.e. list of fields that should
not be deferred). In this example `only()` adds `name` to the list,
`defer()` removes `name` from the list, so you have empty lists and all
fields will be loaded. It is also
[https://docs.djangoproject.com/en/dev/ref/models/querysets/#django.db.models.query.QuerySet.only
documented]:
{{{
# Final result is that everything except "headline" is deferred.
Entry.objects.only("headline", "body").defer("body")
}}}
> {{{#!python
> Company.objects.only("name").defer("name").defer("country")
> }}}
> also loads all the fields with the same query:
> {{{#!sql
> SELECT "company"."id", "company"."name", "company"."trade_number",
"company"."country" FROM "company"
> }}}
I agree you shouldn't get all field, but only `pk`, `name`, and
`trade_number`:
{{{
SELECT "ticket_32704_company"."id", "ticket_32704_company"."name",
"ticket_32704_company"."trade_number" FROM "ticket_32704_company"
}}}
this is due to the fact that `defer()` doesn't clear the list of deferred
field when chaining with `only()`. I attached a proposed patch.
--
Ticket URL: <https://code.djangoproject.com/ticket/32704#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Manuel Baclet | Owner: nobody
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* Attachment "ticket-32704.diff" added.
Draft.
Django
-------------------------------------+-------------------------------------
Reporter: Manuel Baclet | Owner: nobody
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Manuel Baclet):
After reading the documentation carefully, i cannot say that it is clearly
stated that defering all the fields used in a previous `.only()` call
performs a reset of the deferred set.
Moreover, in the `.defer()`
[https://docs.djangoproject.com/en/dev/ref/models/querysets/#django.db.models.query.QuerySet.defer
section], we have:
> You can make multiple calls to defer(). Each call adds new fields to the
deferred set:
and this seems to suggest that:
- calls to `.defer()` cannot remove items from the deferred set and
evaluating `qs.defer("some_field")` should never fetch the column
`"some_field"` (since this should add `"some_field"` to the deferred set)
- the querysets `qs.defer("field1").defer("field2")` and
`qs.defer("field1", "field2")` should be equivalent
IMHO, there is a mismatch between the doc and the actual implementation.
--
Ticket URL: <https://code.djangoproject.com/ticket/32704#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Manuel Baclet | Owner: nobody
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
> - calls to `.defer()` cannot remove items from the deferred set and
evaluating `qs.defer("some_field")` should never fetch the column
`"some_field"` (since this should add `"some_field"` to the deferred set)
Feel-free to propose a docs clarification (see also #24048).
> - the querysets `qs.defer("field1").defer("field2")` and
`qs.defer("field1", "field2")` should be equivalent
That's why I accepted
`Company.objects.only("name").defer("name").defer("country")` as a bug.
--
Ticket URL: <https://code.djangoproject.com/ticket/32704#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Manuel Baclet | Owner: nobody
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Manuel Baclet):
I think that what is described in the documentation is what users are
expected and it is the implementation that should be fixed!
With your patch proposal, i do not think that:
`Company.objects.only("name").defer("name").defer("country")` is
equivalent to `Company.objects.only("name").defer("name", "country")`
--
Ticket URL: <https://code.djangoproject.com/ticket/32704#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Manuel Baclet | Owner: nobody
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
Replying to [comment:5 Manuel Baclet]:
> I think that what is described in the documentation is what users are
expected and it is the implementation that should be fixed!
I don't agree, and there is no need to shout. As documented: ''"The only()
method is more or less the opposite of defer(). You call it with the
fields that should not be deferred ..."'', so
`.only('name').defer('name')` should return all fields. You can start a
discussion on DevelopersMailingList if you don't agree.
>
> With your patch proposal, i do not think that:
> `Company.objects.only("name").defer("name").defer("country")` is
equivalent to `Company.objects.only("name").defer("name", "country")`
Did you check this? with proposed patch `country` is the only deferred
fields in both cases. As far as I'm aware that's an intended behavior.
--
Ticket URL: <https://code.djangoproject.com/ticket/32704#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Manuel Baclet | Owner: nobody
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Manuel Baclet):
With the proposed patch, I think that:
{{{#!python
Company.objects.only("name").defer("name", "country")
}}}
loads all fields whereas:
{{{#!python
Company.objects.only("name").defer("name").defer("country")
}}}
loads all fields except `"country"`.
Why is that?
In the first case:
{{{#!python
existing.difference(field_names) == {"name"}.difference(["name",
"country"]) == empty_set
}}}
and we go into the `if` branch clearing all the deferred fields and we are
done.
In the second case:
{{{#!python
existing.difference(field_names) == {"name"}.difference(["name"]) ==
empty_set
}}}
and we go into the `if` branch clearing all the deferred fields. Then we
add `"country"` to the set of deferred fields.
--
Ticket URL: <https://code.djangoproject.com/ticket/32704#comment:7>
Django
-------------------------------------+-------------------------------------
| Wobrock
Type: Bug | Status: assigned
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: David Wobrock (added)
* owner: nobody => David Wobrock
* has_patch: 0 => 1
* status: new => assigned
Comment:
Hey all,
Replying to [comment:6 Mariusz Felisiak]:
> Replying to [comment:5 Manuel Baclet]:
> >
> > With your patch proposal, i do not think that:
> > `Company.objects.only("name").defer("name").defer("country")` is
equivalent to `Company.objects.only("name").defer("name", "country")`
>
> Did you check this? with proposed patch `country` is the only deferred
fields in both cases. As far as I'm aware that's an intended behavior.
I believe Manuel is right. This happens because the set difference in one
direction gives you the empty set that will clear out the deferred fields
- but it is missing the fact that we might also be adding more `defer`
fields than we had `only` fields in the first place, so that we actually
switch from an `.only()` to a `.defer()` mode.
See the corresponding PR that should fix this behaviour
https://github.com/django/django/pull/14667
--
Ticket URL: <https://code.djangoproject.com/ticket/32704#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Manuel Baclet | Owner: David
| Wobrock
Component: Database layer | Version: 3.1
(models, ORM) |
Keywords: defer only | 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:"fd999318ad78613227cdb7c5656345d9e216802b" fd999318]:
{{{
#!CommitTicketReference repository=""
revision="fd999318ad78613227cdb7c5656345d9e216802b"
Fixed #32704 -- Fixed list of deferred fields when chaining
QuerySet.defer() after only().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32704#comment:9>