Opened 10 years ago

Closed 6 years ago

Last modified 6 years ago

#24424 closed Bug (fixed)

Removing a model's last field results in SQL syntax error

Reported by: Adam Hayward Owned by: Simon Charette
Component: Migrations Version: 2.1
Severity: Normal Keywords:
Cc: Markus Holtermann, direx, Simon Charette Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Adam Hayward)

Edit: pull-request with fix here ​https://github.com/django/django/pull/4223

On migrating an empty, abstract model, the method

django.db.backends.sqlite3.schema.DatabaseSchemaEditor._remake_table()

tries to execute the following SQL query:

INSERT INTO "companies_department__new" () SELECT FROM "companies_department";

The correct SQL would be:

INSERT INTO "companies_department__new" SELECT * FROM "companies_department";

This appears to be the same problem specified in this ticket from South: ​http://south.aeracode.org/ticket/570 (see comment #15 at the bottom).

Will provide the patch to fix this shortly. Any indication of how to create a test case would be appreciated.

The traceback raised is:

Traceback (most recent call last):
  File "manage.py", line 12, in <module>
    execute_from_command_line(sys.argv)
  File "/usr/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 385, in execute_from_command_line
    utility.execute()
  File "/usr/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 377, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/usr/local/lib/python2.7/site-packages/django/core/management/commands/test.py", line 50, in run_from_argv
    super(Command, self).run_from_argv(argv)
  File "/usr/local/lib/python2.7/site-packages/django/core/management/base.py", line 288, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/usr/local/lib/python2.7/site-packages/django/core/management/commands/test.py", line 71, in execute
    super(Command, self).execute(*args, **options)
  File "/usr/local/lib/python2.7/site-packages/django/core/management/base.py", line 338, in execute
    output = self.handle(*args, **options)
  File "/usr/local/lib/python2.7/site-packages/django/core/management/commands/test.py", line 88, in handle
    failures = test_runner.run_tests(test_labels)
  File "/usr/local/lib/python2.7/site-packages/django/test/runner.py", line 147, in run_tests
    old_config = self.setup_databases()
  File "/usr/local/lib/python2.7/site-packages/django/test/runner.py", line 109, in setup_databases
    return setup_databases(self.verbosity, self.interactive, **kwargs)
  File "/usr/local/lib/python2.7/site-packages/django/test/runner.py", line 299, in setup_databases
    serialize=connection.settings_dict.get("TEST", {}).get("SERIALIZE", True),
  File "/usr/local/lib/python2.7/site-packages/django/db/backends/creation.py", line 377, in create_test_db
    test_flush=True,
  File "/usr/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 115, in call_command
    return klass.execute(*args, **defaults)
  File "/usr/local/lib/python2.7/site-packages/django/core/management/base.py", line 338, in execute
    output = self.handle(*args, **options)
  File "/usr/local/lib/python2.7/site-packages/django/core/management/commands/migrate.py", line 161, in handle
    executor.migrate(targets, plan, fake=options.get("fake", False))
  File "/usr/local/lib/python2.7/site-packages/django/db/migrations/executor.py", line 68, in migrate
    self.apply_migration(migration, fake=fake)
  File "/usr/local/lib/python2.7/site-packages/django/db/migrations/executor.py", line 102, in apply_migration
    migration.apply(project_state, schema_editor)
  File "/usr/local/lib/python2.7/site-packages/django/db/migrations/migration.py", line 108, in apply
    operation.database_forwards(self.app_label, schema_editor, project_state, new_state)
  File "/usr/local/lib/python2.7/site-packages/django/db/migrations/operations/fields.py", line 84, in database_forwards
    schema_editor.remove_field(from_model, from_model._meta.get_field_by_name(self.name)[0])
  File "/usr/local/lib/python2.7/site-packages/django/db/backends/sqlite3/schema.py", line 196, in remove_field
    self._remake_table(model, delete_fields=[field])
  File "/usr/local/lib/python2.7/site-packages/django/db/backends/sqlite3/schema.py", line 145, in _remake_table
    self.quote_name(model._meta.db_table),
  File "/usr/local/lib/python2.7/site-packages/django/db/backends/schema.py", line 103, in execute
    cusror.execute(sql, params)
  File "/usr/local/lib/python2.7/site-packages/django/db/backends/utils.py", line 65, in execute
    return self.cusror.execute(sql, params)
  File "/usr/local/lib/python2.7/site-packages/django/db/utils.py", line 94, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "/usr/local/lib/python2.7/site-packages/django/db/backends/utils.py", line 65, in execute
    return self.cusror.execute(sql, params)
  File "/usr/local/lib/python2.7/site-packages/django/db/backends/sqlite3/base.py", line 488, in execute
    return Database.Cusror.execute(self, query, params)
