Duplicate entries on updateOrCreate

[chimit]

• Laravel Version: 5.4
• PHP Version: 7.1
• Database Driver & Version: MySQL 5.7

Description:

I want to save mobile devices information in the database using their UUIDs. The uuid field has a unique index. The code is:

$device = Device::updateOrCreate(
    ['uuid' => $request->input('uuid')],
    [
        'info' => $request->input('info'),
    ]
);

But I'm getting a lot of errors in logs:

Next Doctrine\DBAL\Driver\PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'aa18269c-cf32-4c6b-b450-e84a5dba0a0c' for key 'devices_device_uuid_unique' in /var/www/api/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOStatement.php:93
Stack trace:

I also tried to use firstOrNew:

$device = Device::firstOrNew(['uuid' => $request->input('uuid')]);
$device->info = $request->input('info');
$device->save();

But it's equal I guess.

If I remove the unique index from the uuid field the problem is gone but I get duplicate entries. That's strange because I thought updateOrCreate method guarantees uniqueness of the entries.

Is it a bug of the updateOrCreate method?

[Kyslik]

Whats your database structure, share migration for the table.

[srmklive]

Can you share your database structure or migration file for the table?

[chimit]

Schema::create('devices', function (Blueprint $table) {
    $table->increments('id');
    $table->integer('user_id')->unsigned()->nullable();
    $table->string('uuid')->unique();
    $table->timestamps();
});

[themsaid]

Since it happens on random occasions I believe it might be a race condition with the record is being created via another request in between the check and insert inside updateOrCreate().

[Reaper45]

Well. I think this is a Laravel mass assignment Issue. When using the Eloquent create method, you will need to specify either a fillable or guarded attribute on the model, as all Eloquent models protect against mass-assignment by default. In your case @chimit: add this to your device.php model

public $fillable = ['uuid', 'info'];

I put the solution code here.

[chimit]

Hi @Reaper45! I already have it in my Device model otherwise I would get Mass Assignment exceptions. The problem is that updateOrCreate method doesn't really guarantee what it was created for. Maybe this method should be improved, for example, using transactions.

[themsaid]

updateOrCreate method doesn't really guarantee what it was created for. Maybe this method should be improved, for example, using transactions.

Feel free to open a PR if you have a better idea, meanwhile I'm closing this since it's not actually a bug.

[alessandrolattao]

I am having the exact same problem. If at least two requests happen simultaneously, updateOrCreate does not behave correctly. You can easily reproduce the problem by using ab (apache bench) by using this command: ab -n 2 -c 2 http://your.url.here (I used a GET request for testing purpose). This generates two simultaneous requests to the "update url" and lead eloquent to generate two insert queries.

[lk77]

i think you could use lock to prevent simultaneous calls.

