framework: Duplicate entries on updateOrCreate

this is still problem.

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.

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.

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.

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

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

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

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().

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

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

@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

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

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();
			}
		}
	}
}

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

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

@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.

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))

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”.

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

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

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

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

Problem still exists. Laravel 7.5.2

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

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.

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

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))?

This is still a problem, albeit occurs very rarely.

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

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.