[7.x] Improve SQL Server last insert id retrieval

[rodrigopedra]

This PR addresses issue #32883

It adds the method SqlServerGrammar@compileInsertGetId, and if it gets merged this method will change
an INSERT statement from this:

insert into [users] ([email]) values (?)

Into this:

set nocount on;
insert into [users] ([email]) values (?);
select scope_identity() as [id]

line breaks added for readability

There are other features in SQL Server that allows retrieving the last inserted ID, but from my research,
for a INSERT statement that inserts a single row, SCOPE_IDENTITY seems to be the more reliable.

The alternative methods are:

• OUTPUT clause: can yield wrong results when target table has an INSTEAD OF INSERT trigger.

  For INSTEAD OF triggers, the returned results are generated as if the INSERT, UPDATE, or DELETE
  had actually occurred, even if no modifications take place as the result of the trigger operation.

  reference: https://docs.microsoft.com/en-us/sql/t-sql/queries/output-clause-transact-sql

• IDENT_CURRENT() function: not transaction safe.

  IDENT_CURRENT returns the last identity value generated for a specific table in any session and any scope

  reference: https://docs.microsoft.com/en-us/sql/t-sql/functions/ident-current-transact-sql

• @@IDENTITY system function: not trigger safe.

  If the statement fires one or more triggers that perform inserts that generate identity values,
  calling @@IDENTITY immediately after the statement returns the last identity value generated by the triggers.

  reference: https://docs.microsoft.com/en-us/sql/t-sql/functions/identity-transact-sql

I first tried using the OUTPUT clause as from my previous knowledge was a recommended solution
from Microsoft due to some bugs with SCOPE_IDENTITY and @@IDENTITY

reference: https://support.microsoft.com/en-us/help/2019779/you-may-receive-incorrect-values-when-using-scope-identity-and-identit

But according to the same reference above the bug was fixed in SQL Server 2005, which is not supported anymore
neither by Microsoft nor by Laravel.

Also using the OUTPUT clause incurred in using a temporary TABLE variable to address triggers usage.
And as noted on the list above the OUTPUT clause can yield incorrect results if the target table
has an INSTEAD OF INSERT trigger.

So the most reliable way seems to use SCOPE_IDENTITY().

Note that I added SCOPE_IDENTITY() in the same SQL generated by compileInsertGetId, so it is guaranteed to be
run in the same session as the INSERT statement.

This is necessary as per docs (and my local testing), SCOPE_IDENTITY() is safe on the
same scope (e.g. triggers won't affect it) and session (connection, transaction, etc).

Returns the last identity value inserted into an identity column in the same scope. A scope is
a module: a stored procedure, trigger, function, or batch. Therefore, if two statements are in
the same stored procedure, function, or batch, they are in the same scope.

reference: https://docs.microsoft.com/en-us/sql/t-sql/functions/scope-identity-transact-sql

I also removed the ODBC part from the SqlServerProcessor as it run the query on separate database call.
In my local testing, when the target table of a INSERT statement has a trigger which inserts a row
on another table, using SCOPE_IDENTITY on separate database call returns an incorrect result
(the auto generated id from the row inserted by trigger).

How this PR differs from previous attempts

I found two other previous attempts to solve this:

1. PR [8.x] SqlServer schema fixes #32957

   This PR ended having a very similar code to that PR (difference being the column name used in the SqlServerGrammar@compileInsertGetId)

   As mentioned above, I first tried using the OUTPUT clause, as from my previous knowledge seemed to be
   a better solution, but after some research on MSDN and other sources I ended up using SCOPE_IDENTITY() as
   PR [8.x] SqlServer schema fixes #32957.

   Difference is this PR only addressees the last insert id reported on issue Create method on model returning wrong primary key value (SQLSRV) #32883, whereas PR [8.x] SqlServer schema fixes #32957 addresses
   other SQL Server related issues.

   I would close this PR in favor of PR [8.x] SqlServer schema fixes #32957 if that is preferred, as it is more feature complete.

2. Branch sqlsrv-insert-id from @staudenmeir fork

   Issue Create method on model returning wrong primary key value (SQLSRV) #32883 OP (@kadevland) mentions that fork. The only difference is that this PR does not fallback
   to @@IDENTITY, as per SCOPE_IDENTITY() docs (linked above):

   The SCOPE_IDENTITY() function returns the null value if the function is invoked before any INSERT statements
   into an identity column occur in the scope.

   Which is not the case as the SELECT SCOPE_IDENTITY() is added in the same SQL code as the INSERT statement.
   Also as noted above @@IDENTITY can return the wrong value if the target table of the INSERT statement has
   an associated trigger.

   Also @staudenmeir did not make a PR from his fork. I consider him to have more knowledge regarding databases and
   Laravel's Eloquent/Query Builder than me. If he knows any issues that prevented him to make a PR from his fork -
   maybe he is researching or know a better solution to this problem - I would bet on his attempt and close this PR.

Implementation notes

SqlServerProcessor

The changed code in the SqlServerProcessor class was copied from the same method in the PostgresProcessor class.

Also as already mentioned above, I removed the ODBC special handling to avoid executing SCOPE_IDENTITY()
in a separate query.

SET NOCOUNT ON

set nocount on is needed so SQL Server returns the SELECT results instead of the affected rows count.

Testing

I updated an existing test and added a new one to test the code changed by this PR.

Real usage testing

I setup a local application with SQL Server 2017 running in a docker instance (I run Linux) and did some local
tests with sample application code. SQL Server version 2017 was used because I already had this docker image installed
for some other projects I need it.

Everything worked as expected. I tested both inserts in a table with no triggers and in a table with a INSERT trigger.

If someone has any suggestions or spot any errors please let me know and I'll make any improvements needed.

[mikizdr]

@rodrigopedra Hi. Please help. I'm trying to realise if this merge is active or reverted. I have installed Laravel v.7.25.0 and I don't have the mentiond changes in the merge linked with PR #32883. I have that issue with MSSQL and returning ghosts PK. When I implement the merged solution mannually, it works fine. I didn't find any side effect which doesn't mean it won't show up. So the bottom line: is the merge active or not? I need it in my project. Thanks

[rodrigopedra]

@mikizdr this PR was reverted due to issues when using freeTDS drivers to interact with a SQL Server database.

PRs that reverted this one were #33488 (in 7.x series) and #33496 (in 6.x series).

Issue #32883 (the issue that motivated this PR) has some comments on suggestions about handling this issue.

Hope it helps.

---

EDIT: to be more correct about this, PR #33496 reverted this one in the 6.x series and was later merged to the 7.x series per Laravel code management workflow

[mikizdr]

Yes, it helped. Thanks for help and fast answer.
BTW: I implementd SET NOCOUNT ON on the trigger but still no result. It seems I must turn it off.