mirror of
				https://github.com/django/django.git
				synced 2025-10-31 09:41:08 +00:00 
			
		
		
		
	Fixed #36301 -- Fixed select_for_update(of) crash when using values()/values_list().
Regression in 65ad4ade74 which allowed for
annotations to be SELECT'ed before model field references through
values()/values_list() and broke assumptions the select_for_update(of)
table infererence logic had about model fields always being first.
Refs #28900.
Thanks OutOfFocus4 for the report and Sarah for the test.
			
			
This commit is contained in:
		
				
					committed by
					
						 Mariusz Felisiak
						Mariusz Felisiak
					
				
			
			
				
	
			
			
			
						parent
						
							8ad3e80e88
						
					
				
				
					commit
					71a19a0e47
				
			| @@ -256,17 +256,8 @@ class SQLCompiler: | ||||
|             # self.query.select is a special case. These columns never go to | ||||
|             # any model. | ||||
|             cols = self.query.select | ||||
|         if cols: | ||||
|             klass_info = { | ||||
|                 "model": self.query.model, | ||||
|                 "select_fields": list( | ||||
|                     range( | ||||
|                         len(self.query.extra_select), | ||||
|                         len(self.query.extra_select) + len(cols), | ||||
|                     ) | ||||
|                 ), | ||||
|             } | ||||
|         selected = [] | ||||
|         select_fields = None | ||||
|         if self.query.selected is None: | ||||
|             selected = [ | ||||
|                 *( | ||||
| @@ -276,18 +267,28 @@ class SQLCompiler: | ||||
|                 *((None, col) for col in cols), | ||||
|                 *self.query.annotation_select.items(), | ||||
|             ] | ||||
|             select_fields = list( | ||||
|                 range( | ||||
|                     len(self.query.extra_select), | ||||
|                     len(self.query.extra_select) + len(cols), | ||||
|                 ) | ||||
|             ) | ||||
|         else: | ||||
|             for alias, expression in self.query.selected.items(): | ||||
|             select_fields = [] | ||||
|             for index, (alias, expression) in enumerate(self.query.selected.items()): | ||||
|                 # Reference to an annotation. | ||||
|                 if isinstance(expression, str): | ||||
|                     expression = self.query.annotations[expression] | ||||
|                 # Reference to a column. | ||||
|                 elif isinstance(expression, int): | ||||
|                     select_fields.append(index) | ||||
|                     expression = cols[expression] | ||||
|                 # ColPairs cannot be aliased. | ||||
|                 if isinstance(expression, ColPairs): | ||||
|                     alias = None | ||||
|                 selected.append((alias, expression)) | ||||
|         if select_fields: | ||||
|             klass_info = {"model": self.query.model, "select_fields": select_fields} | ||||
|  | ||||
|         for select_idx, (alias, expression) in enumerate(selected): | ||||
|             if alias: | ||||
|   | ||||
| @@ -28,3 +28,7 @@ Bugfixes | ||||
|   ``allow_overwrite=True``, where leftover content from a previously larger | ||||
|   file could remain after overwriting with a smaller one due to lack of | ||||
|   truncation (:ticket:`36298`). | ||||
|  | ||||
| * Fixed a regression in Django 5.2 that caused a crash when using | ||||
|   ``QuerySet.select_for_update(of=(…))`` with ``values()/values_list()`` | ||||
|   including expressions (:ticket:`36301`). | ||||
|   | ||||
| @@ -13,6 +13,8 @@ from django.db import ( | ||||
|     router, | ||||
|     transaction, | ||||
| ) | ||||
| from django.db.models import F, Value | ||||
| from django.db.models.functions import Concat | ||||
| from django.test import ( | ||||
|     TransactionTestCase, | ||||
|     override_settings, | ||||
| @@ -149,6 +151,15 @@ class SelectForUpdateTests(TransactionTestCase): | ||||
|         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_values_list(self): | ||||
|         queries = Person.objects.select_for_update( | ||||
|             of=("self",), | ||||
|         ).values_list(Concat(Value("Dr. "), F("name")), "born") | ||||
|         with transaction.atomic(): | ||||
|             values = queries.get(pk=self.person.pk) | ||||
|         self.assertSequenceEqual(values, ("Dr. Reinhardt", self.city1.pk)) | ||||
|  | ||||
|     @skipUnlessDBFeature("has_select_for_update_of") | ||||
|     def test_for_update_sql_model_inheritance_generated_of(self): | ||||
|         with transaction.atomic(), CaptureQueriesContext(connection) as ctx: | ||||
|   | ||||
		Reference in New Issue
	
	Block a user