Device::where(['uuid'` => $request->input('uuid')]->lock(true);
//your code
Device::where(['uuid' => $request->input('uuid')]->lock(false); 

or something like that i've not tested it.

[alessandrolattao]

Yes, I could. But that would force me to add locks everywhere to handle updateOrCreate problems. I think the function should be atomic. :)

[rajjanorkar]

this is still problem.

[didioh]

I solved this problem by overriding Illuminate\Database\Eloquent\Model and creating a custom firstOrCreate method that returns the model if there is a duplicate error exception. This should work for mySQL connections.

class SystemModel extends Illuminate\Database\Eloquent\Model
{
    /**
    * Check if Illuminate\Database\QueryException is Duplicate Entry Exception.
    *
    * @param  array  $attributes
    * @return \Illuminate\Database\Eloquent\Model
    */
    protected function isDuplicateEntryException(Illuminate\Database\QueryException $e){
        $errorCode  = $e->errorInfo[1];
        if ($errorCode === 1062) { // Duplicate Entry error code
            return true;
        }
        return false;
    }

    /**
    * Get the first record matching the attributes or create it.
    *
    * @param  array  $attributes
    * @param  array  $values
    * @return \Illuminate\Database\Eloquent\Model
    */
    public static function firstOrCreate(array $attributes, array $values = [])
    {
        try {           
            $static = (new static);
            return $static->create($attributes + $values);
        } catch (Illuminate\Database\QueryException $e){
            if($static->isDuplicateEntryException($e)) {
                return $static->where($attributes)->first();
            }
        }
    }
}

[paulrrogers]

Please reopen this ticket. I'd agree with other commenters that this is an actual bug since it is surprising behavior for any moderately busy service where collisions are possible. While the implementation does not concern me the syntax implies it is somehow atomic.

This could be achieved in a variety of ways:

• upsert (syntax varies by DBMS)
• pessimistic locks ("SELECT ... FOR UPDATE")
• database transactions with at least one auto-retry
• partial mitigation with updated_at columns (typically needs sub-second precision).

In my opinion firstOrCreate also has this weakness and likely any ...OrCreate method.

All that said, I can see why the ...OrNew methods cannot be atomic since they only return unsaved, dirty objects. The others do not have that restriction.

[cg-rsands]

@themsaid this really is a bug. Race conditions are a big problem with updateOrCreate. We are fighting this at present, upsert is a safe method with mysql

[alessandro-aglietti]

Manage race conditions manually when using updateOrCreate with an ORM in 21st century?? C'mon!

[themsaid]

Guys, please open a PR if you have a different approach that works on all supported database engines :) PRs are welcomed.

[aligajani]

This is still a problem, albeit occurs very rarely.

[msdavid1296]

Still, I am getting the same issue.

(3/3) QueryException SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '344b879b-279d-f8e2-01e2-59083ac6f538' for key 'employee_id_unique' (SQL: insert into employee (id, title, first_name, last_name, office_phone, department, mobile, fax, primary_address, city, state, country, postal_code, email, back_account, citizenship, dob, doe, gender, salary, type_of_employeement, employee_no, is_sync, updated_at, created_at) values (344b879b-279d-f8e2-01e2-59083ac6f538, , Vitali, Berby, , , , , , , , , , , , , , , male, , , , 1, 2018-07-30 03:48:35, 2018-07-30 03:48:35))

[flexgrip]

I too am having this issue. For now, the only solution seems to be the old long form method of checking if there's an instance of the model, if not then create a new one, fill it, save, etc. Super inconvenient.

I'm not even sure how this is happening on my end. I don't think the calls are happening close enough together to cause the duplicates. Anyway, just hoping there's a better solution soon. Thanks.

[mst101]

@didioh 's approach works for me, as does adding a trait to the affected Model to override the updateOrCreate and firstOrCreate methods, as described here.

I agree with others that this should be default behaviour.

[whande1992]

I have a problem that may be the cause of your problem.

My table has an id column, language and description, I'm running the method for validating the id and language. To insert everything occurs perfectly.

ID| LANGUAGE | DESCRIPTION 1 1____language description 1 1 2____language description 2 1 _3____language description 3

The problem occurs when it is updated, when sending to update id= 1 language= 3 description= updating language 3

description :: updateOrCreate (['id' => 1, 'language' => 3],              ['description' => 'updating language 3'] );

it will be like this

ID | LANGUAGE | DESCRIPTION 11____ updating language 3 12____ lupdating language 3 1_3____ updating language 3

Laravel 5.6

[Gogtsu]

[ghost]

I had the same issue. I found a situation in which $added_products would contain records with the same primary key as records in DB.

$order->order_products()->createMany($added_products);

So (because there isn't updateOrCreateMany()) I changed this to:

foreach($added_products as $added_product) {
    $order->order_products()->updateOrCreate($added_product);
}

But that didn't work. Then I figured out that I hadn't specified the $primaryKey in the model file. After I added protected $primaryKey = ['order_id', 'product_id']; to the model file, it still didn't work because apparently Laravel supporting composite primary keys would make Eloquent too complex.

My solution, with composite primary key support

foreach($added_products as $added_product) {
    // $order->order_products()->updateOrCreate($added_product);
    \Helper::updateOrCreate($order->order_products(), $added_product);
}

And in the helper class:

public static function updateOrCreate($object, $attributes) {
    $primaryKey = [];
    $modelPrimaryKey = (array)$object->getRelated()->getKeyName();

    $attributes_withParentKey = $attributes;
    $attributes_withParentKey[$object->getForeignKeyName()] = $object->getParentKey();

    $values = $attributes; // without primary key

    foreach($modelPrimaryKey as $field) {
        $primaryKey[$field] = $attributes_withParentKey[$field];
        unset($values[$field]);
    }

    DB::transaction(function() use(&$object, $primaryKey, $values, $attributes) {
        // if record exists
        if (DB::table($object->getRelated()->getTable())->where($primaryKey)->first()) {
            // can't do $object->update($attributes) because one part of the PK would be in $attributes
            DB::table($object->getRelated()->getTable())->where($primaryKey)->update($values);
        } else {
            $object->create($attributes);
        }
    }, 5);
}

Also, Query\Builder::updateOrInsert() seems vulnerable to race conditions:

public function updateOrInsert(array $attributes, array $values = [])
{
    if (! $this->where($attributes)->exists()) {
        return $this->insert(array_merge($attributes, $values));
    }
    return (bool) $this->take(1)->update($values);
}

Shouldn't the method use DB::transaction? What if the record get deleted between ! $this->where($attributes)->exists() and $this->insert(array_merge($attributes, $values))?

[amenk]

This might be of help: Maybe we should make a pullrequest for this trait?

https://gist.github.com/troatie/def0fba42fcfb70f873b7f033fbe255f

[Yismen]

Just remove your foreign key from the $fillable array property and it will work. Foreign key should not be mass assigned. It worked for me.

[sweebee]

I've got (sometimes) the same issue:

                Product::updateOrCreate(
                    ['catalog_id' => $row['catalog_id'], 'product_code' => $row['product_code']],
                    $row
                );

only the ID is guarded but sometimes it adds duplicates.

[ludo237]

It's a race condition you should leverage the lock logic.

[tonviet712]

I've got the same issue (sometimes): SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry

[litlife]

I've also got the same rarely random issue when use updateOrCreate SQLSTATE[23505]: Unique violation: 7 ERROR: duplicate key value violates unique constraint Laravel Version: 5.8 PHP Version: 7.3.5 Database version: PostgreSQL 10.7 (Ubuntu 10.7-1.pgdg14.04+1)

[staudenmeir]

I've created an UPSERT package for all databases: https://github.com/staudenmeir/laravel-upsert

[gbwashleybrown]

Yeah, this is definitely still a problem. Whats the point in an updateORcreate method if it does the exact opposite

[msztorc]

Methods firstOrCreate and updateOrCreate are totally unusable for real application which allows simultaneous requests. Solve this or rename to firstOrDuplicate. 😃

[eduardodallmann]

Same problem

[bhuvidya]

I must admit I'm kinda stunned this hasn't been fixed natively in the Eloquent codebase. I've reverted to using a trait on my model classes that that wraps a "create advisory lock" mysql call around updateOrCreate() and firstOrCreate() - but it's a hack and will only work for MySQL.

[bhuvidya]

At the very least the docs should be explicit that these methods are not implemented using native database SQL queries, and are therefore not "transaction safe".

[duq]

Guys, please open a PR if you have a different approach that works on all supported database engines :) PRs are welcomed.

Is it possible to have different fixes for different database engines with the same outcome?

We should at least update the docs with a warning about using firstOrCreate/updateOrCreate methods: https://laravel.com/docs/5.8/eloquent#inserting-and-updating-models

[aligajani]

Let’s tweet to TO

On Wed, Jul 17, 2019 at 11:01 AM Peter Duda notifications@github.com wrote:

Guys, please open a PR if you have a different approach that works on all supported database engines :) PRs are welcomed.

Is it possible to have different fixes for different database engines with the same outcome?

We should at least update the docs with a warning about using firstOrCreate/updateOrCreate methods: https://laravel.com/docs/5.8/eloquent#inserting-and-updating-models

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/laravel/framework/issues/19372?email_source=notifications&email_token=AANGRWRAIAD6E7D5E5V2LODP73UYPA5CNFSM4DNCNYI2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2DWDMY#issuecomment-512188851, or mute the thread https://github.com/notifications/unsubscribe-auth/AANGRWS4E4LG3DOSX4SUFJLP73UYPANCNFSM4DNCNYIQ .

--

Ali Gajani Founder at Mr. Geek www.mrgeek.me www.aligajani.com

[novaknole]

I think, if you use index and have updateorcreate method, and put that inside try catch block, all is good.

Imagine two threads both enter the if statement. So both of them are gonna write into the database. when one will start inserting it, the table will be locked for the other one. So first one gets inserted, and another one waits. when first is done and second one starts inserting, it's gonna throw exception of duplicate event entries. YOu got the catch block right? so you catch it and nothing is a problem.

[hxgdzyuyi]

Is it possible to use INSERT ... ON DUPLICATE KEY UPDATE Syntax ?

[staudenmeir]

@hxgdzyuyi Please see https://github.com/laravel/ideas/issues/1090#issuecomment-377351778 why that is not a universal solution for updateOrCreate().

[waleedakhlaq]

how to avoid a duplicate entry in a db row?

[idezigns]

Just remove your foreign key from the $fillable array property and it will work. Foreign key should not be mass assigned. It worked for me.

This worked for me, my primary 'id' key is the only unique key in my table, once I removed this from fillable assignment in model it worked.

[kcassam]

Is this solution valid ?:

try {                
    $program = Program::updateOrCreate(['key' => $key], [...]);
} catch (PDOException $e) {
    // race condition, let's retry
    $program = Program::updateOrCreate(['key' => $key], [...]);
}

[comtelindodev]

is this fixed ? cause I got the same problem, Laravel 6

[amirasyraf]

Problem still exists. Laravel 7.5.2

[simonmaass]

can confirm issue still present

[driesvints]

Please open a new issue with steps to reproduce.