Identity columns

[greg0ire]

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.

[greg0ire]

Not sure why I originally made the title so shouty, this isn't SQL

[morozov]

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:

dbal/src/Platforms/AbstractPlatform.php

Line 3056 in e974d4d

public function supportsIdentityColumns()

It's implemented natively by the SQL Server and IBM DB2 platforms:

dbal/src/Platforms/SQLServer2012Platform.php

Lines 1305 to 1308 in e974d4d

	protected function _getCommonIntegerTypeDeclarationSQL(array $column)
	{
	return ! empty($column['autoincrement']) ? ' IDENTITY' : '';
	}

dbal/src/Platforms/DB2Platform.php

Lines 176 to 180 in e974d4d

	protected function _getCommonIntegerTypeDeclarationSQL(array $column)
	{
	$autoinc = '';
	if (! empty($column['autoincrement'])) {
	$autoinc = ' GENERATED BY DEFAULT AS IDENTITY';

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?

[greg0ire]

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?

[morozov]

as opposed to PG-specific SERIAL notation

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 #2695:

Based on the requirement for the CREATE SEQUENCE privilege, 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.

[greg0ire]

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 🤔

[morozov]

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.

[bendavies]

@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.

[morozov]

@bendavies if you want to give it a try, go for it.

[bendavies]

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.

[morozov]

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.

[bendavies]

Apologies for my irritability. Rough day