MSSQL IDENTITY_INSERT Issues

[jedkirby]

I've run into issues with the IDENTITY_INSERT on MSSQL using this package, on any version that's higher or equal to 2.5.11, however, I'm unable to identify the root cause of it.

Just to be clear, the following is not an issue on 2.5.10.

Migration Commands

I've been using DBAL to run the following from within a database migration:

$this->addSql("SET IDENTITY_INSERT TableName ON");

Then following that I'm doing this to insert fix a fixed ID row entry:

$this->addSql(
    "INSERT INTO TableName (id, title) VALUES (123, 'My Title');"
); 

And after that, turning off the ability to insert with identities:

$this->addSql("SET IDENTITY_INSERT TableName OFF");

The Issue

Regardless of whether I set the IDENTITY_INSERT to ON or not, I get the following error:

SQLSTATE [23000, 544]: [Microsoft][ODBC Driver 11 for SQL Server][SQL Server]Cannot insert explicit value for identity column in table 'TableName' when IDENTITY_INSERT is set to OFF.

Something somewhere in 2.5.11 is actually not performing the setting of the IDENTITY_INSERT.

Please could someone provide some help on this? I'm currently fixing my version of this package to 2.5.10, but that's not a long term solution.

System

Machine: Windows Server 2012 R2 Datacenter
PHP: 5.6.17

[Ocramius]

Attach an SQL logger to the connection, see what is going on with that

…

On 28 Jun 2017 16:49, "James Kirby" ***@***.***> wrote: Background I've run into issues with the IDENTITY_INSERT on MSSQL using this package, on any version that's higher or equal to 2.5.11 <https://github.com/doctrine/dbal/releases/tag/v2.5.11>, however, I'm unable to identify the root cause of it. Just to be clear, the following is not an issue on 2.5.10. Migration Commands I've been using DBAL to run the following from within a database migration: $this->addSql("SET IDENTITY_INSERT TableName ON"); Then following that I'm doing this to insert fix a fixed ID row entry: $this->addSql( "INSERT INTO TableName (id, title) VALUES(38001, 'My Title');" ); And after that, turning off the ability to insert with identities: $this->addSql("SET IDENTITY_INSERT TableName OFF"); The Issue Regardless of whether I set the IDENTITY_INSERT to ON or not, I get the following error: SQLSTATE [23000, 544]: [Microsoft][ODBC Driver 11 for SQL Server][SQL Server]Cannot insert explicit value for identity column in table 'JaduHomepageWidgets' when IDENTITY_INSERT is set to OFF. Something somewhere in 2.5.11 is actually not performing the setting of the IDENTITY_INSERT. Please could someone provide some help on this? I'm currently fixing my version of this package to 2.5.10, but that's not a long term solution. System *Machine:* Windows Server 2012 R2 Datacenter *PHP:* 5.6.17 — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2752>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAJakIADUM1CI20DVqzh3yh243ApVWF8ks5sImW2gaJpZM4OIECQ> .

[jedkirby]

@Ocramius - see the attached below, unfortunately i'm having to obfuscate the output as it's got confidential information within, but, hopefully this'll be able to help you a little more.

Let me know if you need anything else.

[Ocramius]

The queries seem ok: can you reproduce the behaviour by firing manual queries just through the driver itself? Or maybe on a terminal?

[jedkirby]

I've manually added the SQL into Microsoft SQL Studio, and it's processed it correctly:

Unfortunately you may have to try this for yourself, which will involve creating an environment to do so.

Sorry i'm unable to give a more detail explanation, it's a tough issue to figure out as it looks like it's sending the queries correctly, but doesn't seem to be processing them.

[Ocramius]

@jedkirby what about doing it manually in plain PHP? I start thinking this is just one of the many many many many issues with the sqlsrv integration at extension level.

[jedkirby]

Morning @Ocramius, unfortunately running it manually in plain PHP also works.

This is the code that was executed (Again, the actual DB credentials, table name, & data have been removed, but I've tested using the exact data that originally failed):

$connection = sqlsrv_connect(
    '...',
    [
        'UID' => '...',
        'PWD' => '...',
        'Database' => '...',
    ]
);

$queries = [
    [
        'sql' => "SET IDENTITY_INSERT TableName ON",
        'params' => [],
    ],
    [
        'sql' => "INSERT INTO TableName (id, title) VALUES (?, ?)",
        'params' => [123, "My Title"],
    ],
    [
        'sql' => "SET IDENTITY_INSERT TableName OFF",
        'params' => [],
    ],
];

foreach ($queries as $query) {

    $stmt = sqlsrv_query($connection, $query['sql'], $query['params']);

    if ($stmt) {
        echo 'Statement executed.' . "\n\r";
    } else {
        echo 'Error in statement execution.' . "\n\r";
    }

    sqlsrv_free_stmt($stmt);

}

sqlsrv_close($connection);

And this is the response:

Which successfully enters the item into the database with the correct ID.

[Ocramius]

@jedkirby and doing the same exact thing with a Doctrine\DBAL\Connection#query()?

[Ocramius]

Also, it could be that further SQL statements are being executed in between - see #2648

[jedkirby]

@Ocramius - do you specifically mean to use the master branch, i.e. 5c5966? If so, i'm unable to test using that as it's PHP 7.1, and this implementation uses 5.6.

[Ocramius]

@jedkirby you can test with any implementation. We are still searching for the cause of this :-)

[jedkirby]

So i've run the following code against both the 2.5.10 and 2.5.11 versions, of which only 2.5.10 worked, as you'll be able to see in the screenshot attached after the code:

require_once realpath(__DIR__ . '/../vendor/autoload.php');

use Doctrine\DBAL\Configuration;
use Doctrine\DBAL\DriverManager;
use Doctrine\DBAL\Logging\EchoSQLLogger;

$config = new Configuration();
$config->setSQLLogger(
    new EchoSQLLogger
);

$params = array(
    'dbname' => '...',
    'user' => '...',
    'password' => '...',
    'host' => '...',
    'driver' => 'sqlsrv',
);

$conn = DriverManager::getConnection($params, $config);

$queries = [
    [
        'sql' => "SET IDENTITY_INSERT TableName ON"
    ],
    [
        'sql' => "INSERT INTO TableName (id, title) VALUES (123, 'My Title')",
    ],
    [
        'sql' => "SET IDENTITY_INSERT TableName OFF",
    ],
];

foreach ($queries as $query) {

    $stmt = $conn->query($query['sql']);

    if ($stmt) {
        echo 'Statement executed.' . "\n\r";
    } else {
        echo 'Error in statement execution.' . "\n\r";
    }

}

$conn->close();

The results:

As you can probably see, the first command php .\test-2.5.10\src\test.php returns all three of the "Statement executed.", however, the second php .\test-2.5.11\src\test.php returns only the first, I presume because it failed somehow but gave no exception / error message.

NB: After the first command I manually remove the successful entry from the DB to allow the second command to not run into issues.

[jedkirby]

@Ocramius, think we may have found the potential issue (Thanks to @steadweb for the help).

I changed the plain PHP test above (#2752 (comment)) to use sqlsrv_prepare instead of sqlsrv_query, as has been added in https://github.com/doctrine/dbal/pull/2546/files#diff-9fb6b286e7dc6e4b64272eae7fd361fbR263, which is now causing the same error to be produced.

This is the new block of code, which uses the same functions as that PR uses:

// ...

foreach ($queries as $query) {

    $stmt = sqlsrv_prepare($connection, $query['sql'], $query['params']);
    $response = sqlsrv_execute($stmt);

    if ($response) {
        echo 'Statement executed.' . "\n\r";
    } else {
        echo 'Error in statement execution.' . "\n\r";
        die( print_r( sqlsrv_errors(), true));
    }

    sqlsrv_free_stmt($stmt);

}

// ...

As you can see 2.5.10 used sqlsrv_query - https://github.com/doctrine/dbal/blob/v2.5.10/lib/Doctrine/DBAL/Driver/SQLSrv/SQLSrvStatement.php#L200 but now 2.5.11 uses sqlsrv_prepare & sqlsrv_execute, which is where the problem lies.

This may be able to be reverted, as sqlsrv_query actually does the preparing and execution within itself, though..

[jedkirby]

After doing some digging, I've found this issue microsoft/msphpsql#385 which explains, in some depth, that:

Prepared statements result in RPCs ( Remote Procedure Calls ) in SQL Server, which are run in different processes.

That means, because DBAL is now using prepared statements, it executes the SQL on different processes, thus is why the identity is set to off.

I'm happy to have a stab at fixing this up, moving back to sqlsrv_query, if you're happy with the above findings?

[Ocramius]

which are run in different processes.

SERIOUSLY MSSQL?! :-(

[Ocramius]

I'm happy to have a stab at fixing this up, moving back to sqlsrv_query, if you're happy with the above findings?

This depends on the implications of moving back to sqlsrv_query. If I recall correctly, queries containing stored procedure calls wouldn't work in that scenario.

[morozov]

I changed the plain PHP test above (#2752 (comment)) to use sqlsrv_prepare instead of sqlsrv_query, as has been added in https://github.com/doctrine/dbal/pull/2546/files#diff-9fb6b286e7dc6e4b64272eae7fd361fbR263, which is now causing the same error to be produced.

IIRC, the switch from sqlsrv_query() to sqlsrv_prepare() happened in #2494 (not in #2546). @jedkirby could you clarify which of the two changes causes the problem?

I'm happy to have a stab at fixing this up, moving back to sqlsrv_query, if you're happy with the above findings?

This depends on the implications of moving back to sqlsrv_query. If I recall correctly, queries containing stored procedure calls wouldn't work in that scenario.

@Ocramius the reason to switch was better performance and in general correspondence to the rest of supported drivers (see #2493).

[jedkirby]

@morozov, correct me if i'm wrong but #2494 isn't within any of the 2.5.* releases, is it? It's only within the unreleased master/develop branch, which I presume will be 3.0.0?

If so, that'd mean that it was #2546 that brings the problem into the >= 2.5.11 tag(s)? Which would explain why this works in <= 2.5.10, as that PR was only merged after .10.

Here's some references for the files:

• https://github.com/doctrine/dbal/blob/v2.5.10/lib/Doctrine/DBAL/Driver/SQLSrv/SQLSrvStatement.php#L200
• https://github.com/doctrine/dbal/blob/v2.5.11/lib/Doctrine/DBAL/Driver/SQLSrv/SQLSrvStatement.php#L227

With that in mind, the issue will still reside in the new 3.0.0 version, if that's what's planned, however, that's a major release so breaks in BC are possible.

[Ocramius]

@jedkirby I'd gladly merge a revert, if that fixes the issue: at this point, it is clear that obtaining MSSQL compatibility is already going to butcher performance (the new "last insert id compatibility fixes in #2648 even introduce a new query for every executed query, and there's no way around it).

@morozov what's your PoV here?

[morozov]

@Ocramius given that Connection::query() doesn't accept parameters, why do we prepare a statement internally in the SQL Server driver? Would it help if instead of $stmt = $this->prepare($sql); $stmt->execute(); we did $stmt = sqlsrv_query($sql);?

@jedkirby judging by your example, you're not using parameters in your queries. Is it only an example or you're not using them in your production queries? Would it help if the SET IDENTITY query was executed using sqlsrv_query() but the actual queries were executed as statements?

I may be biased as switching to reusing statements saved me ~25% of the execution time in SQL, but it still looks irrational to me to solve a corner case issue (which insertion to an identity column looks to me) at the cost of overall performance. If we can solve the problem by not using prepared statements in this case (i.e. re-implement the query() method in SQL Server Connection), I'd say it should be a suggested workaround.

[jedkirby]

@morozov - the queries that are being shown above simulate what is actually being used within a Doctrine DBAL Migration, although, I've edited it to remove sensitive data. I can confirm there's no parameters being used, and it's only SQL that's being executed (Via the addSql() method), however, I can't say that in another migration, or in future migrations, there's no parameters being used.

I don't think that even if you modify this to use sqlsrv_query() for only SET IDENTITY that this'll work, because, that query and the one that'll be run via $stmt = $this->prepare($sql); $stmt->execute(); will be run in different processes, thus, meaning that the SET IDENTITY is useless.

The only way I can see it working, using execute is combining the SQL so it's prefixed & suffixed with SET IDENTITY on and off respectively, which, for us, will be a break in BC because we can't go back and change previous migrations to enforce that.

[morozov]

@jedkirby thank you for clarification. I see that addSql() accepts parameters so, in general, any query in a migration script can contain them.

@Ocramius I know it doesn't make things cleaner, but what if we introduce a switch which would define whether prepare() + execute() or query() should be used? Essentially, it would be the PDO::SQLSRV_ATTR_DIRECT_QUERY flag implemented in the DBAL.

[Ocramius]

what if we introduce a switch

No, that's probably the worst of both worlds

[jedkirby]

Hey guys, @morozov & @Ocramius - are we any further forward with this, or do you need anything from me?

[Ocramius]

@jedkirby attempting a patch that provides additional tests would be a good start

[Ocramius]

@jedkirby also, if you can help on #2617 that would simplify stabilizing the entire feature

[martinpfeiferdev]

@Ocramius
I'm in the same situation.
Any position?

[martinpfeiferdev]

@Ocramius @jedkirby

I'm working on a project that has several primary composite keys.
Some entities have a primary key with auto increment + composite keys, I understand that the architecture of this project is a horrible one, but unfortunately it is the way I can work authentically.

I got a solution.
example:

`class TesteRepository extends EntityRepository implements TesteRepositoryInterface
{
/**
* @param Teste $teste
* @return void
*/
public function save(Teste $teste)
{
$entityManager = $this->getEntityManager();
$entityManager->persist($teste);

    $con = $entityManager->getConnection();
    $metaData = $entityManager->getClassMetadata(Teste::class);
    $metaData->setIdGeneratorType(ClassMetadata::GENERATOR_TYPE_NONE);
    $con->exec("SET IDENTITY_INSERT {$metaData->getTableName()} ON");

    $entityManager->flush();
}

}`

[morozov]

@martinepic it will work if the query doesn't have parameters like in your case but won't work for migrations as in the original one.

If only we could Statement::query() signature and add $params like query($sql, array $params = []), we could solve this problem entirely. But before that, we need to understand what additional arguments of PDOConnection::query() can be used for and if it's a legit use case.

Looks like the PDO legacy is biting us in the ass same as in #1097.

[gezilixu]

I am using laravel to execute db:seed command to transfer MySQL data to sqlserver. This problem also occurs. Can someone tell me how to solve it?
DBAL: 2.12.x
PHP: 7.4

[Kerrialn]

Hey guys, is there any movement forward on this? is there even a work around? For example a listener appending to the SQL on TransactionBegin and set the identity insert to on. Let me know if I can help.

[DominicDetta]

I am using this kind of workaround to solve the problem:

    private function insert($connection, $table, array $data)
    {
        $columns = [];
        $values = [];
        $set = [];

        foreach ($data as $columnName => $value) {
            $columns[] = $columnName;
            $values[] = $value;
            $set[] = '?';
        }
        $connection->executeStatement(
            'SET IDENTITY_INSERT ' .
                $table .
                ' ON;' .
                'INSERT INTO ' .
                $table .
                ' (' .
                implode(', ', $columns) .
                ')' .
                ' VALUES (' .
                implode(', ', $set) .
                ');' .
                'SET IDENTITY_INSERT ' .
                $table .
                ' OFF;',
            $values
        );
    }

Rewriting the method in my class does the job.