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.

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

feat/compat: support postgres 10 identity columns #244

Closed
rmg opened this issue Jul 3, 2018 · 3 comments
Closed

feat/compat: support postgres 10 identity columns #244

rmg opened this issue Jul 3, 2018 · 3 comments
Milestone

Comments

@rmg
Copy link

rmg commented Jul 3, 2018

The Postgraphile doesn't support the new identity columns (links below) feature in postgres 10 as a drop-in replacement for the traditional serial/sequence primary key.

These 2 tables both have the same general semantics but their internals are different in ways that affect postgraphile:

CREATE TABLE test_old (
    id serial PRIMARY KEY,
    payload text
);
INSERT INTO test_old (payload) VALUES ('a'), ('b'), ('c') RETURNING *;

CREATE TABLE test_new (
    id int GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY,
    payload text
);
INSERT INTO test_new (payload) VALUES ('a'), ('b'), ('c') RETURNING *;

Problem: no visible default value

According to the pg_catalog.pg_attribute table, test_old.id column has a default value and test_new.id does not. Instead, it has a d value in the new attidentity column in that table. Since it is an identity column it has a default value by definition.

Problem: stronger than default

To take things even further, if attidentity was a (GENERATED ALWAYS instead of GENERATED BY DEFAULT) then postgres would actually ignore the values you give that column in your INSERT statements unless you included the OVERRIDING SYSTEM VALUE option. If a column is defined this way then postgraphile will probably need special handling in either the INSERT statement it generates or on the schema side to indicate that the column is read-only.

Descriptions of the identity columns feature added in 10.0:

@benjie
Copy link
Member

benjie commented Jul 4, 2018

See also: graphile/crystal#631

This is definitely an issue. We don’t have any PG10-specific support at the moment. I’m currently planning on turning the introspection query into a template and pre-processing it once we know which database version we’re connected to. Would love a PR for this (with tests!) or for someone to sponsor the feature.

@rmg
Copy link
Author

rmg commented Jul 4, 2018

Wow, I can't believe I didn't see that! I spent a good couple hours digging through the code investigating why I was seeing what I was seeing; I probably did all my searching before researching enough to figure out what to search the issues for 😵

@benjie
Copy link
Member

benjie commented Jul 4, 2018

On the plus side, you're in a perfect position to contribute at least the beginnings of a fix now 😜 Or maybe even just some failing tests!

@benjie benjie added this to the 4.1 milestone Aug 16, 2018
benjie pushed a commit that referenced this issue Sep 8, 2018
Fixes #244 

Uses a template string to support multiple PostgreSQL versions.
madtibo pushed a commit to spacefill/graphile-engine that referenced this issue Dec 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants