[Django] #28944: Queryset cannot handle select_for_update and values/values_list at the same time
Django
time
-------------------------------------+-------------------------------------
Reporter: Thierry | Owner: nobody
Bastian |
Type: Bug | Status: new
Component: Database | Version: 2.0
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 |
-------------------------------------+-------------------------------------
With any of my models, if I do the following:
{{{
model.objects.select_for_update(of=('self',)).values_list('pk')
}}}
I'm getting the following exception:
{{{
Traceback (most recent call last):
File "<console>", line 2, in <module>
File "/usr/local/filewave/python/lib/python3.6/site-
packages/django/db/models/query.py", line 272, in __iter__
self._fetch_all()
File "/usr/local/filewave/python/lib/python3.6/site-
packages/django/db/models/query.py", line 1179, in _fetch_all
self._result_cache = list(self._iterable_class(self))
File "/usr/local/filewave/python/lib/python3.6/site-
packages/django/db/models/query.py", line 139, in __iter__
return compiler.results_iter(tuple_expected=True,
chunked_fetch=self.chunked_fetch, chunk_size=self.chunk_size)
File "/usr/local/filewave/python/lib/python3.6/site-
packages/django/db/models/sql/compiler.py", line 1014, in results_iter
results = self.execute_sql(MULTI, chunked_fetch=chunked_fetch,
chunk_size=chunk_size)
File "/usr/local/filewave/python/lib/python3.6/site-
packages/django/db/models/sql/compiler.py", line 1050, in execute_sql
sql, params = self.as_sql()
File "/usr/local/filewave/python/lib/python3.6/site-
packages/django/db/models/sql/compiler.py", line 507, in as_sql
of=self.get_select_for_update_of_arguments(),
File "/usr/local/filewave/python/lib/python3.6/site-
packages/django/db/models/sql/compiler.py", line 961, in
get_select_for_update_of_arguments
', '.join(_get_field_choices()),
File "/usr/local/filewave/python/lib/python3.6/site-
packages/django/db/models/sql/compiler.py", line 928, in
_get_field_choices
for klass_info in klass_info.get('related_klass_infos', [])
AttributeError: 'NoneType' object has no attribute 'get'
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28944>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
time
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
Marking as release blocker because it's a bug in a newly introduced
feature. (`selected_for_update(of)`).
--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:1>
Django
crashes
-------------------------------------+-------------------------------------
Reporter: Thierry Bastian | Owner: nobody
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Ran Benita (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:2>
Django
crashes
-------------------------------------+-------------------------------------
Reporter: Thierry Bastian | Owner: nobody
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Comment:
Suggested fix: https://github.com/django/django/pull/9487
--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:3>
Django
crashes
-------------------------------------+-------------------------------------
Reporter: Thierry Bastian | Owner: nobody
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Release blocker | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
Comment:
I think the patch is RFC except for a few cosmetic comments and a release
note for 2.0.1.
--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:4>
Django
crashes
-------------------------------------+-------------------------------------
Reporter: Thierry Bastian | Owner: nobody
Component: Database layer | Version: 2.0
(models, ORM) |
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"c21f158295d92e35caf96436bfdbbff554fc5569" c21f1582]:
{{{
#!CommitTicketReference repository=""
revision="c21f158295d92e35caf96436bfdbbff554fc5569"
Fixed #28944 -- Fixed crash when chaining values()/values_list() after
QuerySet.select_for_update(of=()).
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:5>
Django
crashes
-------------------------------------+-------------------------------------
Reporter: Thierry Bastian | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"4e4619a2b8f79699fbdb2c4f1bc4db55f59af6e6" 4e4619a]:
{{{
#!CommitTicketReference repository=""
revision="4e4619a2b8f79699fbdb2c4f1bc4db55f59af6e6"
[2.0.x] Fixed #28944 -- Fixed crash when chaining values()/values_list()
after QuerySet.select_for_update(of=()).
Backport of c21f158295d92e35caf96436bfdbbff554fc5569 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:6>
Django
crashes
-------------------------------------+-------------------------------------
Reporter: Thierry Bastian | Owner: nobody
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Release blocker | Resolution:
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: closed => new
* resolution: fixed =>
Comment:
Ok so you fixed the simple case with self, but it still fails where there
are joins.
So if you have 2 models:
{{{
class A(models.Model):
foo = models.TextField()
class B(models.Model):
bla = models.TextField()
a = models.ForeignKey(a, on_delete=models.CASCADE)
and then if you want to do:
B.objects.all().select_related('a').select_for_update(of=('self', 'a')) ,
that works
but the same with a values_list:
B.objects.all().select_related('a').select_for_update(of=('self',
'a')).values_list('pk', 'a__foo'), that does not work and results in
something like (I have different class names in my code)
Traceback (most recent call last):
File "<console>", line 2, in <module>
File "/usr/local/filewave/python/lib/python3.6/site-
packages/django/db/models/query.py", line 248, in __repr__
data = list(self[:REPR_OUTPUT_SIZE + 1])
File "/usr/local/filewave/python/lib/python3.6/site-
packages/django/db/models/query.py", line 272, in __iter__
self._fetch_all()
File "/usr/local/filewave/python/lib/python3.6/site-
packages/django/db/models/query.py", line 1179, in _fetch_all
self._result_cache = list(self._iterable_class(self))
File "/usr/local/filewave/python/lib/python3.6/site-
packages/django/db/models/query.py", line 139, in __iter__
return compiler.results_iter(tuple_expected=True,
chunked_fetch=self.chunked_fetch, chunk_size=self.chunk_size)
File "/usr/local/filewave/python/lib/python3.6/site-
packages/django/db/models/sql/compiler.py", line 1015, in results_iter
results = self.execute_sql(MULTI, chunked_fetch=chunked_fetch,
chunk_size=chunk_size)
File "/usr/local/filewave/python/lib/python3.6/site-
packages/django/db/models/sql/compiler.py", line 1051, in execute_sql
sql, params = self.as_sql()
File "/usr/local/filewave/python/lib/python3.6/site-
packages/django/db/models/sql/compiler.py", line 508, in as_sql
of=self.get_select_for_update_of_arguments(),
File "/usr/local/filewave/python/lib/python3.6/site-
packages/django/db/models/sql/compiler.py", line 962, in
get_select_for_update_of_arguments
', '.join(_get_field_choices()),
django.core.exceptions.FieldError: Invalid field name(s) given in
select_for_update(of=(...)): mdm_client. Only relational fields followed
in the query are allowed. Choices are: self.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:7>
Django
crashes
-------------------------------------+-------------------------------------
Reporter: Thierry Bastian | Owner: nobody
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Release blocker | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 1 => 0
* stage: Ready for checkin => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:8>
Django
crashes
-------------------------------------+-------------------------------------
Reporter: Thierry Bastian | Owner: nobody
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Ran Benita):
I'm afraid this case is harder. Essentially, the current implementation of
select_for_update(of=(...)) is based on the select_related()
infrastructure, but of course once .values()/.values_list() is invoked
there is no more select_related.
I will try to dig deeper once I am able. Help is welcome.
--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:9>
Django
crashes
-------------------------------------+-------------------------------------
Reporter: Thierry Bastian | Owner: nobody
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham):
It could be acceptable (at least as a temporary measure) to raise a
message that it's not supported.
--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:10>
Django
crashes
-------------------------------------+-------------------------------------
Reporter: Thierry Bastian | Owner: nobody
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Thierry Bastian):
@Ran Benita: I was afraid you'd say something like that. My knowledge of
django internals is not that great at the moment. So I will have to try
and find a better way to do that on my own.
--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:11>
Django
crashes
-------------------------------------+-------------------------------------
Reporter: Thierry Bastian | Owner: nobody
Component: Database layer | Version: 2.0
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* severity: Release blocker => Normal
--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:12>
Django
crashes
-------------------------------------+-------------------------------------
Reporter: Thierry Bastian | Owner: nobody
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
Comment:
Anssi started a [https://github.com/django/django/pull/9984 PR] but
doesn't have time to finish it.
--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:13>
Django
crashes
-------------------------------------+-------------------------------------
Reporter: Thierry Bastian | Owner: nobody
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Can Sarıgöl):
[https://github.com/django/django/pull/11387 PR]
a different approach
--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:14>
Django
crashes
-------------------------------------+-------------------------------------
| Sarıgöl
Type: Bug | Status: assigned
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => assigned
* needs_better_patch: 1 => 0
* cc: Can Sarıgöl (added)
* owner: nobody => Can Sarıgöl
--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:15>
Django
crashes
-------------------------------------+-------------------------------------
Reporter: Thierry Bastian | Owner: Can
| Sarıgöl
Type: Bug | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* version: 2.0 => master
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:16>
Django
crashes
-------------------------------------+-------------------------------------
Reporter: Thierry Bastian | Owner: Can
| Sarıgöl
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
Comment:
[https://github.com/django/django/pull/12258]
I've added a PR based off the one Anssi started. I'll be glad if someone
could take a look and let me know of any improvements needed. All sqlite
and postgres tests were passing when I tested through django-docker-box.
--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:17>
Django
crashes
-------------------------------------+-------------------------------------
| Khanna
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: Can Sarıgöl => Chetan Khanna
--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:18>
Django
crashes
-------------------------------------+-------------------------------------
Reporter: Thierry Bastian | Owner: Chetan
| Khanna
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:19>
Django
crashes
-------------------------------------+-------------------------------------
Reporter: Thierry Bastian | Owner: Chetan
| Khanna
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:20>
Django
crashes
-------------------------------------+-------------------------------------
Reporter: Thierry Bastian | Owner: Chetan
| Khanna
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:21>
Django
crashes
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
-------------------------------------+-------------------------------------
* owner: Chetan Khanna => (none)
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:22>