AlterField should raise an error when changing primary key

[Csirmaz Bendegúz]

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.

[Natalia Bidart]

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 AlterField doesn't do anything, I started getting errors and it seems like AlterField does do something.

Specifically, when I changed a field in a model from primary_key=True to primary_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:

django.db.utils.ProgrammingError: both default and identity specified for column "id" of table "testapp_testmodel"
LINE 1: ...COLUMN "id" bigint DEFAULT 0 NOT NULL PRIMARY KEY GENERATED ...

What do you mean exactly with AlterField doesn't do anything?

[Csirmaz Bendegúz]

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.

[Sarah Boyce]

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

[Csirmaz Bendegúz]

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

The composite primary keys will also work on SQLite (but not the other backends).

[Csirmaz Bendegúz]

I'm sorry for reopening but could you check again (not on SQLite) and let me know what you think?

[Sarah Boyce]

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?