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

[Thierry Bastian]

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'

[Simon Charette]

Marking as release blocker because it's a bug in a newly introduced feature. (selected_for_update(of)).

[Ran Benita]

Suggested fix: ​https://github.com/django/django/pull/9487

[Simon Charette]

I think the patch is RFC except for a few cosmetic comments and a release note for 2.0.1.

[Tim Graham <timograham@…>]

In c21f1582:

Fixed #28944 -- Fixed crash when chaining values()/values_list() after QuerySet.select_for_update(of=()).

[Tim Graham <timograham@…>]

In 4e4619a:

[2.0.x] Fixed #28944 -- Fixed crash when chaining values()/values_list() after QuerySet.select_for_update(of=()).

Backport of c21f158295d92e35caf96436bfdbbff554fc5569 from master

[Thierry Bastian]

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.

[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.

[Tim Graham]

It could be acceptable (at least as a temporary measure) to raise a message that it's not supported.

[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.

[Tim Graham]

Anssi started a ​PR but doesn't have time to finish it.

[Can Sarıgöl]

​PR

a different approach

[Chetan Khanna]

​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.