[Django] #35551: Unique checks skipped when changing primary key

[Django]

#35551: Unique checks skipped when changing primary key
--------------------------------------------+------------------------
Reporter: Csirmaz Bendegúz | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: dev
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
--------------------------------------------+------------------------
I noticed the unique checks are skipped ({{{_perform_unique_checks}}})
when an object's primary key is changed.

Given:

{{{
# models.py
class Dummy(models.Model):
id = models.IntegerField(primary_key=True)

# admin.py
admin.site.register(Dummy)
}}}

1. Create Dummy (1), Dummy (2).
2. Change Dummy (2)'s Id field -> 1
3. No error, unique checks skipped
4. Change Dummy (1)'s Id field -> 3
5. A new Dummy (1) is created (why?)
--
Ticket URL: <https://code.djangoproject.com/ticket/35551>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#35551: Unique checks skipped when changing primary key

----------------------------------+--------------------------------------

Reporter: Csirmaz Bendegúz | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: dev

Severity: Normal | Resolution:

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

----------------------------------+--------------------------------------
Description changed by Csirmaz Bendegúz:

Old description:

> I noticed the unique checks are skipped ({{{_perform_unique_checks}}})
> when an object's primary key is changed.
>
> Given:
>
> {{{
> # models.py
> class Dummy(models.Model):
> id = models.IntegerField(primary_key=True)
>
> # admin.py
> admin.site.register(Dummy)
> }}}
>
> 1. Create Dummy (1), Dummy (2).
> 2. Change Dummy (2)'s Id field -> 1
> 3. No error, unique checks skipped
> 4. Change Dummy (1)'s Id field -> 3
> 5. A new Dummy (1) is created (why?)

New description:

I noticed the unique checks are skipped ({{{_perform_unique_checks}}})
when an object's primary key is changed.

Given:

{{{
# models.py
class Dummy(models.Model):
id = models.IntegerField(primary_key=True)

# admin.py
admin.site.register(Dummy)
}}}

1. Create Dummy (1), Dummy (2).
2. Change Dummy (2)'s Id field -> 1
3. No error, unique checks skipped
4. Change Dummy (1)'s Id field -> 3
5. A new Dummy (1) is created (why?)

I believe step 5 should never happen, since step 3 should fail with a
unique check error.

--
--
Ticket URL: <https://code.djangoproject.com/ticket/35551#comment:1>

[Django]

#35551: Unique checks skipped when changing primary key
----------------------------------+------------------------------------

Reporter: Csirmaz Bendegúz | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:

Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+------------------------------------

Changes (by Sarah Boyce):

* stage: Unreviewed => Accepted

Comment:

Great catch thank you!

> I believe step 5 should never happen, since step 3 should fail with a
unique check error.

Agreed 👍 if you were to add another field to the model then you slightly
override `Dummy (1)` with whatever is in `Dummy (2)` - which is not nice.
--
Ticket URL: <https://code.djangoproject.com/ticket/35551#comment:2>

[Django]

#35551: Unique checks skipped when changing primary key

-------------------------------------+-------------------------------------
Reporter: Csirmaz Bendegúz | Owner: Csirmaz
| Bendegúz
Type: Bug | Status: assigned

Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Csirmaz Bendegúz):

* owner: nobody => Csirmaz Bendegúz
* status: new => assigned

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

[Django]

#35551: Unique checks skipped when changing primary key
-------------------------------------+-------------------------------------
Reporter: Csirmaz Bendegúz | Owner: Csirmaz
| Bendegúz
Type: Bug | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Csirmaz Bendegúz):

So actually there are 2 issues:
1. The unique check is skipped when PK is changed.
2. The object is duplicated when PK is changed.
I'm looking into this.
--
Ticket URL: <https://code.djangoproject.com/ticket/35551#comment:4>

[Django]

#35551: Unique checks skipped when changing primary key
-------------------------------------+-------------------------------------
Reporter: Csirmaz Bendegúz | Owner: Csirmaz
| Bendegúz
Type: Bug | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Description changed by Csirmaz Bendegúz:

Old description:

> I noticed the unique checks are skipped ({{{_perform_unique_checks}}})
> when an object's primary key is changed.
>
> Given:
>
> {{{
> # models.py
> class Dummy(models.Model):
> id = models.IntegerField(primary_key=True)
>
> # admin.py
> admin.site.register(Dummy)
> }}}
>
> 1. Create Dummy (1), Dummy (2).
> 2. Change Dummy (2)'s Id field -> 1
> 3. No error, unique checks skipped
> 4. Change Dummy (1)'s Id field -> 3
> 5. A new Dummy (1) is created (why?)
>

> I believe step 5 should never happen, since step 3 should fail with a
> unique check error.

New description:

I noticed the unique checks are skipped ({{{_perform_unique_checks}}})
when an object's primary key is changed.

Given:

{{{
# models.py
class Dummy(models.Model):
id = models.IntegerField(primary_key=True)

# admin.py
admin.site.register(Dummy)
}}}

1. Create Dummy (1), Dummy (2).
2. Change Dummy (2)'s Id field -> 1
3. No error, unique checks skipped
4. Change Dummy (1)'s Id field -> 3

5. A new Dummy (1) is created (1 and 3 are duplicates)

I believe step 5 should never happen, since step 3 should fail with a
unique check error.

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

[Django]

> 5. A new Dummy (1) is created (1 and 3 are duplicates)
>
> I believe step 5 should never happen, since step 3 should fail with a
> unique check error.

New description:

I noticed the unique checks are skipped ({{{_perform_unique_checks}}})
when an object's primary key is changed.

Given:

{{{
# models.py
class Dummy(models.Model):
id = models.IntegerField(primary_key=True)

# admin.py
admin.site.register(Dummy)
}}}

1. Create Dummy (1), Dummy (2).
2. Change Dummy (2)'s Id field -> 1
3. No error, unique checks skipped
4. Change Dummy (1)'s Id field -> 3

5. A new Dummy (3) is created, Dummy (1) stays unchanged

I believe step 5 should never happen, since step 3 should fail with a
unique check error.

--
--

Ticket URL: <https://code.djangoproject.com/ticket/35551#comment:6>

[Django]

#35551: Unique checks skipped when changing primary key
-------------------------------------+-------------------------------------
Reporter: Csirmaz Bendegúz | Owner: Csirmaz
| Bendegúz
Type: Bug | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Csirmaz Bendegúz):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/35551#comment:7>

[Django]

#35551: Unique checks skipped when changing primary key
-------------------------------------+-------------------------------------
Reporter: Csirmaz Bendegúz | Owner: Csirmaz
| Bendegúz
Type: Bug | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Csirmaz Bendegúz):

I have submitted a basic fix for raising a ValidationError on PK conflict.

The other issue I would like to address is the fact that Django creates a
duplicate if there's no PK conflict.
This should also resolve the race condition issue, i.e. what if an object
is created by a separate process with the same PK **after** this check
passes. An IntegrityError should be raised in that case.
--
Ticket URL: <https://code.djangoproject.com/ticket/35551#comment:8>

[Django]

#35551: Unique checks skipped when changing primary key
-------------------------------------+-------------------------------------
Reporter: Csirmaz Bendegúz | Owner: Csirmaz
| Bendegúz
Type: Bug | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Csirmaz Bendegúz):

I submitted another PR for the above under this ticket, please review
both! Thanks!
--
Ticket URL: <https://code.djangoproject.com/ticket/35551#comment:9>