Opened 8 months ago

Last modified 5 weeks ago

#28944 new Bug

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

Reported by: Thierry Bastian Owned by: nobody
Component: Database layer (models, ORM) Version: 2.0
Severity: Normal Keywords:
Cc: Ran Benita Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

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'

Change History (13)

comment:1 Changed 8 months ago by Simon Charette

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

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

comment:2 Changed 8 months ago by Tim Graham

Cc: Ran Benita added
Summary: Queryset cannot handle select_for_update and values/values_list at the same timeChaining values()/values_list() after QuerySet.select_for_update(of=()) crashes

comment:3 Changed 8 months ago by Ran Benita

Has patch: set

comment:4 Changed 8 months ago by Simon Charette

Triage Stage: AcceptedReady for checkin

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

comment:5 Changed 8 months ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In c21f1582:

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

comment:6 Changed 8 months ago by 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

comment:7 Changed 7 months ago by Thierry Bastian

Resolution: fixed
Status: closednew

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.
Last edited 7 months ago by Tim Graham (previous) (diff)

comment:8 Changed 7 months ago by Tim Graham

Has patch: unset
Triage Stage: Ready for checkinAccepted

comment:9 Changed 7 months ago 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.

comment:10 Changed 7 months ago by Tim Graham

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

comment:11 Changed 7 months ago 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.

comment:12 Changed 7 months ago by Tim Graham

Severity: Release blockerNormal

comment:13 Changed 5 weeks ago by Tim Graham

Has patch: set
Patch needs improvement: set

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

Note: See TracTickets for help on using tickets.
Back to Top