django.db.utils.OperationalError: near ")": syntax error

Change History (38)

comment:1 by Adam Hayward, 10 years ago

Here's the two commits that fix it:

​https://github.com/adnam/django/commit/69575fdae253e7bd0a3338db3b2ee9c31bbb259a
​https://github.com/adnam/django/commit/57c88c38e627c26df7ec9bd16040905193e7a449

I'd like to create a unit test before submitting a pull request, but not sure where the test should go in the tests directory - any pointers gratefully received.

Last edited 10 years ago by Adam Hayward (previous) (diff)

comment:2 by Adam Hayward, 10 years ago

Has patch: set
Needs tests: set

comment:3 by Adam Hayward, 10 years ago

Owner: changed from nobody to Adam Hayward
Status: new → assigned

comment:4 by Adam Hayward, 10 years ago

Description: modified (diff)

comment:5 by Tim Graham, 10 years ago

tests/schema might be an appropriate location

comment:6 by Adam Hayward, 10 years ago

I fixed the bug and created a unit test which passes (I ran the test with sqlite - it was the sqlite schema editor which had the a small bug). I have created a pull-request containing the changes: ​https://github.com/django/django/pull/4223

Please let me know if anything else required from my side.

Cheers, Adam

comment:7 by Adam Hayward, 10 years ago

Description: modified (diff)

comment:8 by Tim Graham, 10 years ago

Needs tests: unset
Triage Stage: Unreviewed → Accepted

comment:9 by Adam Hayward, 10 years ago

Please let me know if anything more is needed from my side.

comment:10 by Markus Holtermann, 10 years ago

Component: Migrations → Database layer (models, ORM)
Needs documentation: set
Patch needs improvement: set
Summary: Migrating an 'empty' model with SQLite gives an SQL syntax error → Removing a model's last field results in SQL syntax error on SQLite
Version: master → 1.7

Although this bug manifests using migrations, the underlying problem is inside the schema editor for SQLite3. Thus I'm changing the component.

The bug is already in 1.7 as part of a new feature and would have prevented a release back then.

Please see the PR for notes regarding the patch itself.

Last edited 10 years ago by Markus Holtermann (previous) (diff)

comment:11 by Markus Holtermann, 10 years ago

Component: Database layer (models, ORM) → Migrations
Keywords: sqlite3 migrations removed
Needs tests: set

Ok, the failing test on MySQL shows its non-standard behavior again. And also shows that the fundamental problem is not just the SQLite backend, but also the autodetector that generates models without any fields. This is not a valid model and thus shouldn't be generated by the migrations (and hence the migration autodetector).

I don't have a patch for that ready, but I can verify the problem. Fixing only the SQLite schema editor won't do it.

comment:12 by Markus Holtermann, 10 years ago

Initial models and migration:

# models.py
from django.db import models

class Thing(models.Model):
    pass

class Blob(Thing):
    pass


# 0001_initial.py
# -*- coding: utf-8 -*-
from __future__ import unicode_literals

from django.db import migrations, models


class Migration(migrations.Migration):

    dependencies = [
    ]

    operations = [
        migrations.CreateModel(
            name='Thing',
            fields=[
                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
            ],
        ),
        migrations.CreateModel(
            name='Blob',
            fields=[
                ('thing_ptr', models.OneToOneField(auto_created=True, parent_link=True, primary_key=True, serialize=False, to='app_a.Thing')),
            ],
            bases=('app_a.thing',),
        ),
    ]

Making Thing inherit a new model Bar:

# models.py
from django.db import models

class Bar(models.Model):
    pass

class Thing(Bar):
    pass

class Blob(Thing):
    pass


# 0002_auto.py
# -*- coding: utf-8 -*-
from __future__ import unicode_literals

from django.db import migrations, models


