#31206 closed New feature (duplicate)
Migration crashes when altering a field to use the db_returning flag and default value returned from database.
Reported by: | Tom Kazimiers | Owned by: | Johannes Maron |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 3.0 |
Severity: | Normal | Keywords: | |
Cc: | Johannes Maron | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
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.
Change History (5)
comment:1 by , 5 years ago
Cc: | added |
---|---|
Summary: | Migration fails when a fields has db_returning = True set → Migration crashes when altering a field to use the db_returning flag and default value returned from database. |
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 5 years ago
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.
comment:3 by , 5 years ago
Has patch: | set |
---|---|
Owner: | changed from | to
Status: | new → assigned |
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
comment:4 by , 5 years ago
Has patch: | unset |
---|---|
Resolution: | → duplicate |
Status: | assigned → closed |
Type: | Bug → New feature |
Thanks for this ticket, we should fix handling "effective" defaults in such cases. It's not a release blocker because
db_returning
is still a part of undocumented and private API.