search

nette / database

💾 A database layer with a familiar PDO-like API but much more powerful. Building queries, advanced joins, drivers for MySQL, PostgreSQL, SQLite, MS SQL Server and Oracle.
https://doc.nette.org/database
Other
492 stars 105 forks source link

Added support to PostgreSQL identity column #277

Closed diegosardina closed 3 years ago

diegosardina commented 3 years ago

(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?

milo commented 3 years ago

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 commented 3 years ago

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

diegosardina commented 3 years ago

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 commented 3 years ago

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