[Django] #31206: Migration fails when a fields has db_returning = True set
Django
-------------------------------------+-------------------------------------
Reporter: Tom | Owner: nobody
Kazimiers |
Type: Bug | Status: new
Component: Database | Version: 3.0
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 |
-------------------------------------+-------------------------------------
I was happy to see PR 9983 merged before Django 3 was released, because it
adds the `db_returning` attribute to the `Field` classes. This makes it
possible to return multiple fields from an INSERT or UPDATE, which is
especially useful to use database transaction timestamps (see issue
#29444).
It seems to work very well, except for migrations: if I have a field like
`CreatedField()` as shown in `tests/queries/models.py` (part of the
original PR), and create a new migration that sets the new default value
on the field (e.g. `django.contrib.postgres.functions.TransactionNow`),
`manage.py migrate` fails with this error:
{{{
File "./manage.py", line 11, in <module>
execute_from_command_line(sys.argv)
File "/home/tom/.virtualenvs/catmaid/lib/python3.6/site-
packages/django/core/management/__init__.py", line 401, in
execute_from_command_line
utility.execute()
File "/home/tom/.virtualenvs/catmaid/lib/python3.6/site-
packages/django/core/management/__init__.py", line 395, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "/home/tom/.virtualenvs/catmaid/lib/python3.6/site-
packages/django/core/management/base.py", line 328, in run_from_argv
self.execute(*args, **cmd_options)
File "/home/tom/.virtualenvs/catmaid/lib/python3.6/site-
packages/django/core/management/base.py", line 369, in execute
output = self.handle(*args, **options)
File "/home/tom/.virtualenvs/catmaid/lib/python3.6/site-
packages/django/core/management/base.py", line 83, in wrapped
res = handle_func(*args, **kwargs)
File "/home/tom/.virtualenvs/catmaid/lib/python3.6/site-
packages/django/core/management/commands/migrate.py", line 233, in handle
fake_initial=fake_initial,
File "/home/tom/.virtualenvs/catmaid/lib/python3.6/site-
packages/django/db/migrations/executor.py", line 117, in migrate
state = self._migrate_all_forwards(state, plan, full_plan, fake=fake,
fake_initial=fake_initial)
File "/home/tom/.virtualenvs/catmaid/lib/python3.6/site-
packages/django/db/migrations/executor.py", line 147, in
_migrate_all_forwards
state = self.apply_migration(state, migration, fake=fake,
fake_initial=fake_initial)
File "/home/tom/.virtualenvs/catmaid/lib/python3.6/site-
packages/django/db/migrations/executor.py", line 245, in apply_migration
state = migration.apply(state, schema_editor)
File "/home/tom/.virtualenvs/catmaid/lib/python3.6/site-
packages/django/db/migrations/migration.py", line 124, in apply
operation.database_forwards(self.app_label, schema_editor, old_state,
project_state)
File "/home/tom/.virtualenvs/catmaid/lib/python3.6/site-
packages/django/db/migrations/operations/fields.py", line 249, in
database_forwards
schema_editor.alter_field(from_model, from_field, to_field)
File "/home/tom/.virtualenvs/catmaid/lib/python3.6/site-
packages/django/db/backends/base/schema.py", line 565, in alter_field
old_db_params, new_db_params, strict)
File "/home/tom/.virtualenvs/catmaid/lib/python3.6/site-
packages/django/db/backends/postgresql/schema.py", line 154, in
_alter_field
new_db_params, strict,
File "/home/tom/.virtualenvs/catmaid/lib/python3.6/site-
packages/django/db/backends/base/schema.py", line 679, in _alter_field
new_default = self.effective_default(new_field)
File "/home/tom/.virtualenvs/catmaid/lib/python3.6/site-
packages/django/db/backends/base/schema.py", line 303, in
effective_default
return field.get_db_prep_save(self._effective_default(field),
self.connection)
File "/home/tom/.virtualenvs/catmaid/lib/python3.6/site-
packages/django/db/models/fields/__init__.py", line 817, in
get_db_prep_save
return self.get_db_prep_value(value, connection=connection,
prepared=False)
File "/home/tom/.virtualenvs/catmaid/lib/python3.6/site-
packages/django/db/models/fields/__init__.py", line 1372, in
get_db_prep_value
value = self.get_prep_value(value)
File "/home/tom/.virtualenvs/catmaid/lib/python3.6/site-
packages/django/db/models/fields/__init__.py", line 1351, in
get_prep_value
value = super().get_prep_value(value)
File "/home/tom/.virtualenvs/catmaid/lib/python3.6/site-
packages/django/db/models/fields/__init__.py", line 1211, in
get_prep_value
return self.to_python(value)
File "/home/tom/.virtualenvs/catmaid/lib/python3.6/site-
packages/django/db/models/fields/__init__.py", line 1312, in to_python
parsed = parse_datetime(value)
File "/home/tom/.virtualenvs/catmaid/lib/python3.6/site-
packages/django/utils/dateparse.py", line 107, in parse_datetime
match = datetime_re.match(value)
TypeError: expected string or bytes-like object
}}}
Of course, there is technically no change required on the database level
during such a migration. Therefore it seems fine to only run those default
updating changes as `state_operations` during a migration, and don't have
Django execute them as regular operations:
{{{
class Migration(migrations.Migration):
dependencies = [
('catmaid', '0099_make_concept_ids_64_bit'),
]
operations = [
migrations.RunSQL(migrations.RunSQL.noop, migrations.RunSQL.noop,
[
migrations.AlterField(
model_name='cardinalityrestriction',
name='creation_time',
field=catmaid.fields.DbDefaultDateTimeField(default=django.contrib.postgres.functions.TransactionNow),
),
]),
]
}}}
This works for now as workaround, but it would be nice if Django could
figure this out on its own.
--
Ticket URL: <https://code.djangoproject.com/ticket/31206>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
default value returned from database.
-------------------------------------+-------------------------------------
Reporter: Tom Kazimiers | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 3.0
(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
-------------------------------------+-------------------------------------
* cc: Johannes Hoppe (added)
* stage: Unreviewed => Accepted
Comment:
Thanks for this ticket, we should fix
[https://github.com/django/django/blob/5d654e1e7104d2ce86ec1b9fe52865a7dca4b4be/django/db/backends/base/schema.py#L671-L686
handling "effective" defaults] in such cases. It's not a release blocker
because `db_returning` is still a part of undocumented and private API.
--
Ticket URL: <https://code.djangoproject.com/ticket/31206#comment:1>
Django
default value returned from database.
-------------------------------------+-------------------------------------
Reporter: Tom Kazimiers | Owner: nobody
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Ashutosh Chauhan):
I would like to work on this issue. Can anyone help me where to start. I
would look into the ticket:29444 to get some idea.
--
Ticket URL: <https://code.djangoproject.com/ticket/31206#comment:2>
Django
default value returned from database.
-------------------------------------+-------------------------------------
| Hoppe
Type: Bug | Status: assigned
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Johannes Hoppe
* status: new => assigned
* has_patch: 0 => 1
Comment:
Hi there,
while looking into it, I discovered a simple fix as well as a way to test
it. So please excuse me snatching that ticket. But it was simpler to just
push my branch instead of explaining how to fix it.
Best
-Joe
--
Ticket URL: <https://code.djangoproject.com/ticket/31206#comment:3>
Django
default value returned from database.
-------------------------------------+-------------------------------------
Reporter: Tom Kazimiers | Owner: Johannes
| Hoppe
Component: Database layer | Version: 3.0
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* type: Bug => New feature
* has_patch: 1 => 0
* resolution: => duplicate
Comment:
After reconsideration I'm closing this ticket as a duplicate of #30032,
because we don't officially support using expressions in `Field.default`.
This should be fixed as a part of #30032.
--
Ticket URL: <https://code.djangoproject.com/ticket/31206#comment:4>
Django
default value returned from database.
-------------------------------------+-------------------------------------
Reporter: Tom Kazimiers | Owner: Johannes
| Hoppe
Type: New feature | Status: closed
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Johannes Hoppe):
Ok, I will include it in the default patch.
--
Ticket URL: <https://code.djangoproject.com/ticket/31206#comment:5>