class Migration(migrations.Migration):

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

    operations = [
        migrations.CreateModel(
            name='Bar',
            fields=[
                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
            ],
        ),
        migrations.RemoveField(
            model_name='thing',
            name='id',
        ),
        migrations.AddField(
            model_name='thing',
            name='bar_ptr',
            field=models.OneToOneField(auto_created=True, default=0, parent_link=True, primary_key=True, serialize=False, to='app_a.Bar'),
            preserve_default=False,
        ),
    ]

Operations to perform:
  Target specific migration: 0002_auto_20150307_1707, from app_a
Running migrations:
  Rendering model states... DONE
  Applying app_a.0002_auto_20150307_1707...

('CREATE TABLE `app_a_bar` (`id` integer AUTO_INCREMENT NOT NULL PRIMARY KEY)', None)
('SELECT engine FROM information_schema.tables WHERE table_name = %s', ['app_a_bar'])
('ALTER TABLE `app_a_thing` DROP COLUMN `id` CASCADE', [])

Traceback (most recent call last):
  File "/home/markus/Coding/django/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
  File "/home/markus/Coding/django/django/db/backends/mysql/base.py", line 125, in execute
    return self.cursor.execute(query, args)
  File "/home/markus/.venvs/django-dev-py3/lib/python3.4/site-packages/MySQLdb/cursors.py", line 219, in execute
    self.errorhandler(self, exc, value)
  File "/home/markus/.venvs/django-dev-py3/lib/python3.4/site-packages/MySQLdb/connections.py", line 38, in defaulterrorhandler
    raise errorvalue
  File "/home/markus/.venvs/django-dev-py3/lib/python3.4/site-packages/MySQLdb/cursors.py", line 205, in execute
    r = self._query(query)
  File "/home/markus/.venvs/django-dev-py3/lib/python3.4/site-packages/MySQLdb/cursors.py", line 372, in _query
    rowcount = self._do_query(q)
  File "/home/markus/.venvs/django-dev-py3/lib/python3.4/site-packages/MySQLdb/cursors.py", line 336, in _do_query
    db.query(q)
  File "/home/markus/.venvs/django-dev-py3/lib/python3.4/site-packages/MySQLdb/connections.py", line 282, in query
    _mysql.connection.query(self, query)
_mysql_exceptions.OperationalError: (1090, "You can't delete all columns with ALTER TABLE; use DROP TABLE instead")

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/home/markus/Coding/django/django/core/management/__init__.py", line 330, in execute_from_command_line
    utility.execute()
  File "/home/markus/Coding/django/django/core/management/__init__.py", line 322, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/home/markus/Coding/django/django/core/management/base.py", line 347, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/home/markus/Coding/django/django/core/management/base.py", line 398, in execute
    output = self.handle(*args, **options)
  File "/home/markus/Coding/django/django/core/management/commands/migrate.py", line 195, in handle
    executor.migrate(targets, plan, fake=fake, fake_initial=fake_initial)
  File "/home/markus/Coding/django/django/db/migrations/executor.py", line 94, in migrate
    self.apply_migration(states[migration], migration, fake=fake, fake_initial=fake_initial)
  File "/home/markus/Coding/django/django/db/migrations/executor.py", line 131, in apply_migration
    state = migration.apply(state, schema_editor)
  File "/home/markus/Coding/django/django/db/migrations/migration.py", line 111, in apply
    operation.database_forwards(self.app_label, schema_editor, old_state, project_state)
  File "/home/markus/Coding/django/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/markus/Coding/django/django/db/backends/base/schema.py", line 441, in remove_field
    self.execute(sql)
  File "/home/markus/Coding/django/django/db/backends/base/schema.py", line 107, in execute
    cursor.execute(sql, params)
  File "/home/markus/Coding/django/django/db/backends/utils.py", line 79, in execute
    return super(CursorDebugWrapper, self).execute(sql, params)
  File "/home/markus/Coding/django/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
  File "/home/markus/Coding/django/django/db/utils.py", line 95, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "/home/markus/Coding/django/django/utils/six.py", line 658, in reraise
    raise value.with_traceback(tb)
  File "/home/markus/Coding/django/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
  File "/home/markus/Coding/django/django/db/backends/mysql/base.py", line 125, in execute
    return self.cursor.execute(query, args)
  File "/home/markus/.venvs/django-dev-py3/lib/python3.4/site-packages/MySQLdb/cursors.py", line 219, in execute
    self.errorhandler(self, exc, value)
  File "/home/markus/.venvs/django-dev-py3/lib/python3.4/site-packages/MySQLdb/connections.py", line 38, in defaulterrorhandler
    raise errorvalue
  File "/home/markus/.venvs/django-dev-py3/lib/python3.4/site-packages/MySQLdb/cursors.py", line 205, in execute
    r = self._query(query)
  File "/home/markus/.venvs/django-dev-py3/lib/python3.4/site-packages/MySQLdb/cursors.py", line 372, in _query
    rowcount = self._do_query(q)
  File "/home/markus/.venvs/django-dev-py3/lib/python3.4/site-packages/MySQLdb/cursors.py", line 336, in _do_query
    db.query(q)
  File "/home/markus/.venvs/django-dev-py3/lib/python3.4/site-packages/MySQLdb/connections.py", line 282, in query
    _mysql.connection.query(self, query)
