Skip to content

Commit

Permalink
Fixed #35149 -- Fixed crashes of db_default with unresolvable output …
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
charettes authored and felixxm committed Feb 4, 2024
1 parent fe1cb62 commit e67d7d7
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 20 deletions.
7 changes: 3 additions & 4 deletions django/db/backends/base/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 13 additions & 8 deletions django/db/models/fields/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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."
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 5 additions & 0 deletions docs/releases/5.0.2.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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`).
4 changes: 2 additions & 2 deletions tests/migrations/test_autodetector.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
12 changes: 6 additions & 6 deletions tests/migrations/test_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
)
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
)
Expand Down
21 changes: 21 additions & 0 deletions tests/schema/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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"
)
Expand Down

0 comments on commit e67d7d7

Please sign in to comment.