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

Current sequence value should not be used for tracking last inserted ID #4687

Closed
morozov opened this issue Jun 23, 2021 · 1 comment · Fixed by #4691
Closed

Current sequence value should not be used for tracking last inserted ID #4687

morozov opened this issue Jun 23, 2021 · 1 comment · Fixed by #4691

Comments

@morozov
Copy link
Member

morozov commented Jun 23, 2021

Bug Report

Q A
BC Break no
Version all

Summary

DBAL replicates the PDO function PDO::lastInsertId(string $name) which will return the current value of the sequence $name. As the function name implies, this should allow doing an INSERT and then getting the value of an auto-increment column that is internally implemented via a sequence (e.g. on Oracle). The value is primarily meant to be used to reference the newly created row.

While it may work, it's not safe in scenarios where multiple concurrent connections insert a row and then introspect the sequence.

Current behaviour

Unlike the identity column value which is scoped to a connection, the current sequence value is global (which is by design: the sequence value must be unique across all connections), so multiple concurrent connections get the same last inserted value.

How to reproduce

The following scenario was run on Oracle:

<?php

use Doctrine\DBAL\Connection;
use Doctrine\DBAL\Schema\Sequence;

function nextValue(Connection $connection, string $sequence) {
    return $connection->fetchOne(
        $connection->getDatabasePlatform()->getSequenceNextValSQL($sequence)
    );
}

$sm = $connection1->createSchemaManager();
$sm->createSequence(new Sequence('test_seq'));

var_dump(nextValue($connection1, 'test_seq'));
// string(1) "1"
var_dump(nextValue($connection2, 'test_seq'));
// string(1) "2"

var_dump($connection1->lastInsertId('test_seq'));
// int(2)
var_dump($connection2->lastInsertId('test_seq'));
// int(2)

The issue is not reproducible on PostgreSQL because the return value of CURRVAL(regclass) is scoped to the current session (documentation).

Additionally, some drivers that don't support sequences will just return the identity column value which is also unsafe:

  1. public function lastInsertId($name = null)
    {
    return $this->conn->insert_id;
    }
  2. public function lastInsertId($name = null)
    {
    return db2_last_insert_id($this->conn);
    }

Expected behavior

The sequence-based implementation has the same semantics as the one based on identity column (e.g. on MySQL):

$connection1->executeStatement('CREATE TABLE test_autoinc (id INT AUTO_INCREMENT, PRIMARY KEY (id))');

$connection1->executeStatement('INSERT INTO test_autoinc VALUES()');
$connection2->executeStatement('INSERT INTO test_autoinc VALUES()');

var_dump($connection1->lastInsertId());
// int(1)
var_dump($connection2->lastInsertId());
// int(2)
morozov added a commit to morozov/dbal that referenced this issue Jun 23, 2021
morozov added a commit to morozov/dbal that referenced this issue Jun 23, 2021
morozov added a commit to morozov/dbal that referenced this issue Jun 23, 2021
morozov added a commit that referenced this issue Jun 25, 2021
[GH-4687] Deprecate Connection::lastInsertId($name)
morozov added a commit that referenced this issue Jun 26, 2021
[GH-4687] Remove support for Connection::lastInsertId($name)
@morozov morozov linked a pull request Jun 28, 2021 that will close this issue
@morozov morozov added this to the 4.0.0 milestone Jun 28, 2021
@morozov morozov closed this as completed Jun 28, 2021
@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant