[10.x] Adds a createOrFirst method to Eloquent

[tonysm]

Added

• [internal] Adds mariadb, postgres, and mssql container images to the local docker-compose.yml to ease local testing. They are commented out because we may not always need them locally, but they are handy to have around when dealing with Database/Eloquent features
• Adds a new UniqueConstraintViolationException that is a sub-class of QueryException whenever a unique constraint violation is detected
• Adds the createOrFirst method to the Eloquent Builder and Relations
• Adds the createOrRestore macro to the soft deleting scope
• POTENTIAL SemVer BC (not sure): Adds a new matchesUniqueConstraintException protected method to the base Connection class. The default implementation returns false as there's no way to generalize the unique constraint violation detection to all drivers. It's up to each child class of Connection to implement the correct way to detect UNIQUE constraint violations. The drivers Laravel ships by default all implemented this method. Custom drivers must implement it if they want to support the usage of createOrFirst. Not implementing it should only be an issue when users of the custom driver attempt to use createOrFirst, since it will throw an exception instead of the correct behavior (which should be easy to catch with tests).

Changed

• The firstOrCreate method now uses the createOrFirst method under the hood (instead of create) to fix the race condition in the firstOrCreate method. It now tries to find the model. If it's missing, it tries to create. If a unique constraint violation happens when trying to create, it will attempt another find.

---

Context

This method is similar to the firstOrCreate, but should work better in highly concurrent environments. With firstOrCreate we may run into a race condition as two processes may not see an existing record created if they are running at the same time (or when you use read/write replicas and your read-replica is catching up), and when both attempt to create the record they would unexpected errors. Either one of the attempts to create would throw an exception unexpectedly due to a UNIQUE constraint violation if there's one in the table, or this race condition would generate duplicate entries if the UNIQUE constraint is missing.

With createOrFirst, we invert this flow and rely on the tables having a UNIQUE constraint. So, first, we attempt to create the record, and if we get an exception back from the database and identify that it's a unique constraint violation, we attempt to find the matching record instead. This way, concurrent processes may rely on the ACID characteristics of the database and never have to worry about that race condition again.

With that being said, the createOrFirst method does not replace firstOrCreate entirely as it relies on having UNIQUE constraints in the database, which the latter does not.

To detect that we ran into a UNIQUE constraint violation, we are matching against the exception message:

