Opened 17 months ago

Closed 17 months ago

Last modified 17 months ago

#22260 closed Bug (fixed)

Custom model field: field.db_type not called for (initial) migration / docs wrong? (DurationField)

Reported by: blueyed Owned by: nobody
Component: Migrations Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

While trying to use the DurationField with Django 1.7 I have noticed that the field is silently ignored from being added to the database.

While the initial migration is correct, the lookup for the db field type fails.

The docs state:

The db_type() method is called by Django when the framework constructs the CREATE TABLE statements for your application – that is, when you first create your tables.

This does not appear to be the case with migrations however, which only call field.db_parameters:

…/django-master/django/db/migrations/executor.py(60)migrate()

58 for migration, backwards in plan:
59 if not backwards:

---> 60 self.apply_migration(migration, fake=fake)

61 else:
62 self.unapply_migration(migration, fake=fake)

…/django-master/django/db/migrations/executor.py(95)apply_migration()

93 import ipdb; ipdb.set_trace()
94 project_state = self.loader.graph.project_state((migration.app_label, migration.name), at_end=False)

---> 95 migration.apply(project_state, schema_editor)

96 # For replacement migrations, record individual statuses
97 if migration.replaces:

…/django-master/django/db/migrations/migration.py(97)apply()

95 operation.state_forwards(self.app_label, new_state)
96 # Run the operation

---> 97 operation.database_forwards(self.app_label, schema_editor, project_state, new_state)

98 # Switch states
99 project_state = new_state

…/django-master/django/db/migrations/operations/models.py(28)database_forwards()

26 model = apps.get_model(app_label, self.name)
27 if router.allow_migrate(schema_editor.connection.alias, model):

---> 28 schema_editor.create_model(model)

29
30 def database_backwards(self, app_label, schema_editor, from_state, to_state):

…/django-master/django/db/backends/schema.py(191)create_model()

189 for field in model._meta.local_fields:
190 # SQL

--> 191 definition, extra_params = self.column_sql(model, field)

192 if definition is None:
193 continue

…/django-master/django/db/backends/schema.py(108)column_sql()

106 """
107 # Get the column's type and use that as the basis of the SQL

--> 108 db_params = field.db_parameters(connection=self.connection)

109 sql = db_paramstype?
110 params = []

From the initial migration:

('runtime', durationfield.db.models.fields.duration.DurationField(help_text=u'Format: HH:MM:SS', null=True, verbose_name=u'Playtime')),

Commit e5983af (from 2012) changed BaseDatabaseSchemaEditor from using db_type to db_parameters.

(In commit 1a9f13d the documentation got improved somehow, but appears to still reflect not the new/different behavior using migrations).

I have filed an issue for django-durationfield.

The source for the custom model field: duration.py.

Let me know if I should provide a simple test case / test app.

While at it, I think that there should have been a warning or assertion being triggered in this case. I have seen that the code considers this case to be a m2m field (or something similar), but those might look different at that point - and an assertion could be made there.

Change History (5)

comment:1 Changed 17 months ago by shai

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

This seems related to #22001.

comment:2 Changed 17 months ago by blueyed

The commit introducing db_parameters (mentioned above, but linked incorrectly) is https://github.com/django/django/commit/ca9c3cd.

comment:3 Changed 17 months ago by blueyed

  • Has patch set
  • Needs tests set

comment:4 Changed 17 months ago by blueyed

  • Resolution set to duplicate
  • Status changed from new to closed

The fix for this is now in the pull request for #22001.

comment:5 Changed 17 months ago by Marc Tamlyn <marc.tamlyn@…>

  • Resolution changed from duplicate to fixed

In d22b291890c1736a40c0ad97448c7318df2eebb2:

Fixed #22001 -- Ensure db_type is respected.

db_parameters should respect an already existing db_type method and
return that as its type string. In particular, this was causing some
fields from gis to not be generated.

Thanks to @bigsassy and @blueyed for their work on the patch.

Also fixed #22260

Note: See TracTickets for help on using tickets.
Back to Top