Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#34131 closed Bug (invalid)

Postgres AutoField change from serial to identity

Reported by: Marco Glauser Owned by: nobody
Component: Database layer (models, ORM) Version: 4.1
Severity: Normal Keywords:
Cc: Florian Apolloner Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

With django 4.1 the following change was added (#30511):

On PostgreSQL, AutoField, BigAutoField, and SmallAutoField are now created as identity columns rather than serial columns with sequences.

This causes issues for us because our migrations depend on sequences being created by the BigAutoField.

We use Luhn's algorithm to generate ids that are harder to mistype by accident. The easiest way for us to do that is to change the default of the id column.

ALTER TABLE invoice_invoice ALTER COLUMN id SET DEFAULT luhn_generate(nextval('invoice_invoice_id_seq'));

This no longer works with 4.1 and we get the following error when trying to apply the migration (for example when running tests)

django.db.utils.ProgrammingError: column "id" of relation "invoice_invoice" is an identity column

Already migrated databases continue to work as expected.
This is currently blocking our upgrade path to 4.1 since we cannot run our unit tests.

Change History (6)

comment:1 by Simon Charette, 2 years ago

I'm afraid that without contrib.postgres support for serial fields (#27452) or database level defaults (#470) this will be hard to solve.

The only way I can see around that is in the mean type is to drop the identity on the column and entirely and re-create it as a serial with something along the lines of

ALTER TABLE invoice_invoice ALTER COLUMN id DROP IDENTITY;
CREATE SEQUENCE invoice_invoice_id_seq OWNED BY invoice_invoice.id;
ALTER TABLE invoice_invoice ALTER COLUMN id SET DEFAULT luhn_generate(nextval('invoice_invoice_id_seq'));

I wonder if this should be considered a release blocker since it could be considered a bug in feature introduced in 4.1. It's a convoluted use case that does have a workaround though so I'm not entirely convinced.

comment:2 by Florian Apolloner, 2 years ago

Cc: Florian Apolloner added

comment:3 by Carlton Gibson, 2 years ago

I'm not sure I'd want to say this is supported 🤔

This a requires a RunSQL yes…?

ALTER TABLE invoice_invoice ALTER COLUMN id SET DEFAULT luhn_generate(nextval('invoice_invoice_id_seq'));

So I'm not sure that we want to say that's covered by the stability of the model field API — it's not like you're using the default kwarg here. 🤔

Rather, I'd want to say that you should be using a custom field for models that need this functionality, and custom migrations to create that in the DB. (Or similar)

comment:4 by Mariusz Felisiak, 2 years ago

Resolution: invalid
Status: newclosed

Agreed with previous comments. Changing defaults for AutoField on PostgreSQL has never been officially supported. Using a custom serial field is probably the best way to move forward for you.

comment:5 by Marco Glauser, 2 years ago

We can confirm that the workaround from Simon is working as expected and passing all our tests we previously wrote for this specific migration. Thank you!

While I understand that not every case is supported, the documentation, including Django 4.0 explicitly specifies that it uses a SERIAL field for the AutoField on Postgres. (and still is mentioning a sequence in 4.1)
This feels like a big change for a minor feature update buried deep in the changelog, without any alternative proposed since there is no "official" SerialField for Django. And to avoid issues, it keeps existing databases as SERIAL fields. So for us the deeper issue is that while our production database would happily run with 4.1 because it would still be using SERIAL under the hood, our unit tests will fail because the new databases use identity columns.
We originally chose to misuse the AutoField because Django doesn't offer any SERIAL field and it's easier to patch the default value than to create the sequences and everything that goes with it ourselves.

comment:6 by Simon Charette, 2 years ago

Glad to hear the workaround worked!

We originally chose to misuse the AutoField because Django doesn't offer any SERIAL field and it's easier to patch the default value than to create the sequences and everything that goes with it ourselves.

I think this the crux of the issue. Given #27452 already tracks the lack of proper support for serial fields, which is something you would have used if it was available, I'm also convinced that this issue should be closed as it's not actionable.

Note: See TracTickets for help on using tickets.
Back to Top