dbal
dbal copied to clipboard
doctrine
Identity columns
Feature Request
| Q | A |
|---|---|
| New Feature | yes |
| RFC | yes |
| BC Break | no |
Summary
As of now, it seems that identity columns are not supported by the DBAL. This feature is available since Oracle 12.1 and PostgreSQL 10.
Supposed pros:
- better performance over sequence+triggers for Oracle
- they allow to omit the id field when inserting
-
unlike
SERIAL, they are standard - they allow configuring whether a user-supplied id is allowed or not.
As of now, it seems that identity columns are not supported by the DBAL.
This looks a bit misleading given that there is an API for that: https://github.com/doctrine/dbal/blob/e974d4deca8c430d9eba0975f441538145a5a8f1/src/Platforms/AbstractPlatform.php#L3056
It's implemented natively by the SQL Server and IBM DB2 platforms: https://github.com/doctrine/dbal/blob/e974d4deca8c430d9eba0975f441538145a5a8f1/src/Platforms/SQLServer2012Platform.php#L1305-L1308 https://github.com/doctrine/dbal/blob/e974d4deca8c430d9eba0975f441538145a5a8f1/src/Platforms/DB2Platform.php#L176-L180
Most of the platforms implement it in their proprietary terms (e.g. MySQL, SQLite) and only PostgreSQL and Oracle currently emulate it via sequences.
I believe, the issue is about implementing native support where it's missing, right?
Yes, you're right, what's missing is native support for the SQL standard (as opposed to PG-specific SERIAL notation). It looks like there is already some support for that in DB2Platform.php, maybe part of it could be moved up in AbstractPlatform.php, since that syntax is supposed to be standard?
as opposed to PG-specific
SERIALnotation
I believe it's not problem #1 from the DBAL perspective: as long as it behaves as the identity column, it shouldn't be a problem from the API standpoint. For instance, MySQL and SQLite will keep using their own syntax anyways.
What's more important is that as long as not all platforms support identity columns, the API consumers have to use two different APIs to achieve effectively the same goal (identity generation) on different platforms. That's the problem I'd like to solve by eventually deprecating supportsIdentityColumns() and have all platforms support them.
Additionally, the identity columns will need the support for lastInsertId() at the drivel level for Oracle or some workaround in the DBAL. I bet neither oci8 nor especially pdo_oci support it.
From the article linked to https://github.com/doctrine/dbal/issues/2695:
Based on the requirement for the
CREATE SEQUENCEprivilege, it is not difficult to deduce that a sequence is being used to populate the identity column.
This is effectively how identity columns are currently implemented in the PostgreSQL platform.
Switching to the standard SQL syntax wherever possible would be the cherry on the cake. Note, that it's a breaking change.
It looks like there is already some support for that in DB2Platform.php, maybe part of it could be moved up in AbstractPlatform.php, since that syntax is supposed to be standard?
Sounds good. This also could be addressed closer to the end.
Note, that it's a breaking change.
Because it will make the diff between a database and its in-memory equivalent not empty, and will suddenly generate a lot of migrations when using doctrine/migrations, right? I wonder how painful a migration from SERIAL to standard identity columns would be, and if the DBAL would generate the correct diff for those :thinking:
Because it will make the diff [...], right?
Yes. Handling this diff automatically in the DBAL may be non-trivial: not only we'd have to properly migrate the table, trigger definitions, etc. we'd have to migrate their state (i.e. next sequence value). It might be easier to document a manual migration path, although even this will take quite some time.
@morozov it doesn't look too bad.
looking at: https://www.postgresql.org/docs/12/sql-altertable.html and https://www.postgresqltutorial.com/postgresql-identity-column/
As long as you can determine the current sequence value (easy), you can then:
ALTER TABLE table_name
ALTER COLUMN column_name
{ SET GENERATED { ALWAYS | BY DEFAULT } |
SET sequence_option | RESTART [ [ WITH ] restart ] }
where restart is the current sequence value.
Apologies. I didn't realise comments weren't welcome unless accompanied by a PR. Let me know if you'd like me to delete my comment.
Apologies. I didn't realise comments weren't welcome unless accompanied by a PR. Let me know if you'd like me to delete my comment.
Not at all. Thank you for the details.