Opened 16 months ago
Closed 15 months ago
#35589 closed New feature (worksforme)
AlterField should raise an error when changing primary key
| Reported by: | Csirmaz Bendegúz | Owned by: | |
|---|---|---|---|
| Component: | Migrations | Version: | dev |
| Severity: | Normal | Keywords: | primary key |
| Cc: | Triage Stage: | Unreviewed | |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
At the moment, AlterField doesn't do anything if changing
primary_key=False -> primary_key=True or
primary_key=True -> primary_key=False.
The same goes for CompositePrimaryKey fields (when/if they are merged #373).
Since changing the primary key is not supported by Django, AlterField should raise an error instead of silently failing.
Change History (8)
follow-up: 2 comment:1 by , 16 months ago
| Resolution: | → needsinfo |
|---|---|
| Status: | new → closed |
comment:2 by , 16 months ago
Replying to Natalia Bidart:
What do you mean exactly with AlterField doesn't do anything?
Thank you for picking up this ticket so quickly!
As far as I understand, it's the AddField that fails in your migration. If you remove AddField, you'll see AlterField doesn't do anything.
comment:3 by , 16 months ago
| Resolution: | needsinfo |
|---|---|
| Status: | closed → new |
follow-up: 6 comment:4 by , 16 months ago
| Resolution: | → wontfix |
|---|---|
| Status: | new → closed |
On SQLite, with a model:
class PkModel(models.Model): field_1 = models.CharField(max_length=255, primary_key=True) field_2 = models.CharField(max_length=255)
The schema is: CREATE TABLE "app1_pkmodel" ("field_2" varchar(255) NOT NULL, "field_1" varchar(255) NOT NULL PRIMARY KEY)
I remove primary_key=True from field_1 and add it to field_2 and makemigrations, the operations generated are:
operations = [ migrations.AlterField( model_name='pkmodel', name='field_1', field=models.CharField(max_length=255), ), migrations.AlterField( model_name='pkmodel', name='field_2', field=models.CharField(max_length=255, primary_key=True, serialize=False), ), ]
This migrates successfully and the schema then becomes: CREATE TABLE "app1_pkmodel" ("field_1" varchar(255) NOT NULL, "field_2" varchar(255) NOT NULL PRIMARY KEY)
I can also migrate backwards successfully and the schema changes back. In the shell, I can create (or not create) data as expected.
Given this, altering a primary key is at the very least supported on SQLite.
Csirmaz, you might have found a specific bug or a backend where this is not supported. We would need more details to replicate this
Given that we support it in some cases, and are actively working on trying to resolve bugs in specific cases (#22997), I think the path forward would be sharing the case you found where it doesn't work and that would be treated as a bug
comment:5 by , 16 months ago
| Resolution: | wontfix → worksforme |
|---|
comment:6 by , 15 months ago
Replying to Sarah Boyce:
Given that we support it in some cases, and are actively working on trying to resolve bugs in specific cases (#22997), I think the path forward would be sharing the case you found where it doesn't work and that would be treated as a bug
Yes, SQLite is an exception, it works on SQLite because the table is "re-made" on AlterField. It doesn't work on any other backend.
I opened this ticket based on our discussion here: https://github.com/django/django/pull/18056#discussion_r1668771284
comment:7 by , 15 months ago
| Resolution: | worksforme |
|---|---|
| Status: | closed → new |
I'm sorry for reopening but could you check again (not on SQLite) and let me know what you think?
comment:8 by , 15 months ago
| Resolution: | → worksforme |
|---|---|
| Status: | new → closed |
I have tried again, this time on Postgres for primary_key=False -> primary_key=True and it worked for me. For example, I started with a model:
class MyModel(models.Model): flag = models.BooleanField() some_field = models.CharField(max_length=20)
Updated some_field be a primary_key - migration operations
operations = [ migrations.RemoveField( model_name='mymodel', name='id', ), migrations.AlterField( model_name='mymodel', name='some_field', field=models.CharField(max_length=20, primary_key=True, serialize=False), ), ]
Migration created and applied successfully, tested result in the shell - looks good to me.
There is #22997 which covers other cases that you might have encountered.
So far I don't see a reason to have primary_key alterations raise an error as they appear to be supported in at least some cases, with tickets for some cases where it isn't supported.
In terms of this conversation https://github.com/django/django/pull/18056#discussion_r1668771284, we can have composite primary keys raise an error on alteration.
If you disagree, can you give me specific examples?
Hello! Thank you for your ticket.
I was about to accept based on Forum discussions (0, 1) and related tickets (#26404) but when I started a small project to confirm that
AlterFielddoesn't do anything, I started getting errors and it seems likeAlterFielddoes do something.Specifically, when I changed a field in a model from
primary_key=Truetoprimary_key=False, the generated migration looks like this:operations = [ migrations.AddField( model_name="testmodel", name="id", field=models.BigAutoField( auto_created=True, default=0, primary_key=True, serialize=False, verbose_name="ID", ), preserve_default=False, ), migrations.AlterField( model_name="testmodel", name="otherid", field=models.IntegerField(), ), ]When applying this migration, it failed with:
What do you mean exactly with AlterField doesn't do anything?