[Django] #30894: Reverse OneToOneField relation: `prefetch_related` uses `related_name` while `select_related` uses `related_query_name`
Django
while `select_related` uses `related_query_name`
-------------------------------------+-------------------------------------
Reporter: Adam | Owner: nobody
Sołtysik |
Type: Bug | Status: new
Component: Database | Version: 2.2
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
The documentation
(https://docs.djangoproject.com/en/2.2/ref/models/querysets/#select-
related) states:
You can also refer to the reverse direction of a OneToOneField in the
list of fields passed to select_related — that is, you can traverse a
OneToOneField back to the object on which the field is defined. Instead of
specifying the field name, use the related_name for the field on the
related object.
But in fact `related_query_name` is used by `select_related` and
`related_name` is used by `prefetch_related` .
When we have the following models:
{{{
class A(Model):
pass
}}}
{{{
class B(Model):
a = OneToOneField('A', related_name='b_set', related_query_name='b')
}}}
then `a.select_related('b')` works and `a.prefetch_related('b')` does not:
{{{AttributeError: Cannot find 'b' on A object, 'b' is an invalid
parameter to prefetch_related()}}}
or if class `A` happens to have a property named `b`:
{{{ValueError: 'b' does not resolve to an item that supports prefetching -
this is an invalid parameter to prefetch_related().}}}
And vice versa, `a.prefetch_related('b_set')` works and
`a.select_related('b_set')` does not:
{{{django.core.exceptions.FieldError: Invalid field name(s) given in
select_related: 'b_set'. Choices are: b}}}
The documentation implies that both methods should use `related_name`, but
I think it would be more intuitive if both used `related_query_name`,
given that we can write lookups like in filter queries etc., and such a
change would probably be less backward-incompatible.
--
Ticket URL: <https://code.djangoproject.com/ticket/30894>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
while `select_related` uses `related_query_name`
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Carlton Gibson):
Hi Adam.
It looks to me as if this case is already covered by the test suite (and
works)...
We have a
[https://github.com/django/django/blob/af8dbbe0d5099ce09fc77abdf8a3fb8a4f4015d9/tests/select_related_onetoone/models.py#L97-L103
LinkedList model]:
{{{
class LinkedList(models.Model):
name = models.CharField(max_length=50)
previous_item = models.OneToOneField(
'self', models.CASCADE,
related_name='next_item',
blank=True, null=True,
)
}}}
This is using the `related_name` `'next_item'`.
Then
[https://github.com/django/django/blob/af8dbbe0d5099ce09fc77abdf8a3fb8a4f4015d9/tests/select_related_onetoone/tests.py#L209-L214
in the test case, we use that in `select_related`]:
{{{
def test_self_relation(self):
item1 = LinkedList.objects.create(name='item1')
LinkedList.objects.create(name='item2', previous_item=item1)
with self.assertNumQueries(1):
item1_db =
LinkedList.objects.select_related('next_item').get(name='item1')
self.assertEqual(item1_db.next_item.name, 'item2')
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30894#comment:1>
Django
while `select_related` uses `related_query_name`
-------------------------------------+-------------------------------------
Type: | Status: closed
Cleanup/optimization |
Component: Documentation | Version: 2.2
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => wontfix
* component: Database layer (models, ORM) => Documentation
* type: Bug => Cleanup/optimization
Comment:
OK, yes. Your models work the same. It's the presence of the
`related_query_name` which causes the failure. Remove it and it works,
because (as per the docs) `related_query_name` defaults to `related_name`
if that's set.
At best this is a documentation cleanup/optimization **but** setting
`related_query_name` explicitly is an much more specialised use case than
setting `related_name`. I think the `related_query_name` docs are clear
enough that setting it overrides the name of the reverse filter, and that
a ''clarification'' in the `select_related()` docs would only muddy the
water for the majority of users. (One should under the consequences before
setting `related_query_name`.)
--
Ticket URL: <https://code.djangoproject.com/ticket/30894#comment:2>
Django
while `select_related` uses `related_query_name`
-------------------------------------+-------------------------------------
Type: | Status: closed
Cleanup/optimization |
Component: Documentation | Version: 2.2
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Adam Sołtysik):
I think the related_query_name docs are clear enough that setting it
overrides the name of the reverse filter, and that a clarification in the
select_related() docs would only muddy the waters for the majority of
users.
OK, but why doesn't it override the name of the reverse filter in the case
of `prefetch_related()` then? My report is about the inconsistency between
`select_related()` and `prefetch_related()`. I want to use different
`related_name` and `related_query_name` in my models and I would expect
that `related_query_name` can be used in both `select_related()` and
`prefetch_related()`.
--
Ticket URL: <https://code.djangoproject.com/ticket/30894#comment:3>
Django
while `select_related` uses `related_query_name`
-------------------------------------+-------------------------------------
Type: | Status: closed
Cleanup/optimization |
Component: Documentation | Version: 2.2
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Carlton Gibson):
Well, they're not really the same thing... `prefetch_related()` is
intended for ''Many'' relations, not one-to-one, and it first of all needs
to pick the related object descriptor off of the model in order to build
the right queryset, for the follow-up query. Thus it needs an actual
attribute on the model.
--
Ticket URL: <https://code.djangoproject.com/ticket/30894#comment:4>
Django
while `select_related` uses `related_query_name`
-------------------------------------+-------------------------------------
Type: | Status: closed
Cleanup/optimization |
Component: Documentation | Version: 2.2
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Adam Sołtysik):
This only explains why it was easier to implement that way, but doesn't
prove it shouldn't work the other way. I started using
`prefetch_related()` instead of `select_related()` when I found out that
it is much faster with `LIMIT`ed queries (at least on PostgreSQL). The
fact that I had to change relation names was a bit confusing. But if you
don't think this should be changed, then so be it.
--
Ticket URL: <https://code.djangoproject.com/ticket/30894#comment:5>
Django
while `select_related` uses `related_query_name`
-------------------------------------+-------------------------------------
Type: | Status: closed
Cleanup/optimization |
Component: Documentation | Version: 2.2
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
I think this should be addressed, it doesn't make much sense that
`related_query_name` is used everywhere but in `prefetch_related`. It was
probably simply overlooked in `prefetch_related` given it works in
`select_related`.
It should not be too hard to deprecate given the rare usages of
`related_query_name` by accepting both `related_query_name` and
`related_name` during the deprecation period and warning when
`related_name != related_query_name` and `related_name` is used.
--
Ticket URL: <https://code.djangoproject.com/ticket/30894#comment:6>
Django
while `select_related` uses `related_query_name`
-------------------------------------+-------------------------------------
Type: | Status: closed
Cleanup/optimization |
(models, ORM) |
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* component: Documentation => Database layer (models, ORM)
Comment:
Hey Simon. I'm happy if you want to reopen and Accept, but I'm not
entirely sure it's the best change...
`related_query_name` serves when to make filtering a little prettier where
the `related_name` doesn't read right, i.e. you might want to write
`...filter(tag__name='...')` rather that `...filter(tag_set__name='...')`
(or `tags` being the example from the docs). That reads better because
you're saying ''where the tag name is ...'', i.e. checking the property on
the individual, rather than the set.
But with `prefetch_related()`, you want to prefetch the set, so you want
`prefetch_related('tag_set')` (or `tags`) — you **want** the
`related_name`.
(So if the issue is inconsistency, maybe it's `select_related` that has it
backwards — via `get_related_name()`, but we can't change that now I'd
guess...)
Q: how does `get_prefetcher()` look if we're **not** retrieving the actual
descriptor off of the model class?
As I say, I leave to you. Thanks.
(For my future ref c.f. #18668, #15962, ...)
--
Ticket URL: <https://code.djangoproject.com/ticket/30894#comment:7>
Django
while `select_related` uses `related_query_name`
-------------------------------------+-------------------------------------
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Adam Sołtysik):
I think I should add a bit more about my exact use case. I have
`OneToOneField` with `_something` as `related_name` and `something` as
`related_query_name`, and I also have a property `something` that returns
`_something` if it exists, and a default object if it doesn't. This way
everywhere in my code I use only `something` without underscore -- except
for `prefetch_related()`, and it's even worse with chained relations, as I
end up with triple underscores, like
`prefetch_related(_something1___something2)`.
However, I've just a found another solution for the problem of missing
`OneToOneField`s: https://stackoverflow.com/a/32106815, which doesn't make
it necessary to use different `related_name` and `related_query_name`. I'm
going to test it, and if it works, then the issue will be lower priority
for me. Although I still think the two methods should work consistently.
--
Ticket URL: <https://code.djangoproject.com/ticket/30894#comment:8>