Framework: Create method on model returning wrong primary key value

[amestsantim]

• Laravel Version: 5.7.20
• PHP Version: 7.2.10
• Database Driver & Version:
  DriverName => "libmsodbcsql-17.2.so.0.1",
  DriverODBCVer => "03.52",
  DriverVer => "17.02.0000",
  ExtensionVer => "5.3.0",

Description:

When creating records of a model using create the returned record will have a wrong value for primary key if the table has a trigger on it which itself creates (inserts) a record in another table.
In effect, the returned primary key value is the value of the record inserted by the trigger and not of the just created record of the model. Same behavior is observed if fresh() method is chained on the create method.

I suspect this is happening because the driver is using @@IDENTITY or SCOPE_IDENTITY whereas IDENT_CURRENT( 'table_name' ) should have been used to get the LastInsertId after the create step.
Here is the relevant section from the SQL Server 2017 documentation (https://docs.microsoft.com/en-us/sql/t-sql/functions/identity-transact-sql?view=sql-server-2017):

"@@IDENTITY and SCOPE_IDENTITY return the last identity value generated in any table in the current session. However, SCOPE_IDENTITY returns the value only within the current scope; @@IDENTITY is not limited to a specific scope.
IDENT_CURRENT is not limited by scope and session; it is limited to a specified table. IDENT_CURRENT returns the identity value generated for a specific table in any session and any scope. "

Steps To Reproduce:

Create a table (with an auto incrementing primary key) and add a trigger to it which inserts another record in another table). Then calling the create method on the model will return the just created record with the wrong value for the primary key field.

[amestsantim]

I believe the relevant code in the source is on line 46 of Illuminate/Database/Query/Processors/SqlServerProcessor.php

'SELECT CAST(COALESCE(SCOPE_IDENTITY(), @@IDENTITY) AS int) AS insertid'

[staudenmeir]

I can reproduce the issue, but I don't fully understand it.

When I test it directly in the database, SCOPE_IDENTITY() returns the correct primary key. In Laravel however, SCOPE_IDENTITY() returns null and so I receive the incorrect result of @@IDENTITY.

The issue with IDENT_CURRENT() is that it returns the last identity value generated in any session. So theoretically, a different connection could insert a new row between the INSERT and the SELECT IDENT_CURRENT() query. Then you would also get an incorrect value.

@darksaboteur You added the code in #13423, can you take a look at this?

[darksaboteur]

I have encountered this issue too when using Merge Replication.
I actually forgot I hadn't submitted my patches for this already.

I've thrown together a branch https://github.com/darksaboteur/framework/commit/0c5a0e23d078292f03d93eadd57cc0731890b1ce with my fixes copied to it for you to look at. I don't have time to test it now, so there may be some syntax errors or other trivial issues.

Please test it and let me know if it fixes triggers too so I can create a Merge Request for it.

[staudenmeir]

Here is a shorter version of @darksaboteur's fix: https://github.com/staudenmeir/framework/commit/b4a8f81c35c27e65c3fbc92c38afb84bbffcec44

@amestsantim Does it work for you?

@darksaboteur Now that SCOPE_IDENTITY() works, do we still need COALESCE(SCOPE_IDENTITY(), @@IDENTITY)? Shouldn't SCOPE_IDENTITY() cover all cases?

[driesvints]

Closing pending author inactivity. Feel free to reply if this is still an issue.

[amestsantim]

I'm not sure I understand why this issue is being closed. Has the bug been fixed in laravel/framework?
If so, great but if not, by closing the issue, we're just kicking the ball down to the next person who will create a new issue for it.

[driesvints]

@amestsantim I closed it because you didn't reply anymore. Can you please check the answers by @darksaboteur and @staudenmeir and answer their questions?

[amestsantim]

Alright, give me a few days to patch my installation with @staudenmeir 's fix and test it.

[driesvints]

Closing this issue because it's inactive, already solved, old or not relevant anymore. Feel free to reply if you're still experiencing this issue.

[skalero01]

I am experience the same issue. Using laravel 5.8 and php 7.3 @driesvints

To reproduce

Create a table

CREATE TABLE `seasons` (
  `season_id` int(4) NOT NULL,
  `name` varchar(30) DEFAULT NULL,
  `deleted_at` timestamp NULL DEFAULT NULL,
  `created_at` timestamp NULL DEFAULT NULL,
  `updated_at` timestamp NULL DEFAULT NULL,
  PRIMARY KEY (`season_id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;

Create a model Season.php

namespace App;

use Illuminate\Database\Eloquent\Model;

class Season extends Model
{
    protected $table = 'seasons';
    protected $primaryKey = 'season_id';
    protected $guarded = [];
}

On tinker execute App\Season::create(['season_id'=>2018, 'name' => 'Season 2018']).

That returns the next message

App\Season {#4086
     season_id: 0,
     name: "Season 2018",
     updated_at: "2019-08-03 20:48:40",
     created_at: "2019-08-03 20:48:40",
   }

My question is.. Why isn't returning the season_id correctly? On the database the season_id was saved correctly.

[staudenmeir]

@skalero01 That's a different issue. You need to set public $incrementing = false; in your model.

[skalero01]

Thanks @staudenmeir! it worked :)

[humblecoder]

@staudenmeir @skalero01 I need to make use of id immediately after create(). Setting $incrementing = false breaks that ability. For example:

$u = User::create(...);
$c = Car::create([
 'name' => 'blah',
 'user_id' => $u->id
]);

In this example, $u->id is actually null -- not even $u->fresh() works. However, if I leave $incrementing = true then I begin to receive incorrect values for $u->id. This is borne out by trying to use DB::getPdo()->lastInsertId() which also reports the wrong value and eventually becomes more and more incorrect.

NOTE: I'm sure there is a "cause" for the value getting off, but I have no earthly idea what it is.
Currently, I mitigate the problem by re-querying for the object based on identifiable traits, but this obviously isn't the fix.

[skalero01]

@humblecoder have you tried using the fresh function?

$u = User::create(...); $u = $u->fresh(); $c = Car::create([ 'name' => 'blah', 'user_id' => $u->id ]);

[humblecoder]

@skalero01 I have. It returns null. My guess is due to the fact that it attempts a lookup based on a bad id.

I need to do some digging into what makes the PDO tick. Is it possible to reset or "re-calibrate" the results of DB::getPdo()->lastInsertId()? 🤔 It's very strange that it gets "off track" and never recovers.

[staudenmeir]

I begin to receive incorrect values for $u->id

@humblecoder What does "incorrect" mean? Are you also using SQL Server?

[humblecoder]

@staudenmeir "incorrect" means the number slowly creeps off track. It's as if it has its own auto-increment mechanism. It has gotten as high as +4 off and seems it would keep climbing if I didn't manually address the issue.

Yes, I'm also using SQL Server.

[Serhioromano]

I have the same issue. I had tables and inserted thee some users but I had problems with constraint. I created a dump and deleted all the tables. Then I edited dump (MySQL) and change all AUTOINCREMENT to 1 so that tables start ёшвё from 1. Created DB again.

Now when I insert user it inserts it into DB with id 1 but User::create returns id 17.

Update

After research, I found out that I use listen to log all my SQL queries. Most probably lastInsertId returns Id of log entry.

How to avoid this?

[kadevland]

Can reopen this issu.
I have the same problem
I have many table with trigger so model don't take the good primary key value.
I use SQL server