summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDaniel Hillier <daniel.hillier@gmail.com>2020-08-08 15:17:36 +1000
committerMariusz Felisiak <felisiak.mariusz@gmail.com>2020-08-11 12:33:18 +0200
commit839f906a23271b24bf435b16575bbbfd0d2ddcf9 (patch)
tree1b4526f383a098cfa44d3e66c084dc6f73680be8
parent30706246e74e2505c03bbc7c59a7b1c5c2c014cb (diff)
downloaddjango-839f906a23271b24bf435b16575bbbfd0d2ddcf9.tar.gz
[2.2.x] Fixed #31866 -- Fixed locking proxy models in QuerySet.select_for_update(of=()).
Backport of 60626162f76f26d32a38d18151700cb041201fb3 from master
-rw-r--r--django/db/models/sql/compiler.py6
-rw-r--r--docs/releases/2.2.16.txt5
-rw-r--r--tests/select_for_update/models.py14
-rw-r--r--tests/select_for_update/tests.py32
4 files changed, 53 insertions, 4 deletions
diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py
index e5c726676a..f3d66cff33 100644
--- a/django/db/models/sql/compiler.py
+++ b/django/db/models/sql/compiler.py
@@ -948,7 +948,8 @@ class SQLCompiler:
the query.
"""
def _get_parent_klass_info(klass_info):
- for parent_model, parent_link in klass_info['model']._meta.parents.items():
+ concrete_model = klass_info['model']._meta.concrete_model
+ for parent_model, parent_link in concrete_model._meta.parents.items():
parent_list = parent_model._meta.get_parent_list()
yield {
'model': parent_model,
@@ -973,8 +974,9 @@ class SQLCompiler:
select_fields is filled recursively, so it also contains fields
from the parent models.
"""
+ concrete_model = klass_info['model']._meta.concrete_model
for select_index in klass_info['select_fields']:
- if self.select[select_index][0].target.model == klass_info['model']:
+ if self.select[select_index][0].target.model == concrete_model:
return self.select[select_index][0]
def _get_field_choices():
diff --git a/docs/releases/2.2.16.txt b/docs/releases/2.2.16.txt
index 7e80406e9d..daba6f6f52 100644
--- a/docs/releases/2.2.16.txt
+++ b/docs/releases/2.2.16.txt
@@ -9,4 +9,7 @@ Django 2.2.16 fixes a data loss bug in 2.2.15.
Bugfixes
========
-* ...
+* Fixed a data loss possibility in the
+ :meth:`~django.db.models.query.QuerySet.select_for_update()`. When using
+ related fields pointing to a proxy model in the ``of`` argument, the
+ corresponding model was not locked (:ticket:`31866`).
diff --git a/tests/select_for_update/models.py b/tests/select_for_update/models.py
index 305e8cac49..0a6396bc70 100644
--- a/tests/select_for_update/models.py
+++ b/tests/select_for_update/models.py
@@ -23,6 +23,20 @@ class EUCity(models.Model):
country = models.ForeignKey(EUCountry, models.CASCADE)
+class CountryProxy(Country):
+ class Meta:
+ proxy = True
+
+
+class CountryProxyProxy(CountryProxy):
+ class Meta:
+ proxy = True
+
+
+class CityCountryProxy(models.Model):
+ country = models.ForeignKey(CountryProxyProxy, models.CASCADE)
+
+
class Person(models.Model):
name = models.CharField(max_length=30)
born = models.ForeignKey(City, models.CASCADE, related_name='+')
diff --git a/tests/select_for_update/tests.py b/tests/select_for_update/tests.py
index 7ef0319477..336d4a62ae 100644
--- a/tests/select_for_update/tests.py
+++ b/tests/select_for_update/tests.py
@@ -15,7 +15,9 @@ from django.test import (
)
from django.test.utils import CaptureQueriesContext
-from .models import City, Country, EUCity, EUCountry, Person, PersonProfile
+from .models import (
+ City, CityCountryProxy, Country, EUCity, EUCountry, Person, PersonProfile,
+)
class SelectForUpdateTests(TransactionTestCase):
@@ -196,6 +198,21 @@ class SelectForUpdateTests(TransactionTestCase):
self.assertTrue(self.has_for_update_sql(ctx.captured_queries, of=expected))
@skipUnlessDBFeature('has_select_for_update_of')
+ def test_for_update_sql_model_proxy_generated_of(self):
+ with transaction.atomic(), CaptureQueriesContext(connection) as ctx:
+ list(CityCountryProxy.objects.select_related(
+ 'country',
+ ).select_for_update(
+ of=('country',),
+ ))
+ if connection.features.select_for_update_of_column:
+ expected = ['select_for_update_country"."entity_ptr_id']
+ else:
+ expected = ['select_for_update_country']
+ expected = [connection.ops.quote_name(value) for value in expected]
+ self.assertTrue(self.has_for_update_sql(ctx.captured_queries, of=expected))
+
+ @skipUnlessDBFeature('has_select_for_update_of')
def test_for_update_of_followed_by_values(self):
with transaction.atomic():
values = list(Person.objects.select_for_update(of=('self',)).values('pk'))
@@ -354,6 +371,19 @@ class SelectForUpdateTests(TransactionTestCase):
EUCountry.objects.select_for_update(of=('name',)).get()
@skipUnlessDBFeature('has_select_for_update', 'has_select_for_update_of')
+ def test_model_proxy_of_argument_raises_error_proxy_field_in_choices(self):
+ msg = (
+ 'Invalid field name(s) given in select_for_update(of=(...)): '
+ 'name. Only relational fields followed in the query are allowed. '
+ 'Choices are: self, country, country__entity_ptr.'
+ )
+ with self.assertRaisesMessage(FieldError, msg):
+ with transaction.atomic():
+ CityCountryProxy.objects.select_related(
+ 'country',
+ ).select_for_update(of=('name',)).get()
+
+ @skipUnlessDBFeature('has_select_for_update', 'has_select_for_update_of')
def test_reverse_one_to_one_of_arguments(self):
"""
Reverse OneToOneFields may be included in of=(...) as long as NULLs