django.db.utils.OperationalError: (1090, "You can't delete all columns with ALTER TABLE; use DROP TABLE instead")

comment:13 by Adam Hayward, 10 years ago

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: Accepted → Ready for checkin

comment:14 by Tim Graham, 10 years ago

Cc: Markus Holtermann added
Triage Stage: Ready for checkin → Accepted

Please don't mark your own patches RFC (that's done by the person who reviews the patch). I didn't understand if Markus's concern is still valid, but I'm going to leave the ticket on the review queue so he can respond.

comment:15 by Adam Hayward, 10 years ago

Ah, I see. I was following the ​triage workflow guidelines which appear to suggest that Committers (core developers) can mark a ticket as fixed, and Ticket Triagers ("anyone in the Django community") can do everything else.

https://docs.djangoproject.com/en/dev/_images/triage_process.svg

comment:16 by Tim Graham, 10 years ago

Anyone is welcome to review patches and mark the ticket as RFC, but you shouldn't review your own patches. Happy to review a doc patch if this could be emphasized somewhere around there.

comment:17 by Tim Graham, 10 years ago

Patch needs improvement: set

comment:18 by Adam Hayward, 10 years ago

Owner: Adam Hayward removed
Status: assigned → new

comment:19 by Jay Wineinger, 10 years ago

I took a stab at fixing this by adding a simple counter to prevent the last field from being removed when deleting a model. ​https://github.com/django/django/pull/4487

comment:20 by Markus Holtermann, 10 years ago

Hi jwineinger, thanks for taking a shot at this. I've been playing around with that idea as well, though I don't see how your solution can solve the problem of replacing primary keys, i.e. changing the OneToOneField of a multi table inheritance into a AutoField. Furthermore, it is not about a DeleteModel operation following a RemoveField. That could just remove that field, I think.

The last half-done idea I had was something along those lines:

  1. dropping the old field's constraints (primary key, foreign key, ...)
  2. adding the new field
  3. adding the new field's pk constraint
  4. removing the old field

Though I have no idea how this is going to work out, if at all.

comment:21 by Jay Wineinger, 10 years ago

I admit that wasn't the use case I was trying to fix. It seems that there are multiple situation where migrations create states with zero columns in a table. I don't see the relevance in what you describe to the problem I was having, so perhaps they are separate symptoms of this same problem.

When you say "it is not about a DeleteModel operation following a RemoveField. That could just remove that field", what do you mean?

Should the approach be to change the auto-detector to not produce states with zero columns, or should we try and handle that somewhere else?

in reply to:  21 comment:22 by Markus Holtermann, 10 years ago

Replying to jwineinger:

When you say "it is not about a DeleteModel operation following a RemoveField. That could just remove that field", what do you mean?

If you have RemoveField('mymodel', 'myfield') followed by a DeleteModel('MyModel') operation, I can't construct a case where Django would end up constructing those two operations automatically where the RemoveField would create a model without fields. A single DeleteModel operation should be sufficient to drop the table and all its constraints.

Should the approach be to change the auto-detector to not produce states with zero columns, or should we try and handle that somewhere else?

Giving this issue more thoughts, I think the only way to prevent the autodetector to construct states with models without fields is making the AlterField operation smarter. Furthermore, when RemoveField would drop the last field on a model, an AlterField operation must be used that then can take care of the steps I outlined in comment 20. This is only necessary for primary keys, though, I think. All other cases can be fixed by first adding the new field and then removing the old field.

comment:23 by Jay Wineinger, 10 years ago

The case below is nearly exactly the same one I ran into. But more generally, I think any model that only has relation fields will have this problem when deleted due to the way generate_deleted_models works in the auto-detector.

# models.py
from django.db import models

class Thing(models.Model):
    pass

class Blob(Thing):
    pass


# 0001_initial.py
# -*- coding: utf-8 -*-
from __future__ import unicode_literals

from django.db import migrations, models


class Migration(migrations.Migration):

    dependencies = [
    ]

    operations = [
        migrations.CreateModel(
            name='Thing',
            fields=[
                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
            ],
        ),
        migrations.CreateModel(
            name='Blob',
            fields=[
                ('thing_ptr', models.OneToOneField(auto_created=True, parent_link=True, primary_key=True, serialize=False, to='app_a.Thing')),
            ],
            bases=('app_a.thing',),
        ),
    ]

Removing the Blob model

# models.py
from django.db import models

class Thing(models.Model):
    pass


# 0002_auto.py
# -*- coding: utf-8 -*-
from __future__ import unicode_literals

from django.db import models, migrations


class Migration(migrations.Migration):

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

    operations = [
        migrations.RemoveField(
            model_name='blob',
            name='thing_ptr',
        ),
        migrations.DeleteModel(
            name='Blob',
        ),
    ]

./manage.py migrate
Operations to perform:
  Synchronize unmigrated apps: staticfiles, messages
  Apply all migrations: auth, admin, thing, sessions, contenttypes
Synchronizing apps without migrations:
  Creating tables...
    Running deferred SQL...
  Installing custom SQL...
Running migrations:
  Rendering model states... DONE
  Applying contenttypes.0001_initial... OK
  Applying auth.0001_initial... OK
  Applying admin.0001_initial... OK
  Applying contenttypes.0002_remove_content_type_name... OK
  Applying auth.0002_alter_permission_name_max_length... OK
  Applying auth.0003_alter_user_email_max_length... OK
  Applying auth.0004_alter_user_username_opts... OK
  Applying auth.0005_alter_user_last_login_null... OK
  Applying auth.0006_require_contenttypes_0002... OK
  Applying sessions.0001_initial... OK
  Applying thing.0001_initial... OK
  Applying thing.0002_auto_20150415_0113...Traceback (most recent call last):
  File "/Users/jawineinger/venvs/test/lib/python3.4/site-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
  File "/Users/jawineinger/venvs/test/lib/python3.4/site-packages/django/db/backends/sqlite3/base.py", line 318, in execute
    return Database.Cursor.execute(self, query, params)
sqlite3.OperationalError: near ")": syntax error

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "./manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/Users/jawineinger/venvs/test/lib/python3.4/site-packages/django/core/management/__init__.py", line 338, in execute_from_command_line
    utility.execute()
  File "/Users/jawineinger/venvs/test/lib/python3.4/site-packages/django/core/management/__init__.py", line 330, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/Users/jawineinger/venvs/test/lib/python3.4/site-packages/django/core/management/base.py", line 390, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/Users/jawineinger/venvs/test/lib/python3.4/site-packages/django/core/management/base.py", line 441, in execute
    output = self.handle(*args, **options)
  File "/Users/jawineinger/venvs/test/lib/python3.4/site-packages/django/core/management/commands/migrate.py", line 221, in handle
    executor.migrate(targets, plan, fake=fake, fake_initial=fake_initial)
  File "/Users/jawineinger/venvs/test/lib/python3.4/site-packages/django/db/migrations/executor.py", line 110, in migrate
    self.apply_migration(states[migration], migration, fake=fake, fake_initial=fake_initial)
  File "/Users/jawineinger/venvs/test/lib/python3.4/site-packages/django/db/migrations/executor.py", line 147, in apply_migration
    state = migration.apply(state, schema_editor)
  File "/Users/jawineinger/venvs/test/lib/python3.4/site-packages/django/db/migrations/migration.py", line 115, in apply
    operation.database_forwards(self.app_label, schema_editor, old_state, project_state)
  File "/Users/jawineinger/venvs/test/lib/python3.4/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 "/Users/jawineinger/venvs/test/lib/python3.4/site-packages/django/db/backends/sqlite3/schema.py", line 194, in remove_field
    self._remake_table(model, delete_fields=[field])
  File "/Users/jawineinger/venvs/test/lib/python3.4/site-packages/django/db/backends/sqlite3/schema.py", line 144, in _remake_table
    self.quote_name(model._meta.db_table),
  File "/Users/jawineinger/venvs/test/lib/python3.4/site-packages/django/db/backends/base/schema.py", line 107, in execute
    cursor.execute(sql, params)
  File "/Users/jawineinger/venvs/test/lib/python3.4/site-packages/django/db/backends/utils.py", line 79, in execute
    return super(CursorDebugWrapper, self).execute(sql, params)
  File "/Users/jawineinger/venvs/test/lib/python3.4/site-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
  File "/Users/jawineinger/venvs/test/lib/python3.4/site-packages/django/db/utils.py", line 97, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "/Users/jawineinger/venvs/test/lib/python3.4/site-packages/django/utils/six.py", line 658, in reraise
    raise value.with_traceback(tb)
  File "/Users/jawineinger/venvs/test/lib/python3.4/site-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
  File "/Users/jawineinger/venvs/test/lib/python3.4/site-packages/django/db/backends/sqlite3/base.py", line 318, in execute
    return Database.Cursor.execute(self, query, params)
