Autoincrement via identity columns on Oracle

[morozov]

See #4744, #5396.

[morozov]

@greg0ire @derrabus if we decide to move forward with this, we need to consider changes in the IdentityGenerator API in the ORM since Oracle does support identity columns but its drivers don't support lastInsertId().

Oracle allows retrieval of the identity column value as follows:

$stmt = $connection->prepare('INSERT INTO AUTO_INCREMENT_TABLE VALUES(DEFAULT) RETURNING ID INTO :ID');
$stmt->bindParam('id', $id);
$stmt->executeStatement();
echo $id, PHP_EOL;
// 1

Despite it not being widespread, personally, I believe it's the most reasonable way of dealing with autogenerated values in SQL.

The current IdentityGenerator API design is quite unfortunate: it exposes the implementation details like sequence name and limits itself to being used before or after the insert (which is also an implementation detail). This limitation doesn't allow to implement the above approach. Instead, this API should do the insert. E.g.:

interface SomeNewName
{
    public function insert(EntityManagerInterface $em, $entity);
}

In addition to having autoincrement columns implemented in a more similar way across platforms, this may allow dropping the support of sequences at all, since they are used only as an implementation of autoincrement columns.

What do you think? Would you be interested in making the necessary API changes in the ORM? Otherwise, I don't think we can proceed with this change.

[derrabus]

The new API you're suggesting makes perfect sense to me. However, I wonder if a piece of code that uses the DBAL for performing inserts should be aware of how to retrieve the generated ID for the current DBMS. Would it make sense to create an API for that in DBAL, something like a low-level version of Connection::insert() that returns the generated ID or writes it into an output variable?

[morozov]

I wonder if a piece of code that uses the DBAL for performing inserts should be aware of how to retrieve the generated ID for the current DBMS.

I tried but couldn't really come up with an abstract DBAL-level API for that. I propose that we implement a working code prototype first and then decide which library will own the new API. Personally, I believe that it should belong to the ORM because when it comes to building SQL, the DBAL isn't aware of any entity metadata while the ORM is.

[greg0ire]

Applying "tell, don't ask" here too sounds great on paper 👍

In practice, I've looked at the ORM code, and I've found 4 places where id generators is used:

• https://github.com/doctrine/orm/blob/09ff36cda044d5ecec355c7127625d9fd4bfc33c/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php#L280-L287
• https://github.com/doctrine/orm/blob/09ff36cda044d5ecec355c7127625d9fd4bfc33c/lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php#L161-L168
• https://github.com/doctrine/orm/blob/09ff36cda044d5ecec355c7127625d9fd4bfc33c/lib/Doctrine/ORM/UnitOfWork.php#L989-L1003
• https://github.com/doctrine/orm/blob/09ff36cda044d5ecec355c7127625d9fd4bfc33c/lib/Doctrine/ORM/UnitOfWork.php#L1632-L1645

The 3 first one seem related to insertion, although they are not all located in the same place. The last one is about getting the entity state, and I'm not sure towards what we should migrate it, but maybe you do?

[morozov]

The last one is about getting the entity state, and I'm not sure towards what we should migrate it, but maybe you do?

I'm not familiar with the ORM at all, but the last case looks like a hack to me — determining the state of the entity based on whether or not it has an identifier. Why cannot this state be explicitly updated after a successful attempt to persist the entity?

We can agree that the responsibilities of the new method will be (in no particular order):

1. Do the insert.
2. Generate the identifier.
3. Set it on the entity.

If this operation succeeds, then something else can update the state of the entity to something appropriate.

[morozov]

Just to clarify, I'm not going to make the API changes in the ORM myself. Somebody else will have to do that. Do you @greg0ire or @derrabus want to give it a try or maybe someone from the community does?

[greg0ire]

I don't feel up to that task, sorry 😞

[morozov]

Closing it for now.