Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added support for PostgreSQL identity column #5502

Merged
merged 1 commit into from
May 5, 2021
Merged

Added support for PostgreSQL identity column #5502

merged 1 commit into from
May 5, 2021

Conversation

Oreilles
Copy link
Contributor

@Oreilles Oreilles commented May 5, 2021 •
edited
Loading

PostgreSQL identity columns were introduced in version 10. They aim to replace the commonly used SERIAL type, as it not compliant with the SQL standard - and is error prone, as you have to manage the sequence associated to. An identity column is basically a generated integer type with a sequence built-in it knows everything about (it won't pick nextval if nextval already exists).

Solves #5501.
Potentially breaking on PostgreSQL < 10, but 9 is EOL anyway.

@benhaynes benhaynes requested a review from rijkvanzanten May 5, 2021 21:21
@rijkvanzanten rijkvanzanten added this to the v9.0.0-rc.63 milestone May 5, 2021
@rijkvanzanten
Copy link
Member

Thanks @Oreilles! We might want to send a PR into knex/knex in order to make identity the default instead of serial based IDs 🙂 Directus just uses .increments() to create the PK columns

@rijkvanzanten rijkvanzanten merged commit da1c3ed into directus:main May 5, 2021
@rijkvanzanten rijkvanzanten mentioned this pull request May 5, 2021
Closed
@Oreilles
Copy link
Contributor Author

Oreilles commented May 5, 2021

Thanks @Oreilles! We might want to send a PR into knex/knex in order to make identity the default instead of serial based IDs 🙂 Directus just uses .increments() to create the PK columns

That would be great! I'm not sure though if we would need changes in the Directus code to handle that, and what would be the implications be if people have mixed in serials and identity ?

@rijkvanzanten
Copy link
Member

rijkvanzanten commented May 5, 2021 •
edited
Loading

I'm not sure though if we would need changes in the Directus code to handle that

Don't think so! Sounds like this is an abstraction in lower-level Knex, not something Directus should be concerned about.

and what would be the implications be if people have mixed in serials and identity ?

Seeing that your PR now fixes the differences between the two, there shouldn't be any implications for Directus users 🤔

@Oreilles
Copy link
Contributor Author

Oreilles commented May 6, 2021

Sounds like this is an abstraction in lower-level Knex, not something Directus should be concerned about.

OK this answered the two questions I had! Will have a look at how knex handles this and see if making identity the default is feasible when I have some spare time.

@Oreilles Oreilles deleted the postgres-identity-column branch May 21, 2021 12:59
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rijkvanzanten rijkvanzanten rijkvanzanten approved these changes

Assignees

@Oreilles Oreilles

Labels
Bug
Projects
None yet
Milestone
v9.0.0-rc.63
Development

Successfully merging this pull request may close these issues.
2 participants
@Oreilles @rijkvanzanten