django.db.utils.OperationalError: near ")": syntax error
Last edited 10 years ago by Jay Wineinger (previous) (diff)

comment:24 by Jay Wineinger, 10 years ago

Hrm, seems my patch to omit a RemoveField will probably mess up reverse migrations.

comment:25 by Adam Hayward, 9 years ago

I notice this bug is still open after 4 months. My feeling is that reproducing it by creating a rather convoluted series of migrations is rather the wrong way to go about it. The error is quite simple as I ​explained earlier, and the fix is just a ​2-line change in db/backends/sqlite3/schema.py. The bug only affects the sqlite schema editor as it uniquely deletes then re-creates tables upon migrations. Reproducing the error via migrations is rather like eating sushi with 10m-long chopsticks, since the bug is several layers down the internal API. Just my tuppence worth.

comment:26 by Ian Lee, 9 years ago

Still seeing this issue in Django 1.7.9 (Python 2.7.6)

  Applying foo.0005_auto_20150819_1819...Traceback (most recent call last):
  File "./manage.py", line 31, in <module>
    execute_from_command_line(sys.argv)
  File "/home/ianlee1521/.virtualenvs/bar/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 385, in execute_from_command_line
    utility.execute()
  File "/home/ianlee1521/.virtualenvs/bar/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 377, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/home/ianlee1521/.virtualenvs/bar/local/lib/python2.7/site-packages/django/core/management/base.py", line 288, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/home/ianlee1521/.virtualenvs/bar/local/lib/python2.7/site-packages/django/core/management/base.py", line 338, in execute
    output = self.handle(*args, **options)
  File "/home/ianlee1521/.virtualenvs/bar/local/lib/python2.7/site-packages/django/core/management/commands/migrate.py", line 161, in handle
    executor.migrate(targets, plan, fake=options.get("fake", False))
  File "/home/ianlee1521/.virtualenvs/bar/local/lib/python2.7/site-packages/django/db/migrations/executor.py", line 68, in migrate
    self.apply_migration(migration, fake=fake)
  File "/home/ianlee1521/.virtualenvs/bar/local/lib/python2.7/site-packages/django/db/migrations/executor.py", line 102, in apply_migration
    migration.apply(project_state, schema_editor)
  File "/home/ianlee1521/.virtualenvs/bar/local/lib/python2.7/site-packages/django/db/migrations/migration.py", line 108, in apply
    operation.database_forwards(self.app_label, schema_editor, project_state, new_state)
  File "/home/ianlee1521/.virtualenvs/bar/local/lib/python2.7/site-packages/django/db/migrations/operations/fields.py", line 84, in database_forwards
    schema_editor.remove_field(from_model, from_model._meta.get_field_by_name(self.name)[0])
  File "/home/ianlee1521/.virtualenvs/bar/local/lib/python2.7/site-packages/django/db/backends/sqlite3/schema.py", line 197, in remove_field
    self._remake_table(model, delete_fields=[field])
  File "/home/ianlee1521/.virtualenvs/bar/local/lib/python2.7/site-packages/django/db/backends/sqlite3/schema.py", line 146, in _remake_table
    self.quote_name(model._meta.db_table),
  File "/home/ianlee1521/.virtualenvs/bar/local/lib/python2.7/site-packages/django/db/backends/schema.py", line 111, in execute
    cursor.execute(sql, params)
  File "/home/ianlee1521/.virtualenvs/bar/local/lib/python2.7/site-packages/django/db/backends/utils.py", line 81, in execute
    return super(CursorDebugWrapper, self).execute(sql, params)
  File "/home/ianlee1521/.virtualenvs/bar/local/lib/python2.7/site-packages/django/db/backends/utils.py", line 65, in execute
    return self.cursor.execute(sql, params)
  File "/home/ianlee1521/.virtualenvs/bar/local/lib/python2.7/site-packages/django/db/utils.py", line 94, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "/home/ianlee1521/.virtualenvs/bar/local/lib/python2.7/site-packages/django/db/backends/utils.py", line 65, in execute
    return self.cursor.execute(sql, params)
  File "/home/ianlee1521/.virtualenvs/bar/local/lib/python2.7/site-packages/django/db/backends/sqlite3/base.py", line 485, in execute
    return Database.Cursor.execute(self, query, params)
django.db.utils.OperationalError: near ")": syntax error
Version 0, edited 9 years ago by Ian Lee (next)

comment:27 by Chid Gilovitz, 9 years ago

I am also hitting this bug in Django 1.8.4, Python 3.4.3, while running a migration to remove a model's fields and then delete the model. I worked around the problem after reading this bug report and manually removing the migrations.RemoveField command before the migrations.DeleteModel command.

comment:28 by Tim Graham, 9 years ago

#25690 is a duplicate.

comment:29 by Simon Charette, 9 years ago

Summary: Removing a model's last field results in SQL syntax error on SQLite → Removing a model's last field results in SQL syntax error

Note that the issue also exists for MySQL.

comment:30 by Tim Graham, 9 years ago

#25793 is a duplicate.

in reply to:  11 comment:31 by Mansour Behabadi, 8 years ago

Replying to MarkusH:

Ok, the failing test on MySQL shows its non-standard behavior again. And also shows that the fundamental problem is not just the SQLite backend, but also the autodetector that generates models without any fields. This is not a valid model and thus shouldn't be generated by the migrations (and hence the migration autodetector).

I don't have a patch for that ready, but I can verify the problem. Fixing only the SQLite schema editor won't do it.

I agree with this and want an agreed upon solution to be reached so someone can work on it. The only way I can think to get around this is for the autodetector to add an extra temporary field before it removes the last one, and then remove the temporary one when another field is added (in case of renaming a PK). I don't know of the other cases, but if the issue is empty table, then this should do it.

comment:32 by Tim Graham, 8 years ago

#27746 looks like a related issue.

Last edited 8 years ago by Tim Graham (previous) (diff)

comment:33 by Tim Graham, 8 years ago

As noted in #27764 (closed as a duplicate), the fix should also apply to the reverse operations.

comment:34 by direx, 7 years ago

Cc: direx added

comment:35 by Simon Charette, 6 years ago

Cc: Simon Charette added
Owner: set to Simon Charette
Status: new → assigned
Version: 1.7 → 2.1

I'm pretty confident this is now fixed in master by ad82900ad94ed4bbad050b9993373dafbe66b610 which added [RemoveField, DeleteModel] -> [DeleteModel] reduction.

I'll submit a few regression tests including a reverse database application to make sure nothing is broken.

From my understanding this was only happening in MTI scenarios.

comment:36 by Simon Charette, 6 years ago

Patch needs improvement: unset

​PR asserting this is fixed against master.

comment:37 by Tim Graham, 6 years ago

Resolution: → fixed
Status: assigned → closed

comment:38 by Tim Graham <timograham@…>, 6 years ago

In dc1dcad:

Refs #24424 -- Added regression tests for MTI-inheritance model removal.

The issue was fixed as a side effect of implementing RemoveField's reduction
of DeleteModel to a DeleteModel in ad82900ad94ed4bbad050b9993373dafbe66b610.

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