• For MySQL, we only really needed to match the error code 1066, but for some reason, the $exception->getCode() was returning 23000, so instead of matching on the code, I went with matching against the error message (see here, look for "ER_NONUNIQ_TABLE").
• For SQLite, I'm matching on the error message, too, using the same RegEx that Rails uses on their unique error handling (see here).
• Postgres returns a 23505 error message for unique constraint violations (here).
• SQLServer I matched the error message (I don't know much about it, tbh).

[tonysm]

One thing missing here is an extension hook for custom drivers to add their own way of detecting unique constraint violations.

[driesvints]

@antonkomarev @michaelnabil230 instead of downvoting, could you please tell why you don't like the PR?

[devajmeireles]

@antonkomarev @michaelnabil230 instead of downvoting, could you please tell why you don't like the PR?

Don't wait for answers

Particularly I believe that the PR is very well done and has a clear purpose given the author's explanation, I just don't agree with the name, I think it can be confusing mainly about the fact that we already have something similar, firstOrCreate. Maybe another name is better for it, such as uniqueOrCreate ou firstOrCreateSafe? 🤷‍♂️

[michaelnabil230]

@antonkomarev @michaelnabil230 instead of downvoting, could you please tell why you don't like the PR?

Don't wait for answers

Particularly I believe that the PR is very well done and has a clear purpose given the author's explanation, I just don't agree with the name, I think it can be confusing mainly about the fact that we already have something similar, firstOrCreate. Maybe another name is better for it, such as uniqueOrCreate ou firstOrCreateSafe? 🤷‍♂️

I agree with you on the same thing

[antonkomarev]

@driesvints I downvoted this PR because it will work as expected only in case when there is UNIQUE constraint in the table, but nobody will protect developer from using new method without such constraint. Then it will work exactly same way as default create method.

I'd prefer to have new exception type which will be thrown from create and firstOrCreate methods on constraint violation. Let developers to catch it and decide what they want to do, find latest record or do anything else.

[tonysm]

@devajmeireles

Particularly I believe that the PR is very well done and has a clear purpose given the author's explanation, I just don't agree with the name, I think it can be confusing mainly about the fact that we already have something similar, firstOrCreate. Maybe another name is better for it, such as uniqueOrCreate ou firstOrCreateSafe? 🤷‍♂️

I thought that too, but not sure if it's an issue. Sounds more like a documentation problem and the difference should be noted in the docs. Also, other frameworks have a similar API for these cases, like the find_or_create_by vs. create_or_find_by in Rails' ActiveRecord, for instance (see here).

Actually, after taking a look at the Rails implementation, I noticed that the find_or_create_by method actually uses the create_or_find_by one behind the scenes, which makes sense as it would mitigate the race condition issue in the first_or_create_by (it first tries to find, then tries to create if missing, but if a unique violation exception is thrown, it assumes it was a race condition and switches to another find attempt one more time). I can work on those changes to this PR if it's accepted.

@antonkomarev

I downvoted this PR because it will work as expected only in case when there is UNIQUE constraint in the table, but nobody will protect developer from using new method without such constraint. Then it will work exactly same way as default create method.

Right now, a race condition may cause unexpected exceptions with the current implementation of the firstOrCreate method, besides allowing data integrity issues (if the table has no UNIQUE constraint, you may end up with duplicated data anyways). To me, the issue you mentioned can be fixed with documentation. We should ensure the documentation points out the difference between both methods.

I'd prefer to have new exception type which will be thrown from create and firstOrCreate methods on constraint violation. Let developers to catch it and decide what they want to do, find latest record or do anything else.

I actually thought about introducing a new UniqueConstraintException that would be a sub-type of QueryException. This would simplify the detection of a UNIQUE constraint violation. I originally thought it would be a lot of effort to make that happen on this PR, but after another look, looks like we only have to add the new exception type and we should be able to run the existing detection code and throw the more specific exception in the Connection class (as it looks like this is the only place where the QueryException is thrown - see here).

[tonysm]

Actually, the more I think about the UniqueConstraintException, the more it makes sense. Doing that would allow us to move the UNIQUE detection to each child's Connection classes, so they would be co-located in the driver. I'm gonna make that change anyways.

[tonysm]

Not sure about this default implementation. Please, let me know what y'all think about this. The only issue I see here is that custom drivers should implement this method, otherwise the createOrFirst won't work.

[tonysm]

Alright, y'all. I've made some changes and updated the PR description. In summary, I've added a UniqueConstraintViolationException that extends QueryException, and then pushed the detection to the base Connection class. The default implementation return false, it's up to each child of the Connection class to implement the detection. It's ready for review again. Let me know what y'all think!

[devajmeireles]

Alright, y'all. I've made some changes and updated the PR description. In summary, I've added a UniqueConstraintViolationException that extends QueryException, and then pushed the detection to the base Connection class. The default implementation return false, it's up to each child of the Connection class to implement the detection. It's ready for review again. Let me know what y'all think!

Awesome! 🔥

[taylorotwell]

Thanks 👍

[crishoj]

For MySQL, we only really needed to match the error code 1066, but for some reason, the $exception->getCode() was returning 23000

@tonysm — just FYI, the MySQL error code is available in QueryException->errorInfo[1]. If you wish to avoid matching against the error message, this could work:

    protected function isUniqueConstraintError(Exception $exception)
    {
        return $exception instanceof QueryException
            && is_array($exception->errorInfo)
            && $exception->errorInfo[1] == 1062;
    }

[mpyw]

@crishoj Nice comment! I used to provide this feature as a third-party library and was careful not to rely on string comparison for messages, but rather to use the information provided by errorInfo as much as possible.

• mpyw/unique-violation-detector: Detect primary/unique key or constraint violation errors from PDOException.
  • Depended by mpyw/laravel-unique-violation-detector: Detect primary/unique key or constraint violation errors from PDOException.
    • Depended by mpyw/laravel-retry-on-duplicate-key: Automatically retry non-atomic upsert operation when unique key constraints are violated.

unique-violation-detector/src/MySQLDetector.php at master · mpyw/unique-violation-detector

class MySQLDetector implements UniqueViolationDetector
{
    public function uniqueConstraintViolated(PDOException $e): bool
    {
        // SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry
        return $e->getCode() === '23000' && ($e->errorInfo[1] ?? 0) === 1062;
    }
}

I think the impact of the breaking changes is minimal, so I don’t see any problem with having a pull request that implements these changes. If you have the motivation, I think it would be great to submit one.

[mpyw]

I have summarized the exchanges so far on Zenn, a Japanese technical article sharing site. I hope that non-Japanese speakers would read it through translation.

[Laravel] createOrFirst の登場から激変した firstOrCreate, updateOrCreate に迫る！