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

Added support to PostgreSQL identity column #277

Closed
wants to merge 17 commits into from

Conversation

diegosardina
Copy link
Contributor

  • bug fix
  • BC break? no

(Sorry for the new PR but I deleted the old branch and repository)

Since PostgreSQL 10, identity columns are recommended instead of serial types as stated in the documentation (and serial types are even discouraged here).

In the old PR a.attidentity = ANY (ARRAY['a', 'd']) and pg_get_serial_sequence() were used. While the first only caused problem with older PostgreSQL, the latter has (known) issues with multiple schemas. The best way was to directly query pg_catalog (that is not something trival).

Tests are successful (here) both with serial types and identity columns. Does it make sense to keep both or we will update the current ones to use just the new identity column?

@dg dg force-pushed the master branch 4 times, most recently from f8e1e59 to 4862e4d Compare May 4, 2021 19:12
@milo milo self-assigned this May 19, 2021
@milo
Copy link
Member

milo commented May 19, 2021

I would stay with SERIAL support, PostgreSQL 9.6 has no identities and is still supported (well, to November 2021).

There is one bug in this PR, missing parenthesis around new OR operands. In that case, it marks autoincrement column event if not primary key. Sorry, there is no test for it.

Please, fix your commit (or I can do that, I'll leave you as author but signature will be missing). And I have prepeared test too.

@milo
Copy link
Member

milo commented May 19, 2021

One day, query may be siplified to use pg_attribute.attidentity column since PostgreSQL 10.

@diegosardina
Copy link
Contributor Author

One day, query may be siplified to use pg_attribute.attidentity column since PostgreSQL 10.

Yup, also the latter line may be simplified by using pg_get_serial_sequence() when they fix problems with multiple schemas.

Thanks for the review :-)

@milo
Copy link
Member

milo commented May 19, 2021

Thank you. I squashed the commits and merged them as c498d8c

milo pushed a commit that referenced this pull request May 19, 2021
milo pushed a commit that referenced this pull request May 19, 2021
@milo milo closed this May 19, 2021
dg pushed a commit that referenced this pull request May 31, 2021
dg pushed a commit that referenced this pull request May 31, 2021
dg pushed a commit that referenced this pull request May 31, 2021
dg pushed a commit that referenced this pull request May 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants