[Django] #30266: Migrating a model's default primary key to a BigAutoField causes Postgres sequence to lose owner
Django
sequence to lose owner
-------------------------------------+-------------------------------------
Reporter: Dolan | Owner: nobody
Antenucci |
Type: Bug | Status: new
Component: Database | Version: 1.11
layer (models, ORM) |
Severity: Normal | Keywords: postgres migration
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-------------------------------------+-------------------------------------
Note: I'm marking this as easy pickings, but feel free to change.
== Summary of Issue ==
Start with the following models (below assumes application name is
"sandbox"):
{{{#!python
class Test1(models.Model):
id = models.BigAutoField(primary_key=True)
name = models.CharField(max_length=100)
class Test2(models.Model):
name = models.CharField(max_length=100)
}}}
After migrating, go the the `dbshell` and run `\d sandbox_test1_id_seq`
and `\d sandbox_test2_id_seq`. The results will include "Owned by:
public.sandbox_test1.id" and "Owned by: public.sandbox_test2.id"
respectively.
Next, change Test2 to a `BigIntField`:
{{{#!python
class Test2(models.Model):
id = models.BigAutoField(primary_key=True)
name = models.CharField(max_length=100)
}}}
Make a new migration, migrate, and then go back to `dbshell` and run `\d
sandbox_test2_id_seq`. There will no longer be an owner listed for this
sequence.
== Result of this issue ==
When using `loaddata` with fixtures on the `Test2` model, the sequence
will not be incremented because the command Django uses to reset the
sequence for the primary key fails. Specifically, in the
[https://github.com/django/django/blob/master/django/db/backends/postgresql/operations.py#L157
postgres backend], Django calls the postgres function
`pg_get_serial_sequence(<table>, <column>)`, which returns nothing when
the sequence is missing an owner.
This can be verified in postgres shell via `select
pg_get_serial_sequence('sandbox_test1', 'id');` and `select
pg_get_serial_sequence('sandbox_test2', 'id');`
The result is that after the fixture is loaded, any other inserts will
fail because their primary keys will conflict with those in the fixture.
== Potential fixes ==
1. It seems like `makemigrations` should be setting the sequence owner, or
using a different command to migrate so that the sequence owner
information is not lost. For example, the
[https://www.postgresql.org/docs/10/datatype-numeric.html#DATATYPE-SERIAL
postgres docs on serial columns] show that this can be done with `ALTER
SEQUENCE tablename_colname_seq OWNED BY tablename.colname;`
2. For tables already migrated and missing the owner information, perhaps
the `makemigrations` command needs to confirm that the owner information
is set correctly.
3. Not a fan of this solution myself, but one could also change the
sequence is reset the
[https://github.com/django/django/blob/master/django/db/backends/postgresql/operations.py#L157
postgres backend] (i.e., not use `pg_get_serial_sequence`)
Long-term, maybe `makemigrations` should be using postgres's SERIAL and
BIGSERIAL field types, which automatically create the sequence and set the
sequence owner.
--
Ticket URL: <https://code.djangoproject.com/ticket/30266>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
sequence to lose owner
-------------------------------------+-------------------------------------
| Antenucci
Type: Bug | Status: assigned
Component: Database layer | Version: 1.11
(models, ORM) |
Severity: Normal | Resolution:
Keywords: postgres migration | Triage Stage:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Dolan Antenucci
* status: new => assigned
Comment:
I have a fix, but may need help with writing my test case.. will reach out
to mailing list if I can't figure out
--
Ticket URL: <https://code.djangoproject.com/ticket/30266#comment:1>
Django
sequence to lose owner
-------------------------------------+-------------------------------------
| Antenucci
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: postgres migration | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
* component: Database layer (models, ORM) => Migrations
* stage: Unreviewed => Accepted
Comment:
[https://github.com/django/django/pull/11098 PR]
Please uncheck "Patch needs improvement" if you're able to rewrite the
test as suggested on the PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/30266#comment:2>
Django
sequence to lose owner
-------------------------------------+-------------------------------------
| Antenucci
Type: Bug | Status: assigned
Component: Migrations | Version: 1.11
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):
* easy: 1 => 0
Old description:
New description:
--
--
Ticket URL: <https://code.djangoproject.com/ticket/30266#comment:3>
Django
sequence to lose owner
-------------------------------------+-------------------------------------
| Antenucci
Type: Bug | Status: assigned
Component: Migrations | Version: 1.11
Severity: Normal | Resolution:
Keywords: postgres migration | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Dolan Antenucci):
FYI: I'm hoping to get to the pending changes in the coming week. Will
update once they're done
--
Ticket URL: <https://code.djangoproject.com/ticket/30266#comment:4>
Django
sequence to lose owner
-------------------------------------+-------------------------------------
| Antenucci
Type: Bug | Status: assigned
Component: Migrations | Version: 1.11
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
Comment:
Updated PR as requested; ready for review
--
Ticket URL: <https://code.djangoproject.com/ticket/30266#comment:5>
Django
sequence to lose owner
-------------------------------------+-------------------------------------
| Antenucci
Type: Bug | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* version: 1.11 => master
* component: Migrations => Database layer (models, ORM)
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/30266#comment:6>
Django
sequence to lose owner
-------------------------------------+-------------------------------------
| Antenucci
Component: Database layer | Version: master
(models, ORM) |
Keywords: postgres migration | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"f944cb3d3bc17a97216f8990ff3bb4bee14b6f6b" f944cb3d]:
{{{
#!CommitTicketReference repository=""
revision="f944cb3d3bc17a97216f8990ff3bb4bee14b6f6b"
Fixed #30266 -- Kept a sequence owner when altering an
AutoField/BigAutoField on PostgreSQL.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30266#comment:7>