[10.x] Revival of the reverted changes in 10.25.0: firstOrCreate updateOrCreate improvement through createOrFirst + additional query tests

[mpyw]

Prior to createOrFirst Improvement

Traditionally, Laravel's firstOrCreate and updateOrCreate merely performed actions of:

• Try fetching, if not found, create a new one
• Try fetching, if not found, create a new one, if found, update it

These were not exclusively locked, leading to duplicate entries being created in highly concurrent environments, possibly triggering errors if unique key constraints were present.

Introduction of createOrFirst

In light of this, changes were made between Laravel 10.19 to 10.21:

• [10.x] Adds a createOrFirst method to Eloquent #47973
• [10.x] Fix createOrFirst on transactions #48144
• [10.x] Enhancing updateOrCreate() to Use firstOrCreate() #48160
• [10.x] Ensuring Primary Reference on Retry in createOrFirst() #48161
• [10.x] Make the firstOrCreate methods in relations use createOrFirst behind the scenes #48192
• [10.x] Make the updateOrCreate methods in relations use firstOrCreate behind the scenes #48213
• [10.x] HotFix: throw captured UniqueConstraintViolationException if there are no matching records on SELECT retry #48234

With these changes, createOrFirst was introduced, which performed:

• Try creating, if a unique key error occurs, fetch

This utilizes the unique key constraint. The improvement also refactored firstOrCreate and updateOrCreate to internally reuse createOrFirst, making them all resilient to parallel execution.

Degradations Due to New Features/Improvements

However, these changes triggered several issues:

Unintended behavior in tests using RefreshDatabase

Issues and PRs:

• RefreshDatabase + double wrapping in DB::transaction() breaks $afterCommit functionality #48451
• afterCommit() callbacks do not fire when RefreshDatabase handles multiple $connectionsToTransact #48472
• [10.x] Fix "after commit" callbacks not running on nested transactions using RefreshDatabase or DatabaseMigrations #48523

This was due to a poor compatibility with afterCommit introduced in #35373, a feature to execute events after commit. Rather than the PRs related to createOrFirst directly causing issues, they merely surfaced bugs that were already present. This has already been fixed.

Unintended behavior with HasManyThrough

Issues and PRs:

• [10.x] Failing test/bug report in HasManyThrough updateOrCreate() since Laravel 10.21 #48529
• [10.x] Revert from using createOrFirst in other *OrCreate methods #48531
• [10.x] Adds the firstOrCreate and createOrFirst methods to the HasManyThrough relation #48541
• [10.x] Fix firstOrNew on HasManyThrough relations #48542

This occurred because of having defined HasManyThrough::updateOrCreate without defining HasManyThrough::createOrFirst HasManyThrough::firstOrCreate methods. The absence of definitions for HasManyThrough::firstOrCreate HasManyThrough::updateOrCreate was itself a latent bug, but it seems to have been working correctly by chance up until now. This surfaced when Builder::firstOrCreate was used in HasManyThrough::updateOrCreate.

This has also been fixed, but as a result, many of the improvements were reverted.

Future Outlook

Concerning the reversion of semantic changes to firstOrCreate updateOrCreate facilitated by createOrFirst, I have no disagreement as it was indeed too packed for a non-breaking change in 10.x. Initially, I was quite wary of it.

Nonetheless, it's a too precious improvement to be completely discarded. I am inclined to positively consider reviving the reverted changes in 11.x. So far, every time I engage in highly concurrent application development, standard firstOrCreate updateOrCreate prove to be inadequate, forcing me to implement retry processes using my custom library https://github.com/mpyw/laravel-retry-on-duplicate-key. It’s disheartening that incorporating a custom library becomes virtually a necessity in future development; hence, I earnestly desire this aspect to be adopted within Laravel's core.

However, consensus within the community is necessary to undertake this. I am in favor of a revival post 11.x but would love to hear everyone's thoughts. If opposed, I’d appreciate suggestions for alternatives (like continuing with third-party libraries as before).

[fuwasegu]

I have commented on and helped validate several of the above PRs and issues．
I hope that the HasManyThrough bug is properly addressed and that we can get back to the pre-revert state soon!
Therefore, I agree with @mpyw that revert should be reinstated in 11.x!

[mpyw]

Possible options:

• Revival in Laravel 11.x
  • I am eagerly hoping for this.
• Revival from Laravel 12.x onwards
  • If more caution is needed, I don't mind it being revived in version 12 or later.
• Continue relying on third-party libraries
  • I would be very sad, but if the community members have strong reservations, this is also fine.
• Add optional arguments to firstOrCreate updateOrCreate to control behavior
  • Not highly recommended as it adds complexity.
• Add methods with different names from firstOrCreate updateOrCreate
  • Not highly recommended as it adds complexity.
  • I cannot think of a suitable name for the methods.

[mpyw]

@taylorotwell @crynobone @tonysm @ralphjsmit @ryanldy We need your opinions! Please feel free to comment here. Thank you 🙇

[ralphjsmit]

I'd say that we can try adding it back to 10.x, because the new method is also already used under-the-hood in the normal non-relation updateOrCreate() (and firstOrCreate() method). So in that sense it would feel a bit unlogical to me introduce the underlying change in the normal builder and not add it/skip the HasManyThrough.

[SakiTakamachi]

I've been watching this issue because it was affected by an issue with PDO SQLSRV that I was working on.

This is a very positive and nice change, but it may be beyond the scale of a minor update.

I also vote for introduction in 11.x.

[tonysm]

I'm not against the changes. We only have to be extra careful when touching these existing methods. To be fair, the updateOrCreate issue wasn't caused by the createOrFirst. The error was already present in the framework (the missing firstOrCreate method in the HasManyThrough relation) and surfaced when the updateOrCreate was changed to use firstOrCreate. It was more like an unfortunate "blind spot" (now fixed with regression tests).

As I see it, we have two possibilities:

1. Reintroduce the changes, but this time take a closer look at the generated queries before and after each change on each relation
2. Introduce a new createOrUpdate method that relies on unique constraints using the createOrFirst method

Neither of these changes would require a major version bump, IMO. In the case of reintroducing the changes, we're not changing the API of the methods, just how it works internally. Again, the updateOrCreate issue on HasManyThrough was an unfortunate "blind spot", which now has tests and was fixed. The PR would have to include before/after SQL generated for the builder and each relation (if @taylorotwell still wants to see that happen, ofc.)

[trevorgehman]

We have a highly concurrent application and firstOrCreate would often throw PDO exceptions due to unique constraints, so I was happy when it started using createOrFirst under the hood.

Unfortunately with this revert, I'll need to reimplement that custom solution again. I too would like to see this reintroduced in 11.x.

[mpyw]

I emailed Taylor and asked him about it. I'm waiting for his reply.

[mpyw]

@tonysm Taylor told me that any PRs were okay if there were additional tests. We'll start for 10.x!

[mpyw]

I reconsidered going with 10.x because there are more people looking forward to this feature, so it would be better to allow the re-change earlier. Let's proceed with this plan!

[mpyw]

• [10.x] Revival of the reverted changes in 10.25.0: firstOrCreate updateOrCreate improvement through createOrFirst + additional query tests  #48637

[mpyw]

Ready for review.

[mpyw]

Merged!