[Django] #33260: Crash when using query set select_for_update(of=(...)) with exists()

[Django]

#33260: Crash when using query set select_for_update(of=(...)) with exists()
-------------------------------------+-------------------------------------
Reporter: Hannes | Owner: Hannes Ljungberg
Ljungberg |
Type: Bug | Status: assigned
Component: Database | Version: dev
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 |
-------------------------------------+-------------------------------------
For example specifying `(self,)` as an argument to `of`:
{{{
Person.objects.select_for_update(of=("self",)).exists()
}}}

Will crash with the following traceback:
{{{
Traceback (most recent call last):
File "/tests/django/django/test/testcases.py", line 1305, in
skip_wrapper
return test_func(*args, **kwargs)
File "/tests/django/tests/select_for_update/tests.py", line 599, in
test_select_for_update_with_exists
self.assertIs(Person.objects.select_for_update(of=("self",)).exists(),
True)
File "/tests/django/django/db/models/query.py", line 818, in exists
return self.query.has_results(using=self.db)
File "/tests/django/django/db/models/sql/query.py", line 546, in
has_results
return compiler.has_results()
File "/tests/django/django/db/models/sql/compiler.py", line 1174, in
has_results
return bool(self.execute_sql(SINGLE))
File "/tests/django/django/db/models/sql/compiler.py", line 1191, in
execute_sql
sql, params = self.as_sql()
File "/tests/django/django/db/models/sql/compiler.py", line 612, in
as_sql
of=self.get_select_for_update_of_arguments(),
File "/tests/django/django/db/models/sql/compiler.py", line 1087, in
get_select_for_update_of_arguments
col = _get_first_selected_col_from_model(klass_info)
File "/tests/django/django/db/models/sql/compiler.py", line 1053, in
_get_first_selected_col_from_model
concrete_model = klass_info['model']._meta.concrete_model
TypeError: 'NoneType' object is not subscriptable
}}}

Selecting a related model like:
{{{
Person.objects.select_for_update(of=("born",)).exists()
}}}

Will crash with the following traceback:
{{{
Traceback (most recent call last):
File "/tests/django/django/test/testcases.py", line 1305, in
skip_wrapper
return test_func(*args, **kwargs)
File "/tests/django/tests/select_for_update/tests.py", line 599, in
test_select_for_update_with_exists
self.assertIs(Person.objects.select_for_update(of=("born")).exists(),
True)
File "/tests/django/django/db/models/query.py", line 818, in exists
return self.query.has_results(using=self.db)
File "/tests/django/django/db/models/sql/query.py", line 546, in
has_results
return compiler.has_results()
File "/tests/django/django/db/models/sql/compiler.py", line 1174, in
has_results
return bool(self.execute_sql(SINGLE))
File "/tests/django/django/db/models/sql/compiler.py", line 1191, in
execute_sql
sql, params = self.as_sql()
File "/tests/django/django/db/models/sql/compiler.py", line 612, in
as_sql
of=self.get_select_for_update_of_arguments(),
File "/tests/django/django/db/models/sql/compiler.py", line 1091, in
get_select_for_update_of_arguments
*klass_info.get('related_klass_infos', []),
AttributeError: 'NoneType' object has no attribute 'get'
}}}

With https://code.djangoproject.com/ticket/32888 in consideration we
should probably just drop locking anything as no table columns are
selected.

Thinking something like:
{{{
diff --git a/django/db/models/sql/compiler.py
b/django/db/models/sql/compiler.py
index d1009847e7..7f6251aa34 100644
--- a/django/db/models/sql/compiler.py
+++ b/django/db/models/sql/compiler.py
@@ -1081,6 +1081,8 @@ class SQLCompiler:
invalid_names = []
for name in self.query.select_for_update_of:
klass_info = self.klass_info
+ if not klass_info:
+ continue
if name == 'self':
col = _get_first_selected_col_from_model(klass_info)
else:
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/33260>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#33260: Crash when using query set select_for_update(of=(...)) with exists()
-------------------------------------+-------------------------------------

Reporter: Hannes Ljungberg | Owner: Hannes

| Ljungberg
Type: Bug | Status: assigned

Component: Database layer | Version: dev
(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 Mariusz Felisiak):

* stage: Unreviewed => Accepted

Comment:

Thanks for the report. We can add an early return even before the loop:
{{{#!python
if not self.klass_info:
return []
result = []
invalid_names = []
for name in ...
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/33260#comment:1>

[Django]

#33260: Crash when using query set select_for_update(of=(...)) with exists()
-------------------------------------+-------------------------------------

Reporter: Hannes Ljungberg | Owner: Hannes

| Ljungberg
Type: Bug | Status: assigned

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: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Hannes Ljungberg):

* has_patch: 0 => 1

Comment:

PR: https://github.com/django/django/pull/15051

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

[Django]

#33260: Crash when using query set select_for_update(of=(...)) with exists()
-------------------------------------+-------------------------------------

Reporter: Hannes Ljungberg | Owner: Hannes

| Ljungberg
Type: Bug | Status: assigned

Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | 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 Mariusz Felisiak):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/33260#comment:3>

[Django]

#33260: Crash when using query set select_for_update(of=(...)) with exists()
-------------------------------------+-------------------------------------

Reporter: Hannes Ljungberg | Owner: Hannes
| Ljungberg

Type: Bug | Status: closed

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

Severity: Normal | 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 Mariusz Felisiak <felisiak.mariusz@…>):

* status: assigned => closed
* resolution: => fixed

Comment:

In [changeset:"25157033e979c134d455d46995a6db0838457d98" 25157033]:
{{{
#!CommitTicketReference repository=""
revision="25157033e979c134d455d46995a6db0838457d98"
Fixed #33260 -- Fixed crash when chaining QuerySet.exists() after
select_for_update(of=()).
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/33260#comment:4>