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:
- 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"
- Modify the primary key:
class Place(models.Model): uuid = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False)
- 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),
),
]
- 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 , 3 weeks ago
| Component: | Database layer (models, ORM) → Migrations |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
follow-up: 3 comment:2 by , 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.
comment:3 by , 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 , 3 weeks ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
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 , 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 , 3 weeks ago
@Jacob, ah got you, thanks for the clarification :)
Thanks for the hint Simon!
comment:7 by , 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 , 11 days ago
| Cc: | added |
|---|
comment:9 by , 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 , 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.
Thanks, reproduced at f6167b8bc881babd19b67c004e8f37954afc192e. Looks like another circumstance that manifests the failure described in #24424.