Opened 3 weeks ago

Last modified 7 days ago

#37006 assigned Bug

Attempting to recreate the PK in a model with no other fields generates a migration that crashes on SQLite

Reported by: Carol Naranjo Owned by: Carol Naranjo
Component: Migrations Version: 6.0
Severity: Normal Keywords: sqlite, migrations
Cc: Simon Charette Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

When a model contains no fields other than the primary key, and the PK is recreated (e.g., renaming and switching from an AutoField to a UUIDField), Django generates a migration with two operations: RemoveField followed by AddField.

In SQLite, applying this migration fails because SQLite cannot create a table with no columns. It only affects SQLite because in other databases the table can be altered by droping the column, whereas SQLite requires that a new table is created and then the data is copied.

Steps to reproduce:

  1. Starting with a model with only the primary key:
    class Place(models.Model):
        pass # PK is created by default as AutoField with column name "id"
    
  1. Modify the primary key:
    class Place(models.Model):
        uuid = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False)
    
  1. Run makemigrations. Which generates following migration:
class Migration(migrations.Migration):

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

    operations = [
        migrations.RemoveField(
            model_name='place',
            name='id',
        ),
        migrations.AddField(
            model_name='place',
            name='uid',
            field=models.AutoField(primary_key=True, serialize=False),
        ),
    ]
  1. Apply the migration using SQLite.

Observed behavior:

During the RemoveField operation, the generated migration tries to create a table with zero non-PK columns, resulting in:

CREATE TABLE "new__myapp_place" (); (params None)
    sqlite3.OperationalError: near ")": syntax error

Expected behavior:

SQLite migration should not crash. Either prevent a migration that would create a table with zero columns or handle it gracefully.

On a side note: This is an edge case—tables without additional fields are rare—but it still causes an unexpected crash.

Change History (10)

comment:1 by Jacob Walls, 3 weeks ago

Component: Database layer (models, ORM)Migrations
Triage Stage: UnreviewedAccepted

Thanks, reproduced at f6167b8bc881babd19b67c004e8f37954afc192e. Looks like another circumstance that manifests the failure described in #24424.

comment:2 by Simon Charette, 3 weeks ago

Related tickets are #22997 and #29790.

The way SQLite requires the table to be rebuilt on field removal and alterations would likely force us to add a _django_empty_col column to support this workflow.

There might be a way to approach this at the auto-detector level by turning a removal and addition of a Field(primary_key=True) with a different name as a [AlterField, RenameField] instead of a [RemoveField, AddField]. Since a model/table can only have one primary key at a time that seems like a better way to approach this problem as it would then allow us to focus our efforts on getting AlterField(from_pk, to_pk) to work in most cases which is what #29790 is about.

in reply to:  2 comment:3 by Carol Naranjo, 3 weeks ago

Replying to Jacob Walls:

Thanks, reproduced at f6167b8bc881babd19b67c004e8f37954afc192e. Looks like another circumstance that manifests the failure described in #24424.

Hi Jacob, can you double check the commit link? it seems unrelated.

---

Replying to Simon Charette:

There might be a way to approach this at the auto-detector level by turning a removal and addition of a Field(primary_key=True) with a different name as a [AlterField, RenameField] instead of a [RemoveField, AddField].

Hi Simon, I like that approach. If it’s not time-sensitive, I’d be happy to work on a fix and assign the ticket to myself.

comment:4 by Jacob Walls, 3 weeks ago

Owner: set to Carol Naranjo
Status: newassigned

Oh it's definitely unrelated. I wasn't blaming that commit, I was just leaving a reference in case it's retriaged a few years from now and we can't reproduce.

If it’s not time-sensitive, I’d be happy to work on a fix and assign the ticket to myself.

Not time-sensitive, thanks for the offer!

comment:5 by Simon Charette, 3 weeks ago

Sure thing Carol, please assign the ticket to yourself and take some time to read the contribution guidelines.

You'll like be looking at code living in autodetector.py and associated tests in tests/migrations/test_autodetector.py.

comment:6 by Carol Naranjo, 3 weeks ago

@Jacob, ah got you, thanks for the clarification :)

Thanks for the hint Simon!

comment:7 by Carol Naranjo, 11 days ago

Has patch: set

Hello Jacob, Simon, I'm back after a short holiday break, happy Easter!

I have now created a PR ready for review here: https://github.com/django/django/pull/21073 . Let me know if something is missing. Thanks!

comment:8 by Simon Charette, 11 days ago

Cc: Simon Charette added

comment:9 by Carol Naranjo, 11 days ago

By the way, regarding the PR tags, it might be worth mentioning that I’m part of the current Djangonauts cohort

comment:10 by Simon Charette, 7 days ago

Patch needs improvement: set

Thanks for the patch Carol, excited to hear you are part of Djangonauts!

I left some comments on the PR. It is heading in the right direction but it should offer the possibility of a rename and not force it when a primary is changed because we won't be able to support all field type alterations so we need an escape hatch to allow the users to drop the field and create a new one when it's the case.

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