What is the bug? It seems like this is how sqlserver is documented to behave.

according to the user in slack, it works if the following change is being done in the PDOAdapter

            if($this->getConnection()->getDriver() instanceof \Cake\Database\Driver\Sqlserver){
               $sql = 'SET IDENTITY_INSERT '.$this->quoteTableName($table->getName()). ' ON; '. $sql;
               $sql = $sql.' SET IDENTITY_INSERT '.$this->quoteTableName($table->getName()). ' OFF; ';
            }
            $this->getConnection()->execute($sql, $vals);

i think its just weird, that it works if its executed as 1 SQL command but does not, if its spread up on multiple commands.

The user reporting the issue explained it as "not being the same session/context" but as far as i can tell migrations doesnt recreate the connection object, therefore using the same PDO instance to do the queries.

It could be related to implicit transactions. I wonder if this would work if run within a transaction 🤔

As can be seen by the last commit and CI run even manually starting a transaction doesn't work.

Well that's unfortunate. SQLserver is a strange beast. I'm not in love with the type check in the middle of an abstract class.

We could duplicate the insert method into SqlserverAdapter and add the required changes. That would avoid the leaky implementation and lets us accommodate any other peculiarities that may arise.

nd add the required changes. That would avoid the leaky implementation and lets us accommodate any other peculiarities that may arise.

As the one who submitted the bug I am ALSO not a fan.. I did that as a Proof of Concept that the issue was with the Identity Insert not being "in context" if its executed in a seporate call even if its the same connection.... If there was a best practice as a user to override the adapters myself I would..

Or maybe add an "setIdentityInsert(bool:$enabled)" function and then for SQL that adds the identity statements to the inserts, and for other adapters they just ignore the new property? But then you have to update PDOAdapter, and pass the property around so they all know to look for it.. it might add a lot of noise to the public API just because SQL Server is being a jerk..

If you guys are ok with it, I could write it up and send you guys a PR of what I am suggesting above.

Thank you @markstory for merging the propsed fix into this PR..

Thank you @markstory for merging the propsed fix into this PR..

You're welcome, thank you for the pull request 👍