From e67d7d70fa10c06aca36b9057f82054eda45269d Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Sun, 28 Jan 2024 12:02:33 -0500 Subject: [PATCH] Fixed #35149 -- Fixed crashes of db_default with unresolvable output field. Field.db_default accepts either literal Python values or compilables (as_sql) and wrap the former ones in Value internally. While 1e38f11 added support for automatic resolving of output fields for types such as str, int, float, and other unambigous ones it's cannot do so for all types such as dict or even contrib.postgres and contrib.gis primitives. When a literal, non-compilable, value is provided it likely make the most sense to bind its output field to the field its attached to avoid forcing the user to provide an explicit `Value(output_field)`. Thanks David Sanders for the report. --- django/db/backends/base/schema.py | 7 +++---- django/db/models/fields/__init__.py | 21 +++++++++++++-------- docs/releases/5.0.2.txt | 5 +++++ tests/migrations/test_autodetector.py | 4 ++-- tests/migrations/test_operations.py | 12 ++++++------ tests/schema/tests.py | 21 +++++++++++++++++++++ 6 files changed, 50 insertions(+), 20 deletions(-) diff --git a/django/db/backends/base/schema.py b/django/db/backends/base/schema.py index ed91c6ab1b4c..f442d290a005 100644 --- a/django/db/backends/base/schema.py +++ b/django/db/backends/base/schema.py @@ -412,14 +412,13 @@ def db_default_sql(self, field): """Return the sql and params for the field's database default.""" from django.db.models.expressions import Value + db_default = field._db_default_expression sql = ( - self._column_default_sql(field) - if isinstance(field.db_default, Value) - else "(%s)" + self._column_default_sql(field) if isinstance(db_default, Value) else "(%s)" ) query = Query(model=field.model) compiler = query.get_compiler(connection=self.connection) - default_sql, params = compiler.compile(field.db_default) + default_sql, params = compiler.compile(db_default) if self.connection.features.requires_literal_defaults: # Some databases doesn't support parameterized defaults (Oracle, # SQLite). If this is the case, the individual schema backend diff --git a/django/db/models/fields/__init__.py b/django/db/models/fields/__init__.py index 5186f0c414dd..cc5025af84bc 100644 --- a/django/db/models/fields/__init__.py +++ b/django/db/models/fields/__init__.py @@ -219,12 +219,6 @@ def __init__( self.remote_field = rel self.is_relation = self.remote_field is not None self.default = default - if db_default is not NOT_PROVIDED and not hasattr( - db_default, "resolve_expression" - ): - from django.db.models.expressions import Value - - db_default = Value(db_default) self.db_default = db_default self.editable = editable self.serialize = serialize @@ -408,7 +402,7 @@ def _check_db_default(self, databases=None, **kwargs): continue connection = connections[db] - if not getattr(self.db_default, "allowed_default", False) and ( + if not getattr(self._db_default_expression, "allowed_default", False) and ( connection.features.supports_expression_defaults ): msg = f"{self.db_default} cannot be used in db_default." @@ -994,7 +988,7 @@ def pre_save(self, model_instance, add): from django.db.models.expressions import DatabaseDefault if isinstance(value, DatabaseDefault): - return self.db_default + return self._db_default_expression return value def get_prep_value(self, value): @@ -1047,6 +1041,17 @@ def _get_default(self): return return_None return str # return empty string + @cached_property + def _db_default_expression(self): + db_default = self.db_default + if db_default is not NOT_PROVIDED and not hasattr( + db_default, "resolve_expression" + ): + from django.db.models.expressions import Value + + db_default = Value(db_default, self) + return db_default + def get_choices( self, include_blank=True, diff --git a/docs/releases/5.0.2.txt b/docs/releases/5.0.2.txt index a385fbd13eff..6312dee31270 100644 --- a/docs/releases/5.0.2.txt +++ b/docs/releases/5.0.2.txt @@ -36,3 +36,8 @@ Bugfixes * Fixed a bug in Django 5.0 that caused a migration crash on MySQL when adding a ``BinaryField``, ``TextField``, ``JSONField``, or ``GeometryField`` with a ``db_default`` (:ticket:`35162`). + +* Fixed a bug in Django 5.0 that caused a migration crash on models with a + literal ``db_default`` of a complex type such as ``dict`` instance of a + ``JSONField``. Running ``makemigrations`` might generate no-op ``AlterField`` + operations for fields using ``db_default`` (:ticket:`35149`). diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py index c54349313e71..340805b2598e 100644 --- a/tests/migrations/test_autodetector.py +++ b/tests/migrations/test_autodetector.py @@ -1309,7 +1309,7 @@ def test_add_not_null_field_with_db_default(self, mocked_ask_method): changes, "testapp", 0, 0, name="name", preserve_default=True ) self.assertOperationFieldAttributes( - changes, "testapp", 0, 0, db_default=models.Value("Ada Lovelace") + changes, "testapp", 0, 0, db_default="Ada Lovelace" ) @mock.patch( @@ -1515,7 +1515,7 @@ def test_alter_field_to_not_null_with_db_default(self, mocked_ask_method): changes, "testapp", 0, 0, name="name", preserve_default=True ) self.assertOperationFieldAttributes( - changes, "testapp", 0, 0, db_default=models.Value("Ada Lovelace") + changes, "testapp", 0, 0, db_default="Ada Lovelace" ) @mock.patch( diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py index 5733ba761869..f25bb290a58a 100644 --- a/tests/migrations/test_operations.py +++ b/tests/migrations/test_operations.py @@ -1581,7 +1581,7 @@ def test_add_field_database_default(self): self.assertEqual(len(new_state.models[app_label, "pony"].fields), 6) field = new_state.models[app_label, "pony"].fields["height"] self.assertEqual(field.default, models.NOT_PROVIDED) - self.assertEqual(field.db_default, Value(4)) + self.assertEqual(field.db_default, 4) project_state.apps.get_model(app_label, "pony").objects.create(weight=4) self.assertColumnNotExists(table_name, "height") # Add field. @@ -1632,7 +1632,7 @@ def test_add_field_database_default_special_char_escaping(self): self.assertEqual(len(new_state.models[app_label, "pony"].fields), 6) field = new_state.models[app_label, "pony"].fields["special_char"] self.assertEqual(field.default, models.NOT_PROVIDED) - self.assertEqual(field.db_default, Value(db_default)) + self.assertEqual(field.db_default, db_default) self.assertColumnNotExists(table_name, "special_char") with connection.schema_editor() as editor: operation.database_forwards( @@ -1700,7 +1700,7 @@ def test_add_field_both_defaults(self): self.assertEqual(len(new_state.models[app_label, "pony"].fields), 6) field = new_state.models[app_label, "pony"].fields["height"] self.assertEqual(field.default, 3) - self.assertEqual(field.db_default, Value(4)) + self.assertEqual(field.db_default, 4) pre_pony_pk = ( project_state.apps.get_model(app_label, "pony").objects.create(weight=4).pk ) @@ -2145,7 +2145,7 @@ def test_alter_field_add_database_default(self): old_weight = project_state.models[app_label, "pony"].fields["weight"] self.assertIs(old_weight.db_default, models.NOT_PROVIDED) new_weight = new_state.models[app_label, "pony"].fields["weight"] - self.assertEqual(new_weight.db_default, Value(4.5)) + self.assertEqual(new_weight.db_default, 4.5) with self.assertRaises(IntegrityError), transaction.atomic(): project_state.apps.get_model(app_label, "pony").objects.create() # Alter field. @@ -2187,7 +2187,7 @@ def test_alter_field_change_default_to_database_default(self): self.assertIs(old_pink.db_default, models.NOT_PROVIDED) new_pink = new_state.models[app_label, "pony"].fields["pink"] self.assertIs(new_pink.default, models.NOT_PROVIDED) - self.assertEqual(new_pink.db_default, Value(4)) + self.assertEqual(new_pink.db_default, 4) pony = project_state.apps.get_model(app_label, "pony").objects.create(weight=1) self.assertEqual(pony.pink, 3) # Alter field. @@ -2217,7 +2217,7 @@ def test_alter_field_change_nullable_to_database_default_not_null(self): old_green = project_state.models[app_label, "pony"].fields["green"] self.assertIs(old_green.db_default, models.NOT_PROVIDED) new_green = new_state.models[app_label, "pony"].fields["green"] - self.assertEqual(new_green.db_default, Value(4)) + self.assertEqual(new_green.db_default, 4) old_pony = project_state.apps.get_model(app_label, "pony").objects.create( weight=1 ) diff --git a/tests/schema/tests.py b/tests/schema/tests.py index 04ad299aa63a..ced3367f0020 100644 --- a/tests/schema/tests.py +++ b/tests/schema/tests.py @@ -7,6 +7,7 @@ from django.core.exceptions import FieldError from django.core.management.color import no_style +from django.core.serializers.json import DjangoJSONEncoder from django.db import ( DatabaseError, DataError, @@ -2333,6 +2334,26 @@ class Meta: with connection.schema_editor() as editor, self.assertNumQueries(0): editor.alter_field(Author, Author._meta.get_field("name"), new_field) + @isolate_apps("schema") + def test_db_default_output_field_resolving(self): + class Author(Model): + data = JSONField( + encoder=DjangoJSONEncoder, + db_default={ + "epoch": datetime.datetime(1970, 1, 1, tzinfo=datetime.timezone.utc) + }, + ) + + class Meta: + app_label = "schema" + + with connection.schema_editor() as editor: + editor.create_model(Author) + + author = Author.objects.create() + author.refresh_from_db() + self.assertEqual(author.data, {"epoch": "1970-01-01T00:00:00Z"}) + @skipUnlessDBFeature( "supports_column_check_constraints", "can_introspect_check_constraints" )