Opened 8 months ago

Last modified 8 months ago

#27746 new Bug

Database migration fail when removing a child model containing only foreignkeys in a multi-table inheritance context on MySQL

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

Description

Context

  • I'm not observing this bug when not using multi-table inheritance.
  • I'm using MySQL (MariaDB-10.0), and i don't know if the bug happen with another database backend or server.
  • In an existing project named bobb
  • A similary bug exist with empty child models too, like Account or Device

Prepare the bug

python manage.py startapp buggymigration
echo "INSTALLED_APPS = tuple(list(INSTALLED_APPS) + ['buggymigration'])" >> bobb/settings.py

# creating models using multi-table in heritance in buggymigration
# by making changes to buggymigration/models.py

cat buggymigration/models.py
from __future__ import unicode_literals

from django.db import models

class UniqueSerialNumber(models.Model):
    pass

class Account(UniqueSerialNumber):
    pass

class Device(UniqueSerialNumber):
    pass

class Contract(UniqueSerialNumber):
    customer = models.ForeignKey('Account')
    machine = models.ForeignKey('Device')
python manage.py makemigrations
Migrations for 'buggymigration':
  0001_initial.py:
    - Create model UniqueSerialNumber
    - Create model Account
    - Create model Contract
    - Create model Device
    - Add field machine to contract
python manage.py migrate
Operations to perform:
  Apply all migrations: sessions, admin, auth, buggymigration, contenttypes
Running migrations:
  Applying buggymigration.0001_initial... OK

Trigger the bug

# trying to remove the model Contract
# by editing buggymigration/models.py and commenting the model

cat buggymigration/models.py
from __future__ import unicode_literals

from django.db import models

class UniqueSerialNumber(models.Model):
    pass

class Account(UniqueSerialNumber):
    pass

class Device(UniqueSerialNumber):
    pass

'''
class Contract(UniqueSerialNumber):
    customer = models.ForeignKey('Account')
    machine = models.ForeignKey('Device')
'''
python manage.py makemigrations
Migrations for 'buggymigration':
  0002_auto_20170118_1258.py:
    - Remove field customer from contract
    - Remove field machine from contract
    - Remove field uniqueserialnumber_ptr from contract
    - Delete model Contract
cat buggymigration/migrations/0002_auto_20170118_1258.py
# -*- coding: utf-8 -*-
# Generated by Django 1.9.12 on 2017-01-18 11:58
from __future__ import unicode_literals

from django.db import migrations


class Migration(migrations.Migration):

    dependencies = [
        ('buggymigration', '0001_initial'),
    ]

    operations = [
        migrations.RemoveField(
            model_name='contract',
            name='customer',
        ),
        migrations.RemoveField(
            model_name='contract',
            name='machine',
        ),
        migrations.RemoveField(
            model_name='contract',
            name='uniqueserialnumber_ptr',
        ),
        migrations.DeleteModel(
            name='Contract',
        ),
    ]
python manage.py sqlmigrate buggymigration 0002_auto_20170118_1258
BEGIN;
--
-- Remove field customer from contract
--
ALTER TABLE `buggymigration_contract` DROP FOREIGN KEY `D5e39daefe3dcc06188b9ecc2ad1e28a`;
ALTER TABLE `buggymigration_contract` DROP COLUMN `customer_id` CASCADE;
--
-- Remove field machine from contract
--
ALTER TABLE `buggymigration_contract` DROP FOREIGN KEY `a46c0a1038d68a3574efaf2d757512fd`;
ALTER TABLE `buggymigration_contract` DROP COLUMN `machine_id` CASCADE;
--
-- Remove field uniqueserialnumber_ptr from contract
--
ALTER TABLE `buggymigration_contract` DROP FOREIGN KEY `f406fa7acbef3134feeae510d6e9affd`;
ALTER TABLE `buggymigration_contract` DROP COLUMN `uniqueserialnumber_ptr_id` CASCADE;
--
-- Delete model Contract
--
DROP TABLE `buggymigration_contract` CASCADE;

COMMIT;

The bug

python manage.py migrate buggymigration
Operations to perform:
  Apply all migrations: buggymigration
Running migrations:
  Applying buggymigration.0002_auto_20170118_1258...Traceback (most recent call last):
  File "manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/home/backoffice/virtualenv/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 353, in execute_from_command_line
    utility.execute()
  File "/home/backoffice/virtualenv/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 345, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/home/backoffice/virtualenv/local/lib/python2.7/site-packages/django/core/management/base.py", line 348, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/home/backoffice/virtualenv/local/lib/python2.7/site-packages/django/core/management/base.py", line 399, in execute
    output = self.handle(*args, **options)
  File "/home/backoffice/virtualenv/local/lib/python2.7/site-packages/django/core/management/commands/migrate.py", line 200, in handle
    executor.migrate(targets, plan, fake=fake, fake_initial=fake_initial)
  File "/home/backoffice/virtualenv/local/lib/python2.7/site-packages/django/db/migrations/executor.py", line 92, in migrate
    self._migrate_all_forwards(plan, full_plan, fake=fake, fake_initial=fake_initial)
  File "/home/backoffice/virtualenv/local/lib/python2.7/site-packages/django/db/migrations/executor.py", line 121, in _migrate_all_forwards
    state = self.apply_migration(state, migration, fake=fake, fake_initial=fake_initial)
  File "/home/backoffice/virtualenv/local/lib/python2.7/site-packages/django/db/migrations/executor.py", line 198, in apply_migration
    state = migration.apply(state, schema_editor)
  File "/home/backoffice/virtualenv/local/lib/python2.7/site-packages/django/db/migrations/migration.py", line 123, in apply
    operation.database_forwards(self.app_label, schema_editor, old_state, project_state)
  File "/home/backoffice/virtualenv/local/lib/python2.7/site-packages/django/db/migrations/operations/fields.py", line 121, in database_forwards
    schema_editor.remove_field(from_model, from_model._meta.get_field(self.name))
  File "/home/backoffice/virtualenv/local/lib/python2.7/site-packages/django/db/backends/base/schema.py", line 438, in remove_field
    self.execute(sql)
  File "/home/backoffice/virtualenv/local/lib/python2.7/site-packages/django/db/backends/base/schema.py", line 110, in execute
    cursor.execute(sql, params)
  File "/home/backoffice/virtualenv/local/lib/python2.7/site-packages/django/db/backends/utils.py", line 79, in execute
    return super(CursorDebugWrapper, self).execute(sql, params)
  File "/home/backoffice/virtualenv/local/lib/python2.7/site-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
  File "/home/backoffice/virtualenv/local/lib/python2.7/site-packages/django/db/utils.py", line 95, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "/home/backoffice/virtualenv/local/lib/python2.7/site-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
  File "/home/backoffice/virtualenv/local/lib/python2.7/site-packages/django/db/backends/mysql/base.py", line 112, in execute
    return self.cursor.execute(query, args)
  File "/home/backoffice/virtualenv/local/lib/python2.7/site-packages/MySQLdb/cursors.py", line 205, in execute
    self.errorhandler(self, exc, value)
  File "/home/backoffice/virtualenv/local/lib/python2.7/site-packages/MySQLdb/connections.py", line 36, in defaulterrorhandler
    raise errorclass, errorvalue
django.db.utils.OperationalError: (1090, "You can't delete all columns with ALTER TABLE; use DROP TABLE instead")

Change History (5)

comment:1 Changed 8 months ago by Tim Graham

Resolution: duplicate
Status: newclosed

Duplicate of #24424.

comment:2 Changed 8 months ago by Simon Charette

Resolution: duplicate
Status: closednew
Triage Stage: UnreviewedAccepted
Version: 1.9master

Tim, I re-opened the ticket because I would expect the auto-detector to generate a single migrations.DeleteModel in this case.

#24424 is related in the sense that making sure that altered tables always have a column would prevent the reported crash but issuing a DROP COLUMN for every single fields a model has to finally DROP the table doesn't make much sense.

comment:3 Changed 8 months ago by Tim Graham

Component: Database layer (models, ORM)Migrations

I thought the conclusion of ticket:24424#comment:11 was that it's also an issue with the autodetector. Maybe a different one, I guess.

comment:4 in reply to:  3 Changed 8 months ago by Simon Charette

Replying to Tim Graham:

I thought the conclusion of ticket:24424#comment:11 was that it's also an issue with the autodetector.

Indeed but the only way we'll be able to solve #24424 is by teaching the auto-detector to either issue an AlterField or a combination of equivalent operations instead of a RemoveField + AddField in order to convert the id auto-incrementing field primay key to integer foreign key primary key.

There's a side discussion about the issue encountered here from comments 19 to 22 on #24424. I believe the solution here would be to teach field operations to optimize the [FieldOperation('model', 'field'), RemoveModel('model')] case to [RemoveModel("model')].

comment:5 Changed 8 months ago by Simon Charette

It looks like this can't be done at the optimizer level as not all RemoveField operations can be fold into DeleteModel ones. For example, the RemoveField operations removing many-to-many fields cannot be optimized (as the schema editor's delete_model function doesn't deal with intermediary table deletions) but there's no way to figure out whether or not a RemoveField operation removes a many-to-many field or not from its reduce() method context.

I believe the method that requires adjustments is the MigrationAutodetector.generate_deleted_models() one.

Last edited 8 months ago by Simon Charette (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top