[Django] #28944: Queryset cannot handle select_for_update and values/values_list at the same time

[Django]

#28944: Queryset cannot handle select_for_update and values/values_list at the same
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]

#28944: Queryset cannot handle select_for_update and values/values_list at the same
time
-------------------------------------+-------------------------------------

Reporter: Thierry Bastian | Owner: nobody
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
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* 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]

#28944: Chaining values()/values_list() after QuerySet.select_for_update(of=())
crashes

-------------------------------------+-------------------------------------
Reporter: Thierry Bastian | Owner: nobody

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
-------------------------------------+-------------------------------------

Changes (by Tim Graham):

* cc: Ran Benita (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:2>

[Django]

#28944: Chaining values()/values_list() after QuerySet.select_for_update(of=())
crashes
-------------------------------------+-------------------------------------
Reporter: Thierry Bastian | Owner: nobody

Type: Bug | Status: new

Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Ran Benita):

* 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]

#28944: Chaining values()/values_list() after QuerySet.select_for_update(of=())
crashes
-------------------------------------+-------------------------------------
Reporter: Thierry Bastian | Owner: nobody

Type: Bug | Status: new

Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Release blocker | Resolution:

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* 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]

#28944: Chaining values()/values_list() after QuerySet.select_for_update(of=())
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

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Tim Graham <timograham@…>):

* 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]

#28944: Chaining values()/values_list() after QuerySet.select_for_update(of=())
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

Has patch: 1 | Needs documentation: 0

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]

#28944: Chaining values()/values_list() after QuerySet.select_for_update(of=())
crashes
-------------------------------------+-------------------------------------
Reporter: Thierry Bastian | Owner: nobody

Type: Bug | Status: new

Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Release blocker | Resolution:

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Thierry Bastian):

* 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]

#28944: Chaining values()/values_list() after QuerySet.select_for_update(of=())
crashes
-------------------------------------+-------------------------------------
Reporter: Thierry Bastian | Owner: nobody

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
-------------------------------------+-------------------------------------

Changes (by Tim Graham):

* has_patch: 1 => 0
* stage: Ready for checkin => Accepted

--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:8>

[Django]

#28944: Chaining values()/values_list() after QuerySet.select_for_update(of=())
crashes
-------------------------------------+-------------------------------------
Reporter: Thierry Bastian | Owner: nobody

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
-------------------------------------+-------------------------------------

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]

#28944: Chaining values()/values_list() after QuerySet.select_for_update(of=())
crashes
-------------------------------------+-------------------------------------
Reporter: Thierry Bastian | Owner: nobody

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
-------------------------------------+-------------------------------------

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]

#28944: Chaining values()/values_list() after QuerySet.select_for_update(of=())
crashes
-------------------------------------+-------------------------------------
Reporter: Thierry Bastian | Owner: nobody

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
-------------------------------------+-------------------------------------

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]

#28944: Chaining values()/values_list() after QuerySet.select_for_update(of=())
crashes
-------------------------------------+-------------------------------------
Reporter: Thierry Bastian | Owner: nobody

Type: Bug | Status: new

Component: Database layer | Version: 2.0
(models, ORM) |

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Tim Graham):

* severity: Release blocker => Normal

--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:12>

[Django]

#28944: Chaining values()/values_list() after QuerySet.select_for_update(of=())
crashes
-------------------------------------+-------------------------------------
Reporter: Thierry Bastian | Owner: nobody

Type: Bug | Status: new

Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Tim Graham):

* 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]

#28944: Chaining values()/values_list() after QuerySet.select_for_update(of=())
crashes
-------------------------------------+-------------------------------------
Reporter: Thierry Bastian | Owner: nobody

Type: Bug | Status: new

Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
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]

#28944: Chaining values()/values_list() after QuerySet.select_for_update(of=())
crashes
-------------------------------------+-------------------------------------

Reporter: Thierry Bastian | Owner: Can
| Sarıgöl
Type: Bug | Status: assigned

Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Can Sarıgöl):

* 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]

#28944: Chaining values()/values_list() after QuerySet.select_for_update(of=())
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

Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by felixxm):

* needs_better_patch: 0 => 1

* version: 2.0 => master
* needs_tests: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:16>

[Django]

#28944: Chaining values()/values_list() after QuerySet.select_for_update(of=())
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

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Chetan Khanna):

* 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]

#28944: Chaining values()/values_list() after QuerySet.select_for_update(of=())
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

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by felixxm):

* owner: Can Sarıgöl => Chetan Khanna

--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:18>

[Django]

#28944: Chaining values()/values_list() after QuerySet.select_for_update(of=())
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

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by felixxm):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:19>

[Django]

#28944: Chaining values()/values_list() after QuerySet.select_for_update(of=())
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

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Chetan Khanna):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:20>

[Django]

#28944: Chaining values()/values_list() after QuerySet.select_for_update(of=())
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

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by felixxm):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:21>

[Django]

#28944: Chaining values()/values_list() after QuerySet.select_for_update(of=())
crashes
-------------------------------------+-------------------------------------

Reporter: Thierry Bastian | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: dev

(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak):

* owner: Chetan Khanna => (none)
* status: assigned => new

--
Ticket URL: <https://code.djangoproject.com/ticket/28944#comment:22>