Opened 8 years ago

Closed 4 months ago

#27452 closed New feature (wontfix)

Add Postgres serial field to contrib.postgres

Reported by: Johannes Maron Owned by: Csirmaz Bendegúz
Component: contrib.postgres Version: dev
Severity: Normal Keywords:
Cc: info@…, InvalidInterrupt, Florian Apolloner, Csirmaz Bendegúz Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Burhan Khalid)

Since we have the beautiful contrib.postgres package now, we can add a couple more Postgres specific fields, that are not supported by MySQL but handy to all the Postgres fan boys.

That being said, I guess it's a good idea to start with Serial.

The PostgreSQL documentation: https://www.postgresql.org/docs/9.1/static/datatype-numeric.html describes the field as follows:

The data types serial and bigserial are not true types, but merely a notational convenience for creating unique identifier columns (similar to the AUTO_INCREMENT property supported by some other databases).

Use Cases:

  1. Anywhere an automatic incrementing value is required, but a primary key is often (mis)used.
  2. Providing additional (user controlled) auto-incrementing values.

The primary benefit is that is isolates the primary key for the sole use of maintaining referential integrity.

Change History (35)

comment:1 by Burhan Khalid, 8 years ago

Description: modified (diff)

comment:2 by Johannes Maron, 8 years ago

I opened a PR with an implementation that I am already using somewhere. It added a couple of tests and documentation too.
I guess this is a good starting point for a discussion.

Last edited 8 years ago by Tim Graham (previous) (diff)

comment:3 by Johannes Maron, 8 years ago

Owner: set to Johannes Maron
Status: newassigned

comment:4 by Tim Graham, 8 years ago

Has patch: set
Patch needs improvement: set
Triage Stage: UnreviewedAccepted

comment:5 by Johannes Maron, 6 years ago

Patch needs improvement: unset

comment:6 by Simon Charette, 6 years ago

Patch needs improvement: set

comment:7 by Johannes Maron, 5 years ago

Patch needs improvement: unset

comment:8 by Nick Pope, 5 years ago

Patch needs improvement: set

comment:9 by Nick Pope, 5 years ago

Version: 1.10master

comment:10 by Johannes Maron, 5 years ago

Patch needs improvement: unset

comment:11 by Mariusz Felisiak, 5 years ago

Needs tests: set
Patch needs improvement: set

comment:12 by InvalidInterrupt, 5 years ago

Cc: InvalidInterrupt added

comment:13 by Johannes Maron, 5 years ago

I closed my PR for now, due to a lack of interest. Since Django does support returning values since v3.0 everyone who needs such a field can build it. Yet, I don't have the time get the feature into Django for now.

in reply to:  13 comment:14 by InvalidInterrupt, 5 years ago

Replying to Johannes Hoppe:

I closed my PR for now, due to a lack of interest. Since Django does support returning values since v3.0 everyone who needs such a field can build it. Yet, I don't have the time get the feature into Django for now.

Do you mind if I continue working on this using your branch? Handling sequence resets after loading fixtures will require changes to the DB backend.

comment:15 by Claude Paroz, 5 years ago

Owner: Johannes Maron removed
Status: assignednew

Latest comment from Johannes is clearly an invitation to help :-)

comment:16 by InvalidInterrupt, 5 years ago

Owner: set to InvalidInterrupt
Status: newassigned

comment:17 by InvalidInterrupt, 4 years ago

Needs tests: unset
Patch needs improvement: unset

comment:18 by Mariusz Felisiak, 4 years ago

Needs tests: set
Patch needs improvement: set

comment:19 by InvalidInterrupt, 4 years ago

Needs tests: unset
Patch needs improvement: unset

comment:20 by Johannes Maron, 4 years ago

Hi, the new default patch depends on https://github.com/django/django/pull/11783. Maybe you would care to review this one, to get things moving along faster? Best Joe

comment:21 by Johannes Maron, 4 years ago

Needs documentation: set
Needs tests: set

comment:22 by Claude Paroz, 2 years ago

Does this still make sense now that Django is using identity columns for *AutoFields?

comment:23 by Simon Charette, 2 years ago

Claude, yes I think that the move to identity columns on Postgres made this feature even more desirable (e.g. #34131).

comment:24 by Simon Charette, 9 months ago

FWIW someone was able to implement an IdentityField which is kind of the equivalent of GeneratedField if it supported AS IDENTITY. It's not the equivalent of SerialField though but it demonstrates that all the pieces are there to implement it based on db_returning and pre_save returning a Default expression.

comment:25 by Csirmaz Bendegúz, 7 months ago

Needs documentation: unset
Needs tests: unset
Owner: changed from InvalidInterrupt to Csirmaz Bendegúz

I'm going to push this forward because we need an alternative to #8576. Please review my PR.

Last edited 7 months ago by Csirmaz Bendegúz (previous) (diff)

comment:26 by Sarah Boyce, 6 months ago

Needs tests: set
Patch needs improvement: set

comment:27 by Sarah Boyce, 6 months ago

Needs tests: unset
Patch needs improvement: unset

comment:28 by Florian Apolloner, 6 months ago

Hi folks, I am wondering if we really want & need something like this in Django. What are the arguments that this has to be in Django as opposed to a third party library. The issue exists since 8 years with little to no comments so I would argue that using multiple serial fields on a model or having explicit access to a serial field is not a feature that is needed often. So even if this is in contrib I kinda wonder if we have to support every postgresql feature out there.

Version 0, edited 6 months ago by Florian Apolloner (next)

comment:29 by Florian Apolloner, 6 months ago

Cc: Florian Apolloner added

in reply to:  28 comment:30 by Csirmaz Bendegúz, 5 months ago

Replying to Florian Apolloner:

Hi folks, I am wondering if we really want & need something like this in Django. What are the arguments that this has to be in Django as opposed to a third party library. The issue exists since 8 years with little to no comments so I would argue that using multiple serial fields on a model or having explicit access to a serial field is not a feature that is needed often. So even if this is suggested for contrib I kinda wonder if we have to support every postgresql feature out there.

My goal with this one is to have a field that can be used as a surrogate key in composite primary keys (#373), when/if that feature lands.

If we can't have AutoFields in composite primary keys (#8576), then it would be nice to have SerialFields at least.

comment:31 by Csirmaz Bendegúz, 5 months ago

Cc: Csirmaz Bendegúz added

comment:32 by Florian Apolloner, 5 months ago

To be honest I'd be slightly in favor of removing the existing limitations of AutoFields where databases can support it (if we are able to decide that it makes sense semantically at least). Adding a new field that basically does the same thing as an existing field kinda feels wrong. Granted, an explicit Serial/IdentityField would allow to specifiy more options (like adjusting the sequence name), but I am not sure if that would be a common enough usecase to add it to django itself.

in reply to:  32 comment:33 by Csirmaz Bendegúz, 5 months ago

Replying to Florian Apolloner:

To be honest I'd be slightly in favor of removing the existing limitations of AutoFields where databases can support it (if we are able to decide that it makes sense semantically at least). Adding a new field that basically does the same thing as an existing field kinda feels wrong. Granted, an explicit Serial/IdentityField would allow to specifiy more options (like adjusting the sequence name), but I am not sure if that would be a common enough usecase to add it to django itself.

I was thinking the same thing initially, but this ticket was already Accepted, while #8576 received a lot of push back and was closed as wontfix.

IMO, another legit use-case is an "order column" - an auto-generated sequence that can be changed by the users. Semantically, it makes more sense to use SERIAL over IDENTITY for this.

comment:34 by Csirmaz Bendegúz, 5 months ago

comment:35 by Sarah Boyce, 4 months ago

Resolution: wontfix
Status: assignedclosed

Based off the forum discussion, there doesn't appear to be a strong consensus that this should be added to Django core. Hence, closing as wontfix

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