Opened 10 years ago
Last modified 18 months ago
#23051 assigned Bug
QuerySet.only() fail to work with reverse o2o relationships
Reported by: | Vladimir Dmitriev | Owned by: | Paulo |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | orm only reverse relationship OneToOneField |
Cc: | Dan Davis | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
Specifying a field from reverse relationship model in the .only() queryset method have no effect on compiled query:
# sample models class Person(models.Model): name = models.CharField(max_length=64) class PersonExtra(models.Model): bio = models.TextField() information = models.TextField() person = models.OneToOneField(Person) # manage.py shell >>> from testapp.models import Person >>> print Person.objects.all().only('name').query SELECT "testapp_person"."id", "testapp_person"."name" FROM "testapp_person" >>> print Person.objects.all().select_related('personextra').only('name', 'personextra__bio').query # expected table join and personextra__bio to be loaded SELECT "testapp_person"."id", "testapp_person"."name" FROM "testapp_person"
defer() method works fine:
>>> print Person.objects.all().select_related('personextra').defer('personextra__information').query SELECT "testapp_person"."id", "testapp_person"."name", "testapp_personextra"."id", "testapp_personextra"."bio", "testapp_personextra"."person_id" FROM "testapp_person" LEFT OUTER JOIN "testapp_personextra" ON ( "testapp_person"."id" = "testapp_personextra"."person_id" )
Change History (10)
comment:1 by , 10 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 10 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Will try to at least make a test for this during EP14.
comment:3 by , 10 years ago
Failing test:
diff --git a/tests/defer_regress/tests.py b/tests/defer_regress/tests.py index 63f9e97..5d70da2 100644 --- a/tests/defer_regress/tests.py +++ b/tests/defer_regress/tests.py @@ -144,6 +144,18 @@ class DeferRegressionTest(TestCase): list(SimpleItem.objects.annotate(Count('feature')).only('name')), list) + def test_ticket_23051(self): + item = Item.objects.create(value=1, name='item') + OneToOneItem.objects.create(item=item, name='one_to_one_item') + + obj = Item.objects.only('name', 'one_to_one_item__name').get(name='item') + + with self.assertNumQueries(0): + self.assertEqual(obj.name, 'item') + + with self.assertNumQueries(0): + self.assertEqual(obj.one_to_one_item.name, 'one_to_one_item') + def test_only_and_defer_usage_on_proxy_models(self): # Regression for #15790 - only() broken for proxy models proxy = Proxy.objects.create(name="proxy", value=42)
comment:4 by , 10 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
Version: | 1.6 → master |
I looked in to this, and it seems that at least SQLCompiler.deferred_to_columns
(and all related methods called before then) works as intended in this case. However, I will not be able to solve this, even if I believe that it should not be unsolvable.
comment:5 by , 6 years ago
Cc: | added |
---|
comment:6 by , 5 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:7 by , 5 years ago
Has patch: | set |
---|
The original reported issue was fixed already, calling:
Person.objects.all().select_related('personextra').only('name', 'personextra__bio')
Gives the expected result (on 2.3 master branch).
But as pointed out in the comments above, calling:
# removed select_related() Person.objects.all().only('name', 'personextra__bio')
Will silently fail to prefetch the personextra relationship.
I've opened a pr https://github.com/django/django/pull/11981 that raises a ValueError when passing a reverse field to only(),
without prefetching the relationship first (as in the first example).
comment:8 by , 5 years ago
As noted in comment:7 above, the usecase is now supported when used in conjunction with select_related
.
I did some bisecting to track down the commit involved and found out that two commits were actually involved:
1) With 3daa9d60be6672841ceed5bb812b6fb25950dc16, Django started raising an error: FieldError: Invalid field name(s) given in select_related: 'personextra'. Choices are: personextra
2) With 0456a8b9e66608e2e7b7e64035d477f44c259c94, the error message disappeared and the feature started working
For reference, here's the testcase that I used (using the models from the original ticket):
class TicketTestCase(TestCase): def test_ticket(self): p = Person.objects.create(name="Baptiste") PersonExtra.objects.create(person=p, bio="yes") qs = list(Person.objects.select_related('personextra').only('name', 'personextra__bio')) with self.assertNumQueries(0): self.assertEqual(len(qs), 1) self.assertEqual(qs[0].name, "Baptiste") self.assertEqual(qs[0].personextra.bio, "yes")
I haven't looked in details at the proposed pull request, but I do have some preliminary questions:
1) Is it fundamentally impossible to support using a reverse o2o field in calls to only()
? The PR adds an error message but I think it would be better if we could fix the issue instead.
2) It seems that the problem exists for the case of a normal forward o2o (PersonExtra.objects.only('person__name')
in our example) and also for reverse M2M (#30124). Does the PR address those too?
Thanks
comment:9 by , 5 years ago
Patch needs improvement: | set |
---|
Paulo, please check Baptiste comments and questions.
comment:10 by , 18 months ago
FWIW #34612 was a related regression of 4.2 refactored deferred field refactor. We still silence usage of only
when it refers to related models that are not selected so this ticket is still relevant though.
Hi,
I can indeed reproduce this.
Currently,
Person.objects.only('name', 'personextra__bio')
is accepted by returns the wrong query. If supporting that use case is not possible, it should at least raise an error instead of silently doing the wrong thing.Thanks.