[Django] #35589: AlterField should raise an error when changing primary key

[Django]

#35589: AlterField should raise an error when changing primary key
----------------------------------+---------------------------------------
Reporter: Csirmaz Bendegúz | Type: New feature
Status: new | Component: Migrations
Version: dev | Severity: Normal
Keywords: primary key | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+---------------------------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/35589>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#35589: AlterField should raise an error when changing primary key
----------------------------------+--------------------------------------

Reporter: Csirmaz Bendegúz | Owner: (none)
Type: New feature | Status: closed
Component: Migrations | Version: dev
Severity: Normal | Resolution: needsinfo

Keywords: primary key | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+--------------------------------------

Changes (by Natalia Bidart):

* resolution: => needsinfo
* status: new => closed

Comment:

Hello! Thank you for your ticket.

I was about to accept based on Forum discussions
([https://forum.djangoproject.com/t/serialfields-yay-or-nay/32541 0],
[https://forum.djangoproject.com/t/makemigrations-bug-or-not/4012 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''?
--
Ticket URL: <https://code.djangoproject.com/ticket/35589#comment:1>

[Django]

#35589: AlterField should raise an error when changing primary key
----------------------------------+--------------------------------------
Reporter: Csirmaz Bendegúz | Owner: (none)
Type: New feature | Status: closed
Component: Migrations | Version: dev
Severity: Normal | Resolution: needsinfo
Keywords: primary key | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+--------------------------------------

Comment (by Csirmaz Bendegúz):

Replying to [comment:1 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/35589#comment:2>

[Django]

#35589: AlterField should raise an error when changing primary key
----------------------------------+--------------------------------------
Reporter: Csirmaz Bendegúz | Owner: (none)

Type: New feature | Status: new

Component: Migrations | Version: dev
Severity: Normal | Resolution:

Keywords: primary key | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+--------------------------------------

Changes (by Csirmaz Bendegúz):

* resolution: needsinfo =>
* status: closed => new

--
Ticket URL: <https://code.djangoproject.com/ticket/35589#comment:3>

[Django]

#35589: AlterField should raise an error when changing primary key
----------------------------------+--------------------------------------
Reporter: Csirmaz Bendegúz | Owner: (none)

Type: New feature | Status: closed
Component: Migrations | Version: dev
Severity: Normal | Resolution: wontfix

Keywords: primary key | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+--------------------------------------

Changes (by Sarah Boyce):

* resolution: => wontfix

* status: new => closed

Comment:

On SQLite, with a model:
{{{#!python
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:

{{{#!python
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
--
Ticket URL: <https://code.djangoproject.com/ticket/35589#comment:4>

[Django]

#35589: AlterField should raise an error when changing primary key
----------------------------------+--------------------------------------
Reporter: Csirmaz Bendegúz | Owner: (none)
Type: New feature | Status: closed
Component: Migrations | Version: dev

Severity: Normal | Resolution: worksforme

Keywords: primary key | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+--------------------------------------
Changes (by Sarah Boyce):

* resolution: wontfix => worksforme

--
Ticket URL: <https://code.djangoproject.com/ticket/35589#comment:5>

[Django]

#35589: AlterField should raise an error when changing primary key
----------------------------------+--------------------------------------
Reporter: Csirmaz Bendegúz | Owner: (none)
Type: New feature | Status: closed
Component: Migrations | Version: dev
Severity: Normal | Resolution: worksforme
Keywords: primary key | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+--------------------------------------

Comment (by Csirmaz Bendegúz):

Replying to [comment:4 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
--
Ticket URL: <https://code.djangoproject.com/ticket/35589#comment:6>

[Django]

#35589: AlterField should raise an error when changing primary key
----------------------------------+--------------------------------------
Reporter: Csirmaz Bendegúz | Owner: (none)

Type: New feature | Status: new

Component: Migrations | Version: dev
Severity: Normal | Resolution:

Keywords: primary key | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+--------------------------------------

Changes (by Csirmaz Bendegúz):

* resolution: worksforme =>

* status: closed => new

Comment:

I'm sorry for reopening but could you check again (**not** on SQLite) and
let me know what you think?
--
Ticket URL: <https://code.djangoproject.com/ticket/35589#comment:7>

[Django]

#35589: AlterField should raise an error when changing primary key
----------------------------------+--------------------------------------
Reporter: Csirmaz Bendegúz | Owner: (none)

Type: New feature | Status: closed
Component: Migrations | Version: dev
Severity: Normal | Resolution: worksforme

Keywords: primary key | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+--------------------------------------

Changes (by Sarah Boyce):

* resolution: => worksforme

* status: new => closed

Comment:

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:
{{{#!python
class MyModel(models.Model):
flag = models.BooleanField()
some_field = models.CharField(max_length=20)
}}}
Updated `some_field` be a `primary_key` - migration operations
{{{#!python
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?
--
Ticket URL: <https://code.djangoproject.com/ticket/35589